HPCC-30468 Use service hostname for external dafilesrv if available

Also: + auto-generate secret name for secure dfs service. + Refresh directio endpoint in remapGroupsToDafilesrv on config changes. Signed-off-by: Jake Smith <[email protected]>
hpcc-systems · Oct 11, 2023 · f6da4b8 · f6da4b8
1 parent 0ec6de0
commit f6da4b8
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 23 deletions.
diff --git a/dali/base/dautils.cpp b/dali/base/dautils.cpp
@@ -3612,8 +3612,10 @@ void addStripeDirectory(StringBuffer &out, const char *directory, const char *pl
     }
 }
 
+static CConfigUpdateHook directIOUpdateHook;
 static CriticalSection dafileSrvNodeCS;
 static Owned<INode> dafileSrvNode;
+
 void remapGroupsToDafilesrv(IPropertyTree *file, INamedGroupStore *resolver)
 {
     FileDescriptorFlags fileFlags = static_cast<FileDescriptorFlags>(file->getPropInt("Attr/@flags"));
@@ -3625,15 +3627,14 @@ void remapGroupsToDafilesrv(IPropertyTree *file, INamedGroupStore *resolver)
         Owned<IStoragePlane> plane = getDataStoragePlane(planeName, true);
         if ((0 == plane->queryHosts().size()) && isAbsolutePath(plane->queryPrefix())) // if hosts group, or url, don't touch
         {
+            auto updateFunc = [&](const IPropertyTree *oldComponentConfiguration, const IPropertyTree *oldGlobalConfiguration)
             {
                 CriticalBlock b(dafileSrvNodeCS);
-                if (nullptr == dafileSrvNode)
-                {
-                    auto externalService = k8s::getDafileServiceFromConfig("directio");
-                    VStringBuffer dafilesrvEpStr("%s:%u", externalService.first.c_str(), externalService.second);
-                    dafileSrvNode.setown(createINode(dafilesrvEpStr));
-                }
-            }
+                auto externalService = k8s::getDafileServiceFromConfig("directio");
+                VStringBuffer dafilesrvEpStr("%s:%u", externalService.first.c_str(), externalService.second);
+                dafileSrvNode.setown(createINode(dafilesrvEpStr));
+            };
+            directIOUpdateHook.installOnce(updateFunc, true);
 
             Owned<IGroup> group;
             if (cluster.hasProp("Group"))

diff --git a/esp/clients/ws_dfsclient/ws_dfsclient.cpp b/esp/clients/ws_dfsclient/ws_dfsclient.cpp
@@ -398,7 +398,9 @@ class CServiceDistributedFile : public CServiceDistributedFileBase<IDistributedF
             if (dafileSrvRemoteFilePlane)
             {
                 file->setPropTree("Attr/_remoteStoragePlane", createPTreeFromIPT(dafileSrvRemoteFilePlane));
-                if (remoteStorage->hasProp("@secret"))
+
+                const char *serviceUrl = remoteStorage->queryProp("@service");
+                if (serviceUrl && startsWith(serviceUrl, "https"))
                 {
                     // if remote storage service is secure, dafilesrv connections must be also.
                     // this flag is used by consumers of this IFleDescriptor to tell whether they need to make
@@ -673,7 +675,10 @@ IDFSFile *lookupDFSFile(const char *logicalName, AccessMode accessMode, unsigned
             if (!remoteStorage)
                 throw makeStringExceptionV(0, "Remote storage '%s' not found", remoteName.str());
             serviceUrl.set(remoteStorage->queryProp("@service"));
+
+            // NB: for legacy support only, if the service url is secure, a secret name will be auto-generated
             serviceSecret.set(remoteStorage->queryProp("@secret"));
+
             logicalName = remoteLogicalFileName;
             useDafilesrv = remoteStorage->getPropBool("@useDafilesrv");
         }
@@ -683,20 +688,18 @@ IDFSFile *lookupDFSFile(const char *logicalName, AccessMode accessMode, unsigned
         // auto-discover local environment dfs service.
 #ifdef _CONTAINERIZED
         // NB: only expected to be here if experimental option #option('dfsesp-localfiles', true); is in use.
-        // This finds and uses local dfs service for local read lookukup.
+        // This finds and uses local dfs service for local read lookkups.
         Owned<IPropertyTreeIterator> eclWatchServices = getGlobalConfigSP()->getElements("services[@type='dfs']");
         if (!eclWatchServices->first())
             throw makeStringException(-1, "Dfs service not defined in esp services");
         const IPropertyTree &eclWatch = eclWatchServices->query();
         StringBuffer eclWatchName;
         eclWatch.getProp("@name", eclWatchName);
-        auto result = k8s::getExternalService(eclWatchName);
-        if (result.first.empty())
-            throw makeStringExceptionV(-1, "dfs '%s': service not found", eclWatchName.str());
-        if (0 == result.second)
-            throw makeStringExceptionV(-1, "dfs '%s': service port not defined", eclWatchName.str());
         const char *protocol = eclWatch.getPropBool("@tls") ? "https" : "http";
-        serviceUrl.appendf("%s://%s:%u", protocol, result.first.c_str(), result.second);
+        unsigned port = (unsigned)eclWatch.getPropInt("@port", NotFound);
+        if (NotFound == port)
+            throw makeStringExceptionV(-1, "dfs '%s': service port not defined", eclWatchName.str());
+        serviceUrl.appendf("%s://%s:%u", protocol, eclWatchName.str(), port);
 #else
         {
             CriticalBlock b(dfsServiceUrlCrit);
@@ -714,7 +717,12 @@ IDFSFile *lookupDFSFile(const char *logicalName, AccessMode accessMode, unsigned
 #endif
     }
     bool useSSL = startsWith(serviceUrl, "https");
-    if (!useSSL)
+    if (useSSL)
+    {
+        if (0 == serviceSecret.length())
+            generateDynamicUrlSecretName(serviceSecret, serviceUrl, nullptr);
+    }
+    else
         serviceSecret.clear();
 
     DBGLOG("Looking up file '%s' on '%s'", logicalName, serviceUrl.str());

diff --git a/helm/hpcc/templates/_helpers.tpl b/helm/hpcc/templates/_helpers.tpl
@@ -1140,6 +1140,16 @@ Generate service entries for TLS
   {{- end }}
 {{- end }}
 
+{{/*
+Conditionally generate annotated k8s hostname
+*/}}
+{{- define "hpcc.outputHostname" -}}
+{{- $annotations := .annotations | default dict -}}
+{{- if hasKey $annotations "external-dns.alpha.kubernetes.io/hostname" -}}
+hostname: {{ index $annotations "external-dns.alpha.kubernetes.io/hostname" }}
+{{- end -}}
+{{- end -}}
+
 {{/*
 Generate list of available services
 */}}
@@ -1209,6 +1219,7 @@ Generate list of available services
   port: {{ .service.servicePort | default 7600 }}
   public: {{ (ne ( include "hpcc.isVisibilityPublic" (dict "root" $ "visibility" .service.visibility))  "") | ternary "true" "false" }}
   {{- include "hpcc.addTLSServiceEntries" (dict "root" $ "service" $dafilesrv.service "component" $dafilesrv "visibility" $dafilesrv.service.visibility) }}
+  {{- include "hpcc.outputHostname" .service | nindent 2 }}
  {{ end -}}
 {{ end -}}
 {{- end -}}

diff --git a/system/jlib/jcontainerized.cpp b/system/jlib/jcontainerized.cpp
@@ -379,12 +379,22 @@ std::pair<std::string, unsigned> getDafileServiceFromConfig(const char *applicat
         throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' has no public service defined", application);
     StringBuffer dafilesrvName;
     dafilesrv.getProp("@name", dafilesrvName);
-    auto externalService = getExternalService(dafilesrvName);
-    if (externalService.first.empty())
-        throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' not found", application, dafilesrvName.str());
-    if (0 == externalService.second)
-        throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' port not defined", application, dafilesrvName.str());
-    return externalService;
+    unsigned port = (unsigned)dafilesrv.getPropInt("@port");
+
+    StringBuffer hostname;
+    dafilesrv.getProp("@hostname", hostname);
+    if (hostname.length())
+        return { hostname.str(), port };
+    else
+    {
+        auto externalService = getExternalService(dafilesrvName);
+        if (externalService.first.empty())
+            throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' not found", application, dafilesrvName.str());
+        if (0 == externalService.second)
+            throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' port not defined", application, dafilesrvName.str());
+        assertex(port == externalService.second);
+        return externalService;
+    }
 }
 
 

diff --git a/system/jlib/jsecrets.cpp b/system/jlib/jsecrets.cpp
@@ -214,7 +214,8 @@ static void splitUrlSections(const char *url, const char * &authority, size_t &a
     else
     {
         authorityLen = path-url;
-        fullpath.append(path);
+        if (!streq(path, "/")) // treat empty trailing path as equal to no path
+            fullpath.append(path);
     }
 }