chore: allow dspo to handle multiple ca bundles

When a configmap trust ca store is provided, whether it's from a user
provisioned configmap with a ca bundle via the DSPA's caBundle field,
or whether it's a configmap name "odh-trusted-ca-bundle" (provided
by odh or user), then this change will create an amalgmation of all
of the certs from these configmaps and create a single dspo managed
configmap. The resulting configmap has a single cert that is the
accumulation of all afore mentioned certs, and can be passed as a single
file to the dsp server, or pipeline pods to utilize.

This helps work around issues and overhead work required when mounting
to a ca path with multiple certs, because such a path requires that it
first be re_hashed. Furthermore this allows us to work around issues
related to aws cli being unable to utilize ca paths with it's
AWS_CA_BUNDLE env variable when passing artifacts during the
step-copy-artifacts step.

Signed-off-by: Humair Khan <HumairAK@users.noreply.github.com>

api/v1alpha1/dspipeline_types.go

@@ -117,10 +117,6 @@ 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"`
 }
 

config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml

@@ -66,15 +66,11 @@ 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. \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."
+                      be provided as values within configmaps, mapped to keys.
                     properties:
                       configMapKey:
                         description: Key should map to a CA bundle. The key is also

config/internal/apiserver/default/artifact_script.yaml.tmpl

@@ -8,8 +8,8 @@ data:
         artifact_name=$(basename $2)
 
         aws_cp() {
-{{ if .APIServer.CABundle }}
-          aws s3 --endpoint {{.ObjectStorageConnection.Endpoint}} --ca-bundle {{ .PiplinesCABundleMountPath }}/{{ .APIServer.CABundle.ConfigMapKey }} cp $1.tgz s3://{{.ObjectStorageConnection.Bucket}}/artifacts/$PIPELINERUN/$PIPELINETASK/$1.tgz
+{{ if .CustomCABundle }}
+          aws s3 --endpoint {{.ObjectStorageConnection.Endpoint}} --ca-bundle {{ .PiplinesCABundleMountPath }} cp $1.tgz s3://{{.ObjectStorageConnection.Bucket}}/artifacts/$PIPELINERUN/$PIPELINETASK/$1.tgz
 {{ else }}
           aws s3 --endpoint {{.ObjectStorageConnection.Endpoint}} cp $1.tgz s3://{{.ObjectStorageConnection.Bucket}}/artifacts/$PIPELINERUN/$PIPELINETASK/$1.tgz
 {{ end }}

config/internal/apiserver/default/deployment.yaml.tmpl

@@ -37,13 +37,17 @@ spec:
               value: "{{.DBConnection.Host}}"
             - name: DBCONFIG_PORT
               value: "{{.DBConnection.Port}}"
-            {{ if .APIServer.CABundle }}
+            {{ if .CustomCABundle }}
             - name: ARTIFACT_COPY_STEP_CABUNDLE_CONFIGMAP_NAME
-              value: "{{.APIServer.CABundle.ConfigMapName}}"
+              value: "{{.CustomCABundle.ConfigMapName}}"
             - name: ARTIFACT_COPY_STEP_CABUNDLE_CONFIGMAP_KEY
-              value: "{{.APIServer.CABundle.ConfigMapKey}}"
+              value: "{{.CustomCABundle.ConfigMapKey}}"
             - name: ARTIFACT_COPY_STEP_CABUNDLE_MOUNTPATH
-              value: {{ .PiplinesCABundleMountPath }}
+              value: {{ .CustomCABundleRootMountPath }}
+            {{ end }}
+            {{ if .CustomSSLCertDir }}
+            - name: SSL_CERT_DIR
+              value: {{.CustomSSLCertDir}}
             {{ end }}
             - name: AUTO_UPDATE_PIPELINE_DEFAULT_VERSION
               value: "{{.APIServer.AutoUpdatePipelineDefaultVersion}}"
@@ -206,7 +210,7 @@ spec:
               memory: {{.APIServer.Resources.Limits.Memory}}
               {{ end }}
             {{ end }}
-          {{ if or .APIServer.EnableSamplePipeline .APIServer.CABundle }}
+          {{ if or .APIServer.EnableSamplePipeline .CustomCABundle }}
           volumeMounts:
             - name: server-config
               mountPath: /config/config.json
@@ -218,8 +222,8 @@ spec:
             - name: sample-pipeline
               mountPath: /samples/
             {{ end }}
-            {{ if .APIServer.CABundle }}
-            - mountPath: {{ .APIServerPiplinesCABundleMountPath  }}
+            {{ if .CustomCABundle }}
+            - mountPath: {{ .CustomCABundleRootMountPath  }}
               name: ca-bundle
             {{ end }}
           {{ end }}
@@ -279,13 +283,10 @@ spec:
         - name: server-config
           configMap:
             name: pipeline-server-config-{{.Name}}
-        {{ if .APIServer.CABundle }}
+        {{ if .CustomCABundle }}
         - name: ca-bundle
           configMap:
-            name: {{ .APIServer.CABundle.ConfigMapName }}
-            items:
-              - key: {{ .APIServer.CABundle.ConfigMapKey }}
-                path: {{ .APIServer.CABundle.ConfigMapKey }}
+            name: {{ .CustomCABundle.ConfigMapName }}
         {{ end }}
         {{ if .APIServer.EnableSamplePipeline }}
         - name: sample-config

controllers/config/defaults.go

@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
 	"time"
 
 	dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1"
@@ -25,17 +26,16 @@ import (
 )
 
 const (
-	DefaultImageValue                  = "MustSetInConfig"
-	APIServerPiplinesCABundleMountPath = "/etc/pki/tls/certs"
-	PiplinesCABundleMountPath          = "/etc/pki/tls/certs"
+	DefaultImageValue = "MustSetInConfig"
 
-	// GlobalCaBundleConfigMapName key and label values  are a contract with
+	CustomCABundleRootMountPath = "/dsp-custom-certs"
+
+	// GlobalODHCaBundleConfigMapName 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"
+	GlobalODHCaBundleConfigMapName = "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"
+	CustomDSPTrustedCAConfigMapNamePrefix = "dsp-trusted-ca"
+	CustomDSPTrustedCAConfigMapKey        = "dsp-ca.crt"
 
 	MLPipelineUIConfigMapPrefix       = "ds-pipeline-ui-configmap-"
 	ArtifactScriptConfigMapNamePrefix = "ds-pipeline-artifact-script-"
@@ -205,3 +205,12 @@ func GetDurationConfigWithDefault(configName string, value time.Duration) time.D
 	}
 	return viper.GetDuration(configName)
 }
+
+// GetCABundleFileMountPath provides the location in pipeline step-copy-artifact step where the
+// ca bundle is mounted for aws cli to connect to s3 store.
+// Since pipeline step-copy-artifact step uses aws cli, and there are issues surrounding
+// passing a path to aws cli (see: https://github.com/aws/aws-cli/issues/3425#issuecomment-402289636)
+// as such for pipelines, we concatenate the certs into a single cert bundle and use a separate configmap for this
+func GetCABundleFileMountPath() string {
+	return fmt.Sprintf("%s/%s", CustomCABundleRootMountPath, CustomDSPTrustedCAConfigMapKey)
+}

controllers/database.go

@@ -45,7 +45,7 @@ var mariadbTemplates = []string{
 	"mariadb/default/mariadb-sa.yaml.tmpl",
 }
 
-func tLSClientConfig(pem []byte) (*cryptoTls.Config, error) {
+func tLSClientConfig(pems [][]byte) (*cryptoTls.Config, error) {
 	rootCertPool := x509.NewCertPool()
 
 	if f := os.Getenv("SSL_CERT_FILE"); f != "" {
@@ -55,7 +55,7 @@ func tLSClientConfig(pem []byte) (*cryptoTls.Config, error) {
 		}
 	}
 
-	if len(pem) != 0 {
+	for _, pem := range pems {
 		if ok := rootCertPool.AppendCertsFromPEM(pem); !ok {
 			return nil, fmt.Errorf("error parsing CA Certificate, ensure provided certs are in valid PEM format")
 		}
@@ -96,7 +96,7 @@ var ConnectAndQueryDatabase = func(
 	log logr.Logger,
 	port, username, password, dbname, tls string,
 	dbConnectionTimeout time.Duration,
-	pemCerts []byte,
+	pemCerts [][]byte,
 	extraParams map[string]string) (bool, error) {
 
 	mysqlConfig := createMySQLConfig(

controllers/dspipeline_params.go

@@ -17,11 +17,14 @@ limitations under the License.
 package controllers
 
 import (
+	"bytes"
 	"context"
 	"encoding/base64"
 	"fmt"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/json"
 	"math/rand"
+	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -43,11 +46,8 @@ type DSPAParams struct {
 	Owner                                mf.Owner
 	DSPVersion                           string
 	APIServer                            *dspa.APIServer
-	APIServerPiplinesCABundleMountPath   string
-	PiplinesCABundleMountPath            string
 	APIServerDefaultResourceName         string
 	APIServerServiceName                 string
-	APICustomPemCerts                    []byte
 	OAuthProxy                           string
 	ScheduledWorkflow                    *dspa.ScheduledWorkflow
 	ScheduledWorkflowDefaultResourceName string
@@ -61,6 +61,23 @@ type DSPAParams struct {
 	WorkflowController                   *dspa.WorkflowController
 	DBConnection
 	ObjectStorageConnection
+
+	// TLS
+	// The CA bundle path used by API server
+	CustomCABundleRootMountPath string
+	// This path is used by API server to also look
+	// for CustomCABundleRootMountPath when
+	// verifying certs
+	CustomSSLCertDir *string
+	// The CA bundle path found in the pipeline pods
+	PiplinesCABundleMountPath string
+	// Collects all certs from user & global certs
+	APICustomPemCerts [][]byte
+	// Source of truth for the DSP cert configmap details
+	// If this is defined, then we assume we have additional certs
+	// we need to leverage for tls connections within dsp apiserver
+	// pipeline pods
+	CustomCABundle *dspa.CABundle
 }
 
 type DBConnection struct {
@@ -540,8 +557,8 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 	p.Minio = dsp.Spec.ObjectStorage.Minio.DeepCopy()
 	p.OAuthProxy = config.GetStringConfigWithDefault(config.OAuthProxyImagePath, config.DefaultImageValue)
 	p.MLMD = dsp.Spec.MLMD.DeepCopy()
-	p.APIServerPiplinesCABundleMountPath = config.APIServerPiplinesCABundleMountPath
-	p.PiplinesCABundleMountPath = config.PiplinesCABundleMountPath
+	p.CustomCABundleRootMountPath = config.CustomCABundleRootMountPath
+	p.PiplinesCABundleMountPath = config.GetCABundleFileMountPath()
 
 	log := loggr.WithValues("namespace", p.Namespace).WithValues("dspa_name", p.Name)
 
@@ -566,7 +583,6 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 		setStringDefault(moveResultsImageFromConfig, &p.APIServer.MoveResultsImage)
 		setStringDefault(argoLauncherImageFromConfig, &p.APIServer.ArgoLauncherImage)
 		setStringDefault(argoDriverImageFromConfig, &p.APIServer.ArgoDriverImage)
-
 		setResourcesDefault(config.APIServerResourceRequirements, &p.APIServer.Resources)
 
 		if p.APIServer.ArtifactScriptConfigMap == nil {
@@ -577,38 +593,102 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 		}
 
 		// 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 it exists, include this cert for tls verifications
+		globalCABundleCFGMapName := config.GlobalODHCaBundleConfigMapName
+		err, globalCerts := util.GetConfigMapValues(ctx, globalCABundleCFGMapName, p.Namespace, client)
+		if err != nil {
 			// 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)
+			if !apierrs.IsNotFound(err) {
+				log.Info(fmt.Sprintf("Encountered error when attempting to fetch ConfigMap: [%s], Error: %v", globalCABundleCFGMapName, err))
+				return err
 			}
-		} 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,
+			log.Info(fmt.Sprintf("Found global CA Bundle %s present in this namespace %s, this bundle will be included in external tls connections.", config.GlobalODHCaBundleConfigMapName, p.Namespace))
+			// "odh-trusted-ca-bundle" can have fields: "odh-ca-bundle.crt" and "ca-bundle.crt", we need to utilize both
+			for _, val := range globalCerts {
+				// If the ca-bundle field is empty, ignore it
+				if val != "" {
+					p.APICustomPemCerts = append(p.APICustomPemCerts, []byte(val))
+				}
+			}
+		}
+
+		// If user provided a CA bundle, include this in tls verification
+		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
+			}
+			// If the ca-bundle field is empty, ignore it
+			if dspaProvidedCABundle != "" {
+				p.APICustomPemCerts = append(p.APICustomPemCerts, []byte(dspaProvidedCABundle))
+			}
+		}
+
+		// There are situations where global & user provided certs, or a provided ca trust configmap(s) have various trust bundles
+		// (for example in the case of "odh-trusted-ca-bundle") there is "odh-ca-bundle.crt" and "ca-bundle.crt".
+		// We create a separate configmap and concatenate all the certs into a single bundle, because passing a
+		// full path into the pipeline doesn't seem to work with aws cli used for artifact passing
+		// Ref: https://github.com/aws/aws-cli/issues/3425#issuecomment-402289636
+
+		// If user or global CABundle has been provided
+		// 1) create the dsp-trusted-ca configmap
+		// 2) populate CustomCABundle SOT var for pipeline pods and artifact script to utilize during templating
+		// 3) set ssl_cert_dir for api server
+		if len(p.APICustomPemCerts) > 0 {
+			p.CustomCABundle = &dspa.CABundle{
+				ConfigMapKey:  config.CustomDSPTrustedCAConfigMapKey,
+				ConfigMapName: fmt.Sprintf("%s-%s", config.CustomDSPTrustedCAConfigMapNamePrefix, p.Name),
+			}
+
+			// Combine certs into a single configmap field
+			customCABundleCert := &v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      p.CustomCABundle.ConfigMapName,
+					Namespace: p.Namespace,
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion:         dsp.APIVersion,
+							Kind:               dsp.Kind,
+							Name:               dsp.Name,
+							UID:                dsp.UID,
+							Controller:         util.BoolPointer(true),
+							BlockOwnerDeletion: util.BoolPointer(true),
+						},
+					},
+				},
+
+				Data: map[string]string{
+					p.CustomCABundle.ConfigMapKey: string(bytes.Join(p.APICustomPemCerts, []byte{})),
+				},
+			}
+
+			err := client.Create(ctx, customCABundleCert)
+			if apierrs.IsAlreadyExists(err) {
+				err := client.Update(ctx, customCABundleCert)
+				if err != nil {
+					return err
+				}
+			} else if err != nil {
+				return err
+			}
+
+			// We need to update the default SSL_CERT_DIR to include
+			// dsp custom cert path, used by DSP Api Server
+			var certDirectories = []string{
+				config.CustomCABundleRootMountPath,
+				"/etc/ssl/certs",     // SLES10/SLES11, https://golang.org/issue/12139
+				"/etc/pki/tls/certs", // Fedora/RHEL
 			}
+			// SSL_CERT_DIR accepts a colon separated list of directories
+			sslCertDir := strings.Join(certDirectories, ":")
+			p.CustomSSLCertDir = &sslCertDir
 		}
 	}