From 574cd85bf08ceae7406ae930cf12a47abdc5b006 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Mon, 19 Feb 2024 20:49:15 -0500 Subject: [PATCH] feat: use odh global config when provided. Signed-off-by: Humair Khan --- api/v1alpha1/dspipeline_types.go | 4 ++ ...b.io_datasciencepipelinesapplications.yaml | 8 +++- controllers/config/defaults.go | 17 ++++++-- controllers/dspipeline_params.go | 41 +++++++++++++++---- controllers/util/util.go | 9 +--- 5 files changed, 57 insertions(+), 22 deletions(-) diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index d8922956f..d52f3bdbb 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -108,6 +108,10 @@ type APIServer struct { // provide a PEM formatted CA bundle to be injected into the DSP // server pod to trust this connection. CA Bundle should be provided // as values within configmaps, mapped to keys. + // + // Note that if a global cert via ODH or the User is provided in this DSPA's + // namespace with the name "odh-trusted-ca-bundle", then that configmap + // is automatically used instead, and "caBundle" is ignored. CABundle *CABundle `json:"cABundle,omitempty"` } diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml index 5bbb80be5..9e49832df 100644 --- a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml +++ b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml @@ -62,11 +62,15 @@ spec: description: 'Default: true' type: boolean cABundle: - description: If the Object store/DB is behind a TLS secured connection + description: "If the Object store/DB is behind a TLS secured connection that is unrecognized by the host OpenShift/K8s cluster, then you can provide a PEM formatted CA bundle to be injected into the DSP server pod to trust this connection. CA Bundle should - be provided as values within configmaps, mapped to keys. + be provided as values within configmaps, mapped to keys. \n + Note that if a global cert via ODH or the User is provided in + this DSPA's namespace with the name \"odh-trusted-ca-bundle\", + then that configmap is automatically used instead, and \"caBundle\" + is ignored." properties: configMapKey: description: Key should map to a CA bundle. The key is also diff --git a/controllers/config/defaults.go b/controllers/config/defaults.go index b13ef3c78..14991db2a 100644 --- a/controllers/config/defaults.go +++ b/controllers/config/defaults.go @@ -27,10 +27,19 @@ const ( DefaultImageValue = "MustSetInConfig" APIServerPiplinesCABundleMountPath = "/etc/pki/tls/certs" PiplinesCABundleMountPath = "/etc/pki/tls/certs" - MLPipelineUIConfigMapPrefix = "ds-pipeline-ui-configmap-" - ArtifactScriptConfigMapNamePrefix = "ds-pipeline-artifact-script-" - ArtifactScriptConfigMapKey = "artifact_script" - DSPServicePrefix = "ds-pipeline" + + // GlobalCaBundleConfigMapName key and label values are a contract with + // ODH Platform https://github.com/opendatahub-io/architecture-decision-records/pull/28 + GlobalCaBundleConfigMapName = "odh-trusted-ca-bundle" + + // GlobalCaBundleConfigMapKey is the key provided by the configmap created via OCP Cluster Network Operator + // https://docs.openshift.com/container-platform/4.14/networking/configuring-a-custom-pki.html#certificate-injection-using-operators_configuring-a-custom-pki + GlobalCaBundleConfigMapKey = "ca-bundle.crt" + + MLPipelineUIConfigMapPrefix = "ds-pipeline-ui-configmap-" + ArtifactScriptConfigMapNamePrefix = "ds-pipeline-artifact-script-" + ArtifactScriptConfigMapKey = "artifact_script" + DSPServicePrefix = "ds-pipeline" DBSecretNamePrefix = "ds-pipeline-db-" DBSecretKey = "password" diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index 1d8d8d465..6b861dab2 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -468,16 +468,39 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip } } - // If a Custom CA Bundle is specified for injection into DSP API Server Pod - // then retrieve the bundle to utilize during storage health check - if p.APIServer.CABundle != nil { - cfgKey, cfgName := p.APIServer.CABundle.ConfigMapKey, p.APIServer.CABundle.ConfigMapName - err, val := util.GetConfigMapValue(ctx, cfgKey, cfgName, p.Namespace, client, log) - if err != nil { - log.Error(err, "Encountered error when attempting to retrieve CABundle from configmap") - return err + // Check for Global certs + // If it exists, we use this cert + // If no global cert provided, check if a custom bundle is provided via the DSPA + globalCABundleCFGMapKey, globalCABundleCFGMapName := config.GlobalCaBundleConfigMapKey, config.GlobalCaBundleConfigMapName + err, globalCerVal := util.GetConfigMapValue(ctx, globalCABundleCFGMapKey, globalCABundleCFGMapName, p.Namespace, client, log) + if err != nil && apierrs.IsNotFound(err) { + // If the global cert configmap is not available, that is OK + // proceed to check if the user has provided their + // own configmap via DSPA config + if p.APIServer.CABundle != nil { + dspaCaBundleCfgKey, dspaCaBundleCfgName := p.APIServer.CABundle.ConfigMapKey, p.APIServer.CABundle.ConfigMapName + dspaCACfgErr, dspaProvidedCABundle := util.GetConfigMapValue(ctx, dspaCaBundleCfgKey, dspaCaBundleCfgName, p.Namespace, client, log) + if dspaCACfgErr != nil && apierrs.IsNotFound(dspaCACfgErr) { + log.Info(fmt.Sprintf("ConfigMap [%s] was not found in namespace [%s]", dspaCaBundleCfgKey, p.Namespace)) + return dspaCACfgErr + } else if dspaCACfgErr != nil { + log.Info(fmt.Sprintf("Encountered error when attempting to fetch ConfigMap: [%s], Error: %v", dspaCaBundleCfgName, dspaCACfgErr)) + return dspaCACfgErr + } + p.APICustomPemCerts = []byte(dspaProvidedCABundle) + } + } else if err != nil { + log.Info(fmt.Sprintf("Encountered error when attempting to fetch ConfigMap: [%s], Error: %v", globalCABundleCFGMapKey, err)) + return err + } else { + // Found a global cert, consume this cert, takes precedence over "cABundle" provided via DSPA + log.Info(fmt.Sprintf("Found global CA Bundle %s present in this namespace %s, this cert will be "+ + "included to verify external tls connections in this DSPA.", config.GlobalCaBundleConfigMapName, p.Namespace)) + p.APICustomPemCerts = []byte(globalCerVal) + p.APIServer.CABundle = &dspa.CABundle{ + ConfigMapName: config.GlobalCaBundleConfigMapName, + ConfigMapKey: config.GlobalCaBundleConfigMapKey, } - p.APICustomPemCerts = []byte(val) } } diff --git a/controllers/util/util.go b/controllers/util/util.go index 555ce0938..f55502f4e 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -23,7 +23,6 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" - apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "net/url" @@ -72,16 +71,12 @@ func GetConfigMapValue(ctx context.Context, cfgKey, cfgName, ns string, client c Namespace: ns, } err := client.Get(ctx, namespacedName, cfgMap) - if err != nil && apierrs.IsNotFound(err) { - log.Error(err, fmt.Sprintf("ConfigMap [%s] was not found in namespace [%s]", cfgName, ns)) - return err, "" - } else if err != nil { - log.Error(err, fmt.Sprintf("Encountered error when attempting to fetch ConfigMap. [%s]..", cfgName)) + if err != nil { return err, "" } if val, ok := cfgMap.Data[cfgKey]; ok { return nil, val } else { - return fmt.Errorf("ConfigMap %s sdoes not contain specified key %s", cfgName, cfgKey), "" + return fmt.Errorf("ConfigMap %s sdoes not contain expected key %s", cfgName, cfgKey), "" } }