Merge pull request opendatahub-io#581 from HumairAK/stable

Stable

api/v1alpha1/dspipeline_types.go

@@ -108,10 +108,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/base/params.env

@@ -1,11 +1,11 @@
-IMAGES_APISERVER=quay.io/opendatahub/ds-pipelines-api-server@sha256:414a0ccfbfab8e58b23acbb29fe82369c382e9e6364b7e84e1fba0d1ac7745dd
-IMAGES_ARTIFACT=quay.io/opendatahub/ds-pipelines-artifact-manager@sha256:71a30327d22b32bc63687a5ea6a0de4027733798ac57677ebb2bf9a67dd94075
-IMAGES_PERSISTENTAGENT=quay.io/opendatahub/ds-pipelines-persistenceagent@sha256:3c752c66d2b9b446eac04844e152d74af8a4b5ae2f993aef633deb7184986e60
-IMAGES_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow@sha256:473be9788f38b5bd4a3833843724fa5037d4f31be39b6d25bf9dc60a31984ec8
+IMAGES_APISERVER=quay.io/opendatahub/ds-pipelines-api-server@sha256:579a29f4cd40ef3cf8ed199d0b9e2a6fd09a70fb68afad3db278a616609c6728
+IMAGES_ARTIFACT=quay.io/opendatahub/ds-pipelines-artifact-manager@sha256:a451356d8004c3609438d1fe21bc46e0ae54fa9d35532e8c9dac8cc269afd8d5
+IMAGES_PERSISTENTAGENT=quay.io/opendatahub/ds-pipelines-persistenceagent@sha256:f41b53112114105436155a95e06d35512272d36ab3ff50c3cbd432eb6ac77652
+IMAGES_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow@sha256:fccdcb63c87eef2b7e2db0477a74c2f3b4f42e0a8fc30b226555ec5b39cc4006
 IMAGES_MLMDENVOY=quay.io/opendatahub/ds-pipelines-metadata-envoy@sha256:c491e63c8885c7d59005f9305b77cd1fa776b50e63db90c4f8ccdee963759630
 IMAGES_MLMDGRPC=quay.io/opendatahub/ds-pipelines-metadata-grpc@sha256:4af88c246d77cce33099489090508734978aafa83a0a5745408ae8d139d5378a
-IMAGES_MLMDWRITER=quay.io/opendatahub/ds-pipelines-metadata-writer@sha256:886a52857bd30c18f9d5b4404bd77cbb6eb62b4077c759438f08d47cc65c21e8
-IMAGES_DSPO=quay.io/opendatahub/data-science-pipelines-operator@sha256:dae3f4aa1722e5df38a0e29feb1e10f1ca20e9c51487de49fb06d378163e12da
+IMAGES_MLMDWRITER=quay.io/opendatahub/ds-pipelines-metadata-writer@sha256:f14fe825f27258168f9c5e99d162e3737427d8b6917a697ee42c6dfb15f94879
+IMAGES_DSPO=quay.io/opendatahub/data-science-pipelines-operator@sha256:ad0fbec2c0d8e2dcd383e75eae069feee8394ff920471ff5a377fe3e3a3f7f66
 IMAGES_CACHE=registry.access.redhat.com/ubi8/ubi-minimal@sha256:a49924d9d685a35b2d0817ffe9c84f3429d97e9ad29ed3816c389f45564c9e19
 IMAGES_MOVERESULTSIMAGE=registry.access.redhat.com/ubi8/ubi-micro@sha256:396baed3d689157d96aa7d8988fdfea7eb36684c8335eb391cf1952573e689c1
 IMAGES_MARIADB=registry.redhat.io/rhel8/mariadb-103@sha256:3d30992e60774f887c4e7959c81b0c41b0d82d042250b3b56f05ab67fd4cdee1

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

@@ -62,15 +62,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/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/deployment.yaml.tmpl

@@ -50,13 +50,13 @@ spec:
               value: "{{.APIServer.ArtifactImage}}"
             - name: ARCHIVE_LOGS
               value: "{{.APIServer.ArchiveLogs}}"
-            {{ 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 }}
             - name: TRACK_ARTIFACTS
               value: "{{.APIServer.TrackArtifacts}}"
@@ -102,6 +102,10 @@ spec:
               value: "{{.APIServer.CacheImage}}"
             - name: MOVERESULTS_IMAGE
               value: "{{.APIServer.MoveResultsImage}}"
+            {{ if .CustomSSLCertDir }}
+            - name: SSL_CERT_DIR
+              value: {{.CustomSSLCertDir}}
+            {{ end }}
           image: {{.APIServer.Image}}
           imagePullPolicy: Always
           name: ds-pipeline-api-server
@@ -153,7 +157,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
@@ -165,8 +169,8 @@ spec:
             - name: sample-pipeline
               mountPath: /samples/
             {{ end }}
-            {{ if .APIServer.CABundle }}
-            - mountPath: {{ .APIServerPiplinesCABundleMountPath  }}
+            {{ if .CustomCABundle }}
+            - mountPath: {{ .CustomCABundleRootMountPath  }}
               name: ca-bundle
             {{ end }}
           {{ end }}
@@ -226,13 +230,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,24 +17,24 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
 	dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1"
 	"github.com/spf13/viper"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"time"
 )
 
 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-"
@@ -169,3 +169,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

@@ -41,7 +41,7 @@ var dbTemplates = []string{
 	dbSecret,
 }
 
-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 != "" {
@@ -51,8 +51,10 @@ func tLSClientConfig(pem []byte) (*cryptoTls.Config, error) {
 		}
 	}
 
-	if ok := rootCertPool.AppendCertsFromPEM(pem); !ok {
-		return nil, fmt.Errorf("error parsing CA Certificate, ensure provided certs are in valid PEM format")
+	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")
+		}
 	}
 	tlsConfig := &cryptoTls.Config{
 		RootCAs: rootCertPool,
@@ -88,7 +90,7 @@ var ConnectAndQueryDatabase = func(
 	host string,
 	log logr.Logger,
 	port, username, password, dbname, tls string,
-	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"
@@ -41,11 +44,8 @@ type DSPAParams struct {
 	Namespace                            string
 	Owner                                mf.Owner
 	APIServer                            *dspa.APIServer
-	APIServerPiplinesCABundleMountPath   string
-	PiplinesCABundleMountPath            string
 	APIServerDefaultResourceName         string
 	APIServerServiceName                 string
-	APICustomPemCerts                    []byte
 	OAuthProxy                           string
 	ScheduledWorkflow                    *dspa.ScheduledWorkflow
 	ScheduledWorkflowDefaultResourceName string
@@ -57,6 +57,23 @@ type DSPAParams struct {
 	MLMD                                 *dspa.MLMD
 	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 {
@@ -463,8 +480,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)
 
@@ -479,7 +496,6 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 		setStringDefault(artifactImageFromConfig, &p.APIServer.ArtifactImage)
 		setStringDefault(cacheImageFromConfig, &p.APIServer.CacheImage)
 		setStringDefault(moveResultsImageFromConfig, &p.APIServer.MoveResultsImage)
-
 		setResourcesDefault(config.APIServerResourceRequirements, &p.APIServer.Resources)
 
 		if p.APIServer.ArtifactScriptConfigMap == nil {
@@ -490,38 +506,96 @@ 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 {
+				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
+			}
+			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
 		}
 	}