feat: DSPO handle db tls connections and configs

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"`
 }
 
@@ -169,6 +173,19 @@ type MlPipelineUI struct {
 type Database struct {
 	*MariaDB    `json:"mariaDB,omitempty"`
 	*ExternalDB `json:"externalDB,omitempty"`
+
+	// +kubebuilder:validation:Optional
+	// CustomExtraParams allow users to further customize the sql dsn parameters used by the Pipeline Server
+	// when opening a connection with the Database.
+	// ref: https://github.com/go-sql-driver/mysql?tab=readme-ov-file#dsn-data-source-name
+	//
+	// Value must be a JSON string. For example, to disable tls for Pipeline Server DB connection
+	// the user can provide a string: {"tls":"true"}
+	//
+	// If updating post DSPA deployment, then a manual restart of the pipeline server pod will be required
+	// so the new configmap maybe consumed.
+	CustomExtraParams *string `json:"customExtraParams,omitempty"`
+
 	// Default: false
 	// +kubebuilder:default:=false
 	// +kubebuilder:validation:Optional

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
@@ -177,6 +181,16 @@ spec:
                   DS Pipelines metadata tracking. Specify either the default MariaDB
                   deployment, or configure your own External SQL DB.
                 properties:
+                  customExtraParams:
+                    description: "CustomExtraParams allow users to further customize
+                      the sql dsn parameters used by the Pipeline Server when opening
+                      a connection with the Database. ref: https://github.com/go-sql-driver/mysql?tab=readme-ov-file#dsn-data-source-name
+                      \n Value must be a JSON string. For example, to disable tls
+                      for Pipeline Server DB connection the user can provide a string:
+                      {\"tls\":\"true\"} \n If updating post DSPA deployment, then
+                      a manual restart of the pipeline server pod will be required
+                      so the new configmap maybe consumed."
+                    type: string
                   disableHealthCheck:
                     default: false
                     description: 'Default: false'

config/internal/apiserver/deployment.yaml.tmpl

@@ -155,6 +155,9 @@ spec:
             {{ end }}
           {{ if or .APIServer.EnableSamplePipeline .APIServer.CABundle }}
           volumeMounts:
+            - name: server-config
+              mountPath: /config/config.json
+              subPath: config.json
             {{ if .APIServer.EnableSamplePipeline }}
             - name: sample-config
               mountPath: /config/sample_config.json
@@ -220,6 +223,9 @@ spec:
         - name: proxy-tls
           secret:
             secretName: ds-pipelines-proxy-tls-{{.Name}}
+        - name: server-config
+          configMap:
+            name: pipeline-server-config-{{.Name}}
         {{ if .APIServer.CABundle }}
         - name: ca-bundle
           configMap:

config/internal/apiserver/server-config.yaml.tmpl

@@ -0,0 +1,18 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+    name: pipeline-server-config-{{.Name}}
+    namespace: {{.Namespace}}
+    labels:
+        app: {{.APIServerDefaultResourceName}}
+        component: data-science-pipelines
+data:
+    config.json: |
+      {
+        "DBConfig": {
+          "DriverName": "mysql",
+          "ConMaxLifeTime": "120s",
+          "ExtraParams": {{ .DBConnection.ExtraParams }}
+        },
+        "InitConnectionTimeout": "6m"
+      }

config/overlays/make-deploy/kustomization.yaml

@@ -1,6 +1,6 @@
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
-namespace: odh-applications
+namespace: opendatahub
 resources:
 - ../../base
 patchesStrategicMerge:

controllers/apiserver.go

@@ -28,6 +28,7 @@ const apiServerDefaultResourceNamePrefix = "ds-pipeline-"
 
 var apiServerTemplates = []string{
 	"apiserver/artifact_script.yaml.tmpl",
+	"apiserver/server-config.yaml.tmpl",
 	"apiserver/role_ds-pipeline.yaml.tmpl",
 	"apiserver/role_pipeline-runner.yaml.tmpl",
 	"apiserver/role_ds-pipeline-user-access.yaml.tmpl",

controllers/config/defaults.go

@@ -27,13 +27,23 @@ 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"
 
-	DBSecretNamePrefix = "ds-pipeline-db-"
-	DBSecretKey        = "password"
+	// 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"
+	DBDefaultExtraParams = "{\"tls\":\"%t\"}"
 
 	MariaDBName        = "mlpipeline"
 	MariaDBHostPrefix  = "mariadb"

controllers/database.go

@@ -17,12 +17,18 @@ package controllers
 
 import (
 	"context"
+	cryptoTls "crypto/tls"
+	"crypto/x509"
 	"database/sql"
 	b64 "encoding/base64"
 	"fmt"
+	"github.com/go-logr/logr"
+	"github.com/go-sql-driver/mysql"
 	_ "github.com/go-sql-driver/mysql"
 	dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1"
 	"github.com/opendatahub-io/data-science-pipelines-operator/controllers/config"
+	"k8s.io/apimachinery/pkg/util/json"
+	"os"
 )
 
 const dbSecret = "mariadb/secret.yaml.tmpl"
@@ -35,22 +41,115 @@ var dbTemplates = []string{
 	dbSecret,
 }
 
-// extract to var for mocking in testing
-var ConnectAndQueryDatabase = func(host, port, username, password, dbname string) bool {
+func tLSClientConfig(pem []byte) (*cryptoTls.Config, error) {
+	rootCertPool := x509.NewCertPool()
+
+	if f := os.Getenv("SSL_CERT_FILE"); f != "" {
+		data, err := os.ReadFile(f)
+		if err == nil {
+			rootCertPool.AppendCertsFromPEM(data)
+		}
+	}
+
+	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,
+	}
+	return tlsConfig, nil
+}
+
+func createMySQLConfig(user, password string, mysqlServiceHost string,
+	mysqlServicePort string, dbName string, mysqlExtraParams map[string]string) *mysql.Config {
+
+	params := map[string]string{
+		"charset":   "utf8",
+		"parseTime": "True",
+		"loc":       "Local",
+	}
+
+	for k, v := range mysqlExtraParams {
+		params[k] = v
+	}
+
+	return &mysql.Config{
+		User:                 user,
+		Passwd:               password,
+		Net:                  "tcp",
+		Addr:                 fmt.Sprintf("%s:%s", mysqlServiceHost, mysqlServicePort),
+		Params:               params,
+		DBName:               dbName,
+		AllowNativePasswords: true,
+	}
+}
+
+var ConnectAndQueryDatabase = func(
+	host string,
+	log logr.Logger,
+	port, username, password, dbname, tls string,
+	pemCerts []byte,
+	extraParams map[string]string) (bool, error) {
+
+	mysqlConfig := createMySQLConfig(
+		username,
+		password,
+		host,
+		port,
+		"",
+		extraParams,
+	)
+
 	// Create a context with a timeout of 1 second
 	ctx, cancel := context.WithTimeout(context.Background(), config.DefaultDBConnectionTimeout)
 	defer cancel()
 
-	connectionString := fmt.Sprintf("%s:%s@tcp(%s:%s)/%s", username, password, host, port, dbname)
-	db, err := sql.Open("mysql", connectionString)
+	var tlsConfig *cryptoTls.Config
+	switch tls {
+	case "false", "":
+		// don't set anything
+	case "true":
+		if len(pemCerts) != 0 {
+			var err error
+			tlsConfig, err = tLSClientConfig(pemCerts)
+			if err != nil {
+				log.Info(fmt.Sprintf("Encountered error when processing custom ca bundle, Error: %v", err))
+				return false, err
+			}
+		}
+	case "skip-verify", "preferred":
+		tlsConfig = &cryptoTls.Config{InsecureSkipVerify: true}
+	default:
+		// Unknown config, default to don't set anything
+	}
+
+	// Only register tls config in the case of: "true", "skip-verify", "preferred"
+	if tlsConfig != nil {
+		err := mysql.RegisterTLSConfig("custom", tlsConfig)
+		// If ExtraParams{"tls": ".."} is set, that takes precedent over mysqlConfig.TLSConfig
+		// so we need to make sure we're setting our tls config to be used instead if it exists
+		if _, ok := mysqlConfig.Params["tls"]; ok {
+			mysqlConfig.Params["tls"] = "custom"
+		}
+		// Just to be safe, we also set it here, fallback from mysqlConfig.Params["tls"] not being set
+		mysqlConfig.TLSConfig = "custom"
+		if err != nil {
+			return false, err
+		}
+	}
+
+	db, err := sql.Open("mysql", mysqlConfig.FormatDSN())
 	if err != nil {
-		return false
+		return false, err
 	}
 	defer db.Close()
 
 	testStatement := "SELECT 1;"
 	_, err = db.QueryContext(ctx, testStatement)
-	return err == nil
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
 func (r *DSPAReconciler) isDatabaseAccessible(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication,
@@ -72,16 +171,50 @@ func (r *DSPAReconciler) isDatabaseAccessible(ctx context.Context, dsp *dspav1al
 	}
 
 	decodePass, _ := b64.StdEncoding.DecodeString(params.DBConnection.Password)
-	dbHealthCheckPassed := ConnectAndQueryDatabase(params.DBConnection.Host,
+
+	var extraParamsJson map[string]string
+	err := json.Unmarshal([]byte(params.DBConnection.ExtraParams), &extraParamsJson)
+	if err != nil {
+		log.Info(fmt.Sprintf("Could not parse tls config in ExtraParams, if setting CustomExtraParams, ensure the JSON string is well-formed. Error: %v", err))
+		return false
+	}
+
+	// tls can be true, false, skip-verify, preferred
+	// we default to true if it's an externalDB, false otherwise
+	// (if not specified via CustomExtraParams)
+	tls := "false"
+	if usingExternalDB {
+		tls = "true"
+	}
+
+	// Override tls with the value in ExtraParams, if specified
+	// If users have specified a CustomExtraParams field, have to
+	// check for "tls" existence because users may choose to  leave
+	// out the "tls" param
+	if val, ok := extraParamsJson["tls"]; ok {
+		tls = val
+	}
+
+	dbHealthCheckPassed, err := ConnectAndQueryDatabase(
+		params.DBConnection.Host,
+		log,
 		params.DBConnection.Port,
 		params.DBConnection.Username,
 		string(decodePass),
-		params.DBConnection.DBName)
+		params.DBConnection.DBName,
+		tls,
+		params.APICustomPemCerts,
+		extraParamsJson,
+	)
+	if err != nil {
+		log.Info(fmt.Sprintf("Unable to connect to Database: %v", err))
+		return false
+	}
+
 	if dbHealthCheckPassed {
 		log.Info("Database Health Check Successful")
-	} else {
-		log.Info("Unable to connect to Database")
 	}
+
 	return dbHealthCheckPassed
 }
 

controllers/dspipeline_controller.go

@@ -516,6 +516,38 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.RoleBinding{}).
 		Owns(&routev1.Route{}).
+		// Watch for global ca bundle, if one is added to this namespace
+		// we need to reconcile on all the dspa's in this namespace
+		// so they may mount this cert in the appropriate containers
+		Watches(&source.Kind{Type: &corev1.ConfigMap{}},
+			handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request {
+				thisNamespace := o.GetNamespace()
+				log := r.Log.WithValues("namespace", thisNamespace)
+
+				if o.GetName() != "odh-trusted-ca-bundle" {
+					return []reconcile.Request{}
+				}
+
+				log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change in event on Global CA Bundle: %s", o.GetName()))
+
+				var dspaList dspav1alpha1.DataSciencePipelinesApplicationList
+				ctx := context.Background()
+				if err := r.List(ctx, &dspaList, client.InNamespace(thisNamespace)); err != nil {
+					log.Error(err, "unable to list DSPA's when attempting to handle Global CA Bundle event.")
+					return []reconcile.Request{}
+				}
+
+				var reconcileRequests []reconcile.Request
+				for _, dspa := range dspaList.Items {
+					namespacedName := types.NamespacedName{
+						Name:      dspa.Name,
+						Namespace: thisNamespace,
+					}
+					reconcileRequests = append(reconcileRequests, reconcile.Request{NamespacedName: namespacedName})
+				}
+
+				return reconcileRequests
+			})).
 		// Watch for Pods belonging to DSPA
 		Watches(&source.Kind{Type: &corev1.Pod{}},
 			handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request {