From f6da4b87fd2fae1886229e2267c33721e91cd771 Mon Sep 17 00:00:00 2001 From: Jake Smith Date: Tue, 10 Oct 2023 19:50:11 +0100 Subject: [PATCH] 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 --- dali/base/dautils.cpp | 15 +++++++------ esp/clients/ws_dfsclient/ws_dfsclient.cpp | 26 +++++++++++++++-------- helm/hpcc/templates/_helpers.tpl | 11 ++++++++++ system/jlib/jcontainerized.cpp | 22 +++++++++++++------ system/jlib/jsecrets.cpp | 3 ++- 5 files changed, 54 insertions(+), 23 deletions(-) diff --git a/dali/base/dautils.cpp b/dali/base/dautils.cpp index 88e71fa41a4..db22a9cd49c 100644 --- 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 dafileSrvNode; + void remapGroupsToDafilesrv(IPropertyTree *file, INamedGroupStore *resolver) { FileDescriptorFlags fileFlags = static_cast(file->getPropInt("Attr/@flags")); @@ -3625,15 +3627,14 @@ void remapGroupsToDafilesrv(IPropertyTree *file, INamedGroupStore *resolver) Owned 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 group; if (cluster.hasProp("Group")) diff --git a/esp/clients/ws_dfsclient/ws_dfsclient.cpp b/esp/clients/ws_dfsclient/ws_dfsclient.cpp index 1d8d3ce030e..bdb5b82741d 100644 --- a/esp/clients/ws_dfsclient/ws_dfsclient.cpp +++ b/esp/clients/ws_dfsclient/ws_dfsclient.cpp @@ -398,7 +398,9 @@ class CServiceDistributedFile : public CServiceDistributedFileBasesetPropTree("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 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 index 7769069174b..80797335827 100644 --- 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 index 3b9724e2682..d6b83a7d6fe 100644 --- a/system/jlib/jcontainerized.cpp +++ b/system/jlib/jcontainerized.cpp @@ -379,12 +379,22 @@ std::pair 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 index 5828bc71166..b99e6265c7c 100644 --- 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); } }