Added support for TLS to MLMD GRPC Server

Signed-off-by: hbelmiro <helber.belmiro@gmail.com>
opendatahub-io · Sep 23, 2024 · 4e4bead · 4e4bead
1 parent b81616c
commit 4e4bead
Show file tree

Hide file tree

Showing 25 changed files with 184 additions and 723 deletions.
diff --git a/config/internal/apiserver/default/deployment.yaml.tmpl b/config/internal/apiserver/default/deployment.yaml.tmpl
@@ -121,6 +121,8 @@ spec:
             {{ if .PodToPodTLS }}
             - name: ML_PIPELINE_TLS_ENABLED
               value: "true"
+            - name: METADATA_TLS_ENABLED
+              value: "true"
             {{ end }}
             {{ if (eq .DSPVersion "v2") }}
             ## Argo-Specific Env Vars ##

diff --git a/...tadata-grpc.ml-pipeline.service.yaml.tmpl → ...tadata-grpc.ml-pipeline.service.yaml.tmpl b/...tadata-grpc.ml-pipeline.service.yaml.tmpl → ...tadata-grpc.ml-pipeline.service.yaml.tmpl
diff --git a/...-metadata/metadata-grpc.service.yaml.tmpl → ...c-service/metadata-grpc.service.yaml.tmpl b/...-metadata/metadata-grpc.service.yaml.tmpl → ...c-service/metadata-grpc.service.yaml.tmpl
@@ -3,9 +3,14 @@ kind: Service
 metadata:
   name: ds-pipeline-metadata-grpc-{{.Name}}
   namespace: {{.Namespace}}
+  {{ if .PodToPodTLS }}
+  annotations:
+    service.beta.openshift.io/serving-cert-secret-name: ds-pipeline-metadata-grpc-tls-certs-{{.Name}}
+  {{ end }}
   labels:
     app: ds-pipeline-metadata-grpc-{{.Name}}
     component: data-science-pipelines
+    dspa: {{.Name}}
 spec:
   ports:
     - name: grpc-api

diff --git a/config/internal/ml-metadata/metadata-envoy.configmap.yaml.tmpl b/config/internal/ml-metadata/metadata-envoy.configmap.yaml.tmpl
@@ -69,3 +69,13 @@ data:
                           socket_address:
                             address: metadata-grpc-service
                             port_value: 8080
+              {{ if .PodToPodTLS }}
+              transport_socket:
+                name: envoy.transport_sockets.tls
+                typed_config:
+                  "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
+                  common_tls_context:
+                    validation_context:
+                      trusted_ca:
+                        filename: /etc/ssl/certs/dsp-ca.crt
+              {{ end }}
diff --git a/config/internal/ml-metadata/metadata-envoy.deployment.yaml.tmpl b/config/internal/ml-metadata/metadata-envoy.deployment.yaml.tmpl
@@ -71,6 +71,10 @@ spec:
             - mountPath: /etc/envoy.yaml
               name: envoy-config
               subPath: envoy.yaml
+            {{ if .PodToPodTLS }}
+            - name: proxy-tls-upstream
+              mountPath: "/etc/ssl/certs/"
+            {{ end }}
         {{ if .MLMD.Envoy.DeployRoute }}
         - name: oauth-proxy
           args:
@@ -128,3 +132,6 @@ spec:
         - name: proxy-tls
           secret:
             secretName: ds-pipelines-envoy-proxy-tls-{{.Name}}
+        - name: proxy-tls-upstream
+          configMap:
+            name: dsp-trusted-ca-{{.Name}}
diff --git a/config/internal/ml-metadata/metadata-grpc-tls-config-secret.yaml.tmpl b/config/internal/ml-metadata/metadata-grpc-tls-config-secret.yaml.tmpl
@@ -0,0 +1,23 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: ds-pipeline-metadata-grpc-tls-config-secret-{{.Name}}
+  namespace: {{.Namespace}}
+  labels:
+    component: metadata-grpc-server
+stringData:
+  config.proto: |
+    connection_config {
+      mysql {
+        host: "{{.DBConnection.Host}}"
+        port: {{.DBConnection.Port}}
+        database: "{{.DBConnection.DBName}}"
+        user: "{{.DBConnection.Username}}"
+        password: "{{.DBConnection.DecodedPassword}}"
+      }
+    }
+    ssl_config {
+      server_cert: "{{.MlmdGrpcCertificateContents}}"
+      server_key: "{{.MlmdGrpcPrivateKeyContents}}"
+      client_verify: false // controls mTLS, which we don't use, so hardcode to false
+    }
diff --git a/config/internal/ml-metadata/metadata-grpc.deployment.yaml.tmpl b/config/internal/ml-metadata/metadata-grpc.deployment.yaml.tmpl
@@ -30,6 +30,9 @@ spec:
             - --mysql_config_user=$(DBCONFIG_USER)
             - --mysql_config_password=$(DBCONFIG_PASSWORD)
             - --enable_database_upgrade=true
+            {{ if .PodToPodTLS }}
+            - --metadata_store_server_config_file=/mlmd-tls-config/config.proto
+            {{ end }}
             {{ if .CustomCABundle }}
             - --mysql_config_sslrootcert={{ .PiplinesCABundleMountPath }}
             {{ end }}
@@ -90,10 +93,29 @@ spec:
             - mountPath: {{ .CustomCABundleRootMountPath }}
               name: ca-bundle
             {{ end }}
+            {{ if .PodToPodTLS }}
+            - name: ds-pipeline-metadata-grpc-tls-config-{{.Name}}
+              mountPath: /mlmd-tls-config
+            - name: ds-pipeline-metadata-grpc-tls-certs-{{.Name}}
+              mountPath: "/etc/tls"
+            {{ end }}
       serviceAccountName: ds-pipeline-metadata-grpc-{{.Name}}
       volumes:
         {{ if .CustomCABundle }}
         - name: ca-bundle
           configMap:
             name: {{ .CustomCABundle.ConfigMapName }}
         {{ end }}
+        {{ if .PodToPodTLS }}
+        - name: ds-pipeline-metadata-grpc-tls-config-{{.Name}}
+          secret:
+            secretName: ds-pipeline-metadata-grpc-tls-config-secret-{{.Name}}
+        - name: ds-pipeline-metadata-grpc-tls-certs-{{.Name}}
+          secret:
+            secretName: ds-pipeline-metadata-grpc-tls-certs-{{.Name}}
+            items:
+            - key: tls.key
+              path: tls.key
+            - key: tls.crt
+              path: tls.crt
+        {{ end }}
diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go
@@ -304,7 +304,7 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 			return ctrl.Result{}, err
 		}
 
-		err = r.ReconcileMLMD(dspa, params)
+		err = r.ReconcileMLMD(ctx, dspa, params)
 		if err != nil {
 			r.setStatusAsNotReady(config.MLMDProxyReady, err, dspaStatus.SetMLMDProxyStatus)
 			return ctrl.Result{}, err
@@ -635,6 +635,47 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 				return []reconcile.Request{{NamespacedName: namespacedName}}
 			}),
 		).
+		WatchesRawSource(source.Kind(mgr.GetCache(), &corev1.Secret{}),
+			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request {
+				secret := o.(*corev1.Secret)
+				log := r.Log.WithValues("namespace", secret.Namespace)
+
+				if secret.Annotations["openshift.io/owning-component"] != "service-ca" {
+					return nil
+				}
+
+				log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change on Secret owned by service-ca: %s", secret.Name))
+
+				serviceName := secret.Annotations["service.beta.openshift.io/originating-service-name"]
+
+				namespacedServiceName := types.NamespacedName{
+					Name:      serviceName,
+					Namespace: secret.Namespace,
+				}
+
+				service := &corev1.Service{}
+
+				err := r.Get(ctx, namespacedServiceName, service)
+				if err != nil {
+					return nil
+				}
+
+				dspaName, hasDSPALabel := service.Labels["dspa"]
+				if !hasDSPALabel {
+					msg := fmt.Sprintf("Service is missing dspa "+
+						"label, could not reconcile on [Service: %s] ", serviceName)
+					log.V(1).Info(msg)
+					return nil
+				}
+
+				log.V(1).Info(fmt.Sprintf("Reconcile event triggered by [Service: %s] ", serviceName))
+				namespacedDspaName := types.NamespacedName{
+					Name:      dspaName,
+					Namespace: secret.Namespace,
+				}
+				return []reconcile.Request{{NamespacedName: namespacedDspaName}}
+			}),
+		).
 		WithOptions(controller.Options{
 			MaxConcurrentReconciles: r.MaxConcurrentReconciles,
 		}).

diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go
@@ -66,6 +66,8 @@ type DSPAParams struct {
 	Minio                                *dspa.Minio
 	MLMD                                 *dspa.MLMD
 	MlmdProxyDefaultResourceName         string
+	MlmdGrpcCertificateContents          string
+	MlmdGrpcPrivateKeyContents           string
 	WorkflowController                   *dspa.WorkflowController
 	CustomKfpLauncherConfigMapData       string
 	DBConnection
@@ -101,6 +103,7 @@ type DBConnection struct {
 	DBName            string
 	CredentialsSecret *dspa.SecretKeyValue
 	Password          string
+	DecodedPassword   string
 	ExtraParams       string
 }
 type ObjectStorageConnection struct {
@@ -290,6 +293,8 @@ func (p *DSPAParams) SetupDBParams(ctx context.Context, dsp *dspa.DataSciencePip
 			return err
 		}
 		p.DBConnection.Password = password
+		decodedPasswordBytes, _ := base64.StdEncoding.DecodeString(password)
+		p.DBConnection.DecodedPassword = string(decodedPasswordBytes)
 	} else {
 		// If no externalDB or mariaDB is specified, DSPO assumes
 		// MariaDB deployment with defaults.
@@ -349,6 +354,8 @@ func (p *DSPAParams) SetupDBParams(ctx context.Context, dsp *dspa.DataSciencePip
 			return err
 		}
 		p.DBConnection.Password = dbPassword
+		decodedPasswordBytes, _ := base64.StdEncoding.DecodeString(dbPassword)
+		p.DBConnection.DecodedPassword = string(decodedPasswordBytes)
 	}
 
 	// User specified custom Extra parameters will always take precedence
@@ -582,6 +589,20 @@ func setResourcesDefault(defaultValue dspa.ResourceRequirements, value **dspa.Re
 	}
 }
 
+func (p *DSPAParams) LoadMlmdCertificates(ctx context.Context, client client.Client) (bool, error) {
+	secret, err := util.GetSecret(ctx, "ds-pipeline-metadata-grpc-tls-certs-"+p.Name, p.Namespace, client)
+	if err != nil {
+		if apierrs.IsNotFound(err) {
+			return false, nil
+		} else {
+			return false, err
+		}
+	}
+	p.MlmdGrpcCertificateContents = strings.ReplaceAll(string(secret.Data["tls.crt"]), "\n", "\\n")
+	p.MlmdGrpcPrivateKeyContents = strings.ReplaceAll(string(secret.Data["tls.key"]), "\n", "\\n")
+	return true, nil
+}
+
 func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePipelinesApplication, client client.Client, loggr logr.Logger) error {
 	p.Name = dsp.Name
 	p.Namespace = dsp.Namespace

diff --git a/controllers/mlmd.go b/controllers/mlmd.go
@@ -16,16 +16,19 @@ limitations under the License.
 package controllers
 
 import (
+	"context"
+	"errors"
 	dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1"
 )
 
 const (
 	mlmdTemplatesDir                   = "ml-metadata"
 	mlmdEnvoyRoute                     = mlmdTemplatesDir + "/route/metadata-envoy.route.yaml.tmpl"
 	mlmdProxyDefaultResourceNamePrefix = "ds-pipeline-scheduledworkflow-"
+	mlmdGrpcService                    = "grpc-service"
 )
 
-func (r *DSPAReconciler) ReconcileMLMD(dsp *dspav1alpha1.DataSciencePipelinesApplication,
+func (r *DSPAReconciler) ReconcileMLMD(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication,
 	params *DSPAParams) error {
 
 	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
@@ -57,7 +60,25 @@ func (r *DSPAReconciler) ReconcileMLMD(dsp *dspav1alpha1.DataSciencePipelinesApp
 			return err
 		}
 	} else {
-		err := r.ApplyDir(dsp, params, mlmdTemplatesDir)
+		// We need to create the service first so OpenShift creates the certificate that we'll use later.
+		err := r.ApplyDir(dsp, params, mlmdTemplatesDir+"/"+mlmdGrpcService)
+		if err != nil {
+			return err
+		}
+
+		if params.PodToPodTLS {
+			var certificatesExist bool
+			certificatesExist, err = params.LoadMlmdCertificates(ctx, r.Client)
+			if err != nil {
+				return err
+			}
+
+			if !certificatesExist {
+				return errors.New("secret containing the certificate for MLMD gRPC Server was not created yet")
+			}
+		}
+
+		err = r.ApplyDir(dsp, params, mlmdTemplatesDir)
 		if err != nil {
 			return err
 		}

diff --git a/controllers/mlmd_test.go b/controllers/mlmd_test.go
@@ -94,7 +94,7 @@ func TestDeployMLMDV1(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists
@@ -189,7 +189,7 @@ func TestDeployMLMDV2(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists
@@ -283,7 +283,7 @@ func TestDontDeployMLMDV1(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources still doesn't exist
@@ -413,7 +413,7 @@ func TestDefaultDeployBehaviorMLMDV1(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources still doesn't exist
@@ -505,7 +505,7 @@ func TestDefaultDeployBehaviorMLMDV2(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists
@@ -588,7 +588,7 @@ func TestDeployEnvoyRouteV1(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists
@@ -660,7 +660,7 @@ func TestDeployEnvoyRouteV2(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists
@@ -731,7 +731,7 @@ func TestDontDeployEnvoyRouteV1(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists
@@ -803,7 +803,7 @@ func TestDontDeployEnvoyRouteV2(t *testing.T) {
 	assert.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	assert.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists
@@ -880,7 +880,7 @@ func TestGetEndpointsMLMDV2(t *testing.T) {
 	require.Nil(t, err)
 
 	// Run test reconciliation
-	err = reconciler.ReconcileMLMD(dspa, params)
+	err = reconciler.ReconcileMLMD(ctx, dspa, params)
 	require.Nil(t, err)
 
 	// Ensure MLMD-Envoy resources now exists

diff --git a/controllers/testdata/declarative/case_5/expected/created/metadata-envoy_deployment.yaml b/controllers/testdata/declarative/case_5/expected/created/metadata-envoy_deployment.yaml
@@ -117,3 +117,7 @@ spec:
           secret:
             secretName: ds-pipelines-envoy-proxy-tls-testdsp5
             defaultMode: 420
+        - name: proxy-tls-upstream
+          configMap:
+            name: dsp-trusted-ca-testdsp5
+            defaultMode: 420