Merge pull request #656 from HumairAK/RHOAIENG-4968

(feat): add tls to api server

api/v1alpha1/dspipeline_types.go

@@ -44,6 +44,12 @@ type DSPASpec struct {
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default:="v1"
 	DSPVersion string `json:"dspVersion,omitempty"`
+
+	// PodToPodTLS Set to "true" or "false" to enable or disable TLS communication between DSPA components (pods). Defaults to "true" to enable TLS between all pods. Only supported in DSP V2 on OpenShift.
+	// +kubebuilder:default:=true
+	// +kubebuilder:validation:Optional
+	PodToPodTLS *bool `json:"podToPodTLS"`
+
 	// WorkflowController is an argo-specific component that manages a DSPA's Workflow objects and handles the orchestration of them with the central Argo server
 	// +kubebuilder:validation:Optional
 	*WorkflowController `json:"workflowController,omitempty"`

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

@@ -754,6 +754,12 @@ spec:
                         type: object
                     type: object
                 type: object
+              podToPodTLS:
+                default: true
+                description: PodToPodTLS Set to "true" or "false" to enable or disable
+                  TLS communication between DSPA components (pods). Defaults to "true"
+                  to enable TLS between all pods. Only supported in DSP V2 on OpenShift.
+                type: boolean
               scheduledWorkflow:
                 default:
                   deploy: true

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

@@ -118,6 +118,10 @@ spec:
               value: "8887"
             - name: SIGNED_URL_EXPIRY_TIME_SECONDS
               value: "{{.APIServer.ArtifactSignedURLExpirySeconds}}"
+            {{ if .PodToPodTLS }}
+            - name: ML_PIPELINE_TLS_ENABLED
+              value: "true"
+            {{ end }}
             {{ if (eq .DSPVersion "v2") }}
             ## Argo-Specific Env Vars ##
             - name: EXECUTIONTYPE
@@ -181,32 +185,32 @@ spec:
             {{ if .APIServer.EnableSamplePipeline }}
             - --sampleconfig=/config/sample_config.json
             {{ end }}
+            {{ if .PodToPodTLS }}
+            - --tlsCertPath=/etc/tls/private/tls.crt
+            - --tlsCertKeyPath=/etc/tls/private/tls.key
+            {{ end }}
           ports:
             - containerPort: 8888
               name: http
             - containerPort: 8887
               name: grpc
           livenessProbe:
-            exec:
-              command:
-                - wget
-                - -q
-                - -S
-                - -O
-                - '-'
-                - http://localhost:8888/apis/v1beta1/healthz
+            httpGet:
+              path: /apis/v1beta1/healthz
+              port: http
+              {{ if .PodToPodTLS }}
+              scheme: HTTPS
+              {{ end }}
             initialDelaySeconds: 3
             periodSeconds: 5
             timeoutSeconds: 2
           readinessProbe:
-            exec:
-              command:
-                - wget
-                - -q
-                - -S
-                - -O
-                - '-'
-                - http://localhost:8888/apis/v1beta1/healthz
+            httpGet:
+              path: /apis/v1beta1/healthz
+              port: http
+              {{ if .PodToPodTLS }}
+              scheme: HTTPS
+              {{ end }}
             initialDelaySeconds: 3
             periodSeconds: 5
             timeoutSeconds: 2
@@ -233,6 +237,10 @@ spec:
             - name: server-config
               mountPath: /config/config.json
               subPath: {{ .APIServer.CustomServerConfig.Key }}
+          {{ if .PodToPodTLS }}
+            - mountPath: /etc/tls/private
+              name: proxy-tls
+          {{ end }}
           {{ if or .APIServer.EnableSamplePipeline .CustomCABundle }}
             {{ if .APIServer.EnableSamplePipeline }}
             - name: sample-config
@@ -252,7 +260,14 @@ spec:
             - --https-address=:8443
             - --provider=openshift
             - --openshift-service-account={{.APIServerDefaultResourceName}}
+            {{ if .PodToPodTLS }}
+            # because we use certs signed by openshift, these certs are not valid for
+            # localhost, thus we have to use the service name
+            - --upstream=https://{{.APIServerServiceDNSName}}:8888
+            - --upstream-ca=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt
+            {{ else }}
             - --upstream=http://localhost:8888
+            {{ end }}
             - --tls-cert=/etc/tls/private/tls.crt
             - --tls-key=/etc/tls/private/tls.key
             - --cookie-secret=SECRET

config/internal/mlpipelines-ui/deployment.yaml.tmpl

@@ -45,9 +45,15 @@ spec:
             - name: ARGO_ARCHIVE_LOGS
               value: "true"
             - name: ML_PIPELINE_SERVICE_HOST
-              value: ds-pipeline-{{.Name}}
+              value: {{.APIServerServiceDNSName}}
             - name: ML_PIPELINE_SERVICE_PORT
               value: '8888'
+            {{ if .PodToPodTLS }}
+            - name: ML_PIPELINE_SERVICE_SCHEME
+              value: 'https'
+            - name: NODE_EXTRA_CA_CERTS
+              value: '/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt'
+            {{ end }}
             - name: METADATA_ENVOY_SERVICE_SERVICE_HOST
               value: ds-pipeline-md-{{.Name}}
             - name: METADATA_ENVOY_SERVICE_SERVICE_PORT

config/internal/persistence-agent/deployment.yaml.tmpl

@@ -40,6 +40,10 @@ spec:
               {{ else }}
               value: PipelineRun
               {{ end }}
+            {{ if .PodToPodTLS }}
+            - name: SSL_CERT_DIR
+              value: "/etc/pki/tls/certs:/var/run/secrets/kubernetes.io/serviceaccount/"
+            {{ end }}
           image: "{{.PersistenceAgent.Image}}"
           imagePullPolicy: IfNotPresent
           name: ds-pipeline-persistenceagent
@@ -48,7 +52,10 @@ spec:
             - "--logtostderr=true"
             - "--ttlSecondsAfterWorkflowFinish=86400"
             - "--numWorker={{.PersistenceAgent.NumWorkers}}"
-            - "--mlPipelineAPIServerName={{.APIServerServiceName}}"
+            - "--mlPipelineAPIServerName={{.APIServerServiceDNSName}}"
+            {{ if .PodToPodTLS }}
+            - "--mlPipelineServiceTLSEnabled=true"
+            {{ end }}
             - "--namespace={{.Namespace}}"
             - "--mlPipelineServiceHttpPort=8888"
             - "--mlPipelineServiceGRPCPort=8887"

controllers/config/defaults.go

@@ -42,6 +42,9 @@ const (
 	CustomDSPTrustedCAConfigMapNamePrefix = "dsp-trusted-ca"
 	CustomDSPTrustedCAConfigMapKey        = "dsp-ca.crt"
 
+	OpenshiftServiceCAConfigMapName = "openshift-service-ca.crt"
+	OpenshiftServiceCAConfigMapKey  = "service-ca.crt"
+
 	DefaultSystemSSLCertFile     = "SSL_CERT_FILE"
 	DefaultSystemSSLCertFilePath = "/etc/pki/tls/certs/ca-bundle.crt" // Fedora/RHEL 6
 

controllers/dspipeline_params.go

@@ -86,6 +86,10 @@ type DSPAParams struct {
 	// pipeline pods
 	CustomCABundle *dspa.CABundle
 	DSPONamespace  string
+	// Use to enable tls communication between component pods.
+	PodToPodTLS bool
+
+	APIServerServiceDNSName string
 }
 
 type DBConnection struct {
@@ -578,6 +582,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 	p.APIServer = dsp.Spec.APIServer.DeepCopy()
 	p.APIServerDefaultResourceName = apiServerDefaultResourceNamePrefix + dsp.Name
 	p.APIServerServiceName = fmt.Sprintf("%s-%s", config.DSPServicePrefix, p.Name)
+	p.APIServerServiceDNSName = fmt.Sprintf("%s.%s.svc.cluster.local", p.APIServerServiceName, p.Namespace)
 	p.ScheduledWorkflow = dsp.Spec.ScheduledWorkflow.DeepCopy()
 	p.ScheduledWorkflowDefaultResourceName = scheduledWorkflowDefaultResourceNamePrefix + dsp.Name
 	p.PersistenceAgent = dsp.Spec.PersistenceAgent.DeepCopy()
@@ -589,8 +594,19 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 	p.MLMD = dsp.Spec.MLMD.DeepCopy()
 	p.CustomCABundleRootMountPath = config.CustomCABundleRootMountPath
 	p.PiplinesCABundleMountPath = config.GetCABundleFileMountPath()
+	p.PodToPodTLS = false
 	dspTrustedCAConfigMapKey := config.CustomDSPTrustedCAConfigMapKey
 
+	// PodToPodTLS is only used in v2 dsp
+	if p.UsingV2Pipelines(dsp) {
+		// by default it's enabled when omitted
+		if dsp.Spec.PodToPodTLS == nil {
+			p.PodToPodTLS = true
+		} else {
+			p.PodToPodTLS = *dsp.Spec.PodToPodTLS
+		}
+	}
+
 	log := loggr.WithValues("namespace", p.Namespace).WithValues("dspa_name", p.Name)
 
 	if p.APIServer != nil {
@@ -633,7 +649,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 		// Track whether the "ca-bundle.crt" configmap key from odh-trusted-ca bundle
 		// was found, this will be used to decide whether we need to account for this
 		// ourselves later or not.
-		odhTrustedCABundleAdded := false
+		wellKnownCABundleAdded := false
 
 		// Check for cert bundle provided by the platform instead of by the DSPA user
 		// If it exists, include this cert for tls verifications
@@ -661,7 +677,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 			// however if a user creates this, they may accidentally leave this out, so we need to account for this
 			_, ok := odhTrustedCABundleConfigMap.Data[config.GlobalODHCaBundleConfigMapSystemBundleKey]
 			if ok {
-				odhTrustedCABundleAdded = true
+				wellKnownCABundleAdded = true
 			}
 		}
 
@@ -683,6 +699,22 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 			}
 		}
 
+		// If PodToPodTLS is enabled, we need to include service-ca ca-bundles to recognize the certs
+		// that are signed by service-ca. These can be accessed via "openshift-service-ca.crt"
+		// configmap.
+		if p.PodToPodTLS {
+			serviceCA, serviceCACfgErr := util.GetConfigMap(ctx, config.OpenshiftServiceCAConfigMapName, p.Namespace, client)
+			if serviceCACfgErr != nil {
+				log.Info(fmt.Sprintf("Encountered error when attempting to fetch ConfigMap: [%s]. Error: %v", config.OpenshiftServiceCAConfigMapName, serviceCA))
+				return serviceCACfgErr
+			}
+			serviceCABundle := util.GetConfigMapValue(config.OpenshiftServiceCAConfigMapKey, serviceCA)
+			if serviceCABundle == "" {
+				return fmt.Errorf("expected key %s from configmap %s not found", config.OpenshiftServiceCAConfigMapKey, config.OpenshiftServiceCAConfigMapName)
+			}
+			p.APICustomPemCerts = append(p.APICustomPemCerts, []byte(serviceCABundle))
+		}
+
 		if p.APIServer.CABundleFileMountPath != "" {
 			p.CustomCABundleRootMountPath = p.APIServer.CABundleFileMountPath
 		}
@@ -706,7 +738,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 			// We need to ensure system certs are always part of this new configmap
 			// We can either source this from odh-trusted-ca-bundle cfgmap if provided,
 			// or fetch one from "config-trusted-cabundle" configmap, which is always present in an ocp ns
-			if !odhTrustedCABundleAdded {
+			if !wellKnownCABundleAdded {
 				certs, sysCertsErr := util.GetSystemCerts()
 				if sysCertsErr != nil {
 					return sysCertsErr

controllers/dspipeline_params_test.go

@@ -173,6 +173,38 @@ func TestExtractParams_CABundle(t *testing.T) {
 			},
 			SSLCertFileEnv: "testdata/tls/dummy-ca-bundle.crt",
 		},
+
+		{
+			msg:                         "pod to pod tls enabled",
+			dsp:                         testutil.CreateDSPAWithAPIServerPodtoPodTlsEnabled(),
+			CustomCABundleRootMountPath: "/dsp-custom-certs",
+			CustomSSLCertDir:            strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"),
+			PiplinesCABundleMountPath:   "/dsp-custom-certs/dsp-ca.crt",
+			APICustomPemCerts:           [][]byte{[]byte("service-ca-contents")},
+			CustomCABundle:              &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"},
+			ConfigMapPreReq: []*v1.ConfigMap{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "openshift-service-ca.crt", Namespace: "testnamespace"},
+					Data:       map[string]string{"service-ca.crt": "service-ca-contents"},
+				},
+			},
+		},
+		{
+			msg:                         "pod to pod tls enabled with sys certs",
+			dsp:                         testutil.CreateDSPAWithAPIServerPodtoPodTlsEnabled(),
+			CustomCABundleRootMountPath: "/dsp-custom-certs",
+			CustomSSLCertDir:            strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"),
+			PiplinesCABundleMountPath:   "/dsp-custom-certs/dsp-ca.crt",
+			APICustomPemCerts:           [][]byte{[]byte("service-ca-contents"), []byte("dummycontent")},
+			CustomCABundle:              &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"},
+			ConfigMapPreReq: []*v1.ConfigMap{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "openshift-service-ca.crt", Namespace: "testnamespace"},
+					Data:       map[string]string{"service-ca.crt": "service-ca-contents"},
+				},
+			},
+			SSLCertFileEnv: "testdata/tls/dummy-ca-bundle.crt",
+		},
 	}
 
 	for _, test := range tt {
@@ -199,19 +231,19 @@ func TestExtractParams_CABundle(t *testing.T) {
 			}
 
 			actualCustomCABundleRootMountPath := actualParams.CustomCABundleRootMountPath
-			assert.Equal(t, actualCustomCABundleRootMountPath, test.CustomCABundleRootMountPath)
+			assert.Equal(t, test.CustomCABundleRootMountPath, actualCustomCABundleRootMountPath)
 
 			actualCustomSSLCertDir := actualParams.CustomSSLCertDir
-			assert.Equal(t, actualCustomSSLCertDir, test.CustomSSLCertDir)
+			assert.Equal(t, test.CustomSSLCertDir, actualCustomSSLCertDir)
 
 			actualPipelinesCABundleMountPath := actualParams.PiplinesCABundleMountPath
-			assert.Equal(t, actualPipelinesCABundleMountPath, test.PiplinesCABundleMountPath)
+			assert.Equal(t, test.PiplinesCABundleMountPath, actualPipelinesCABundleMountPath)
 
 			actualAPICustomPemCerts := actualParams.APICustomPemCerts
-			assert.Equal(t, actualAPICustomPemCerts, test.APICustomPemCerts)
+			assert.Equal(t, test.APICustomPemCerts, actualAPICustomPemCerts)
 
 			actualCustomCABundle := actualParams.CustomCABundle
-			assert.Equal(t, actualCustomCABundle, test.CustomCABundle)
+			assert.Equal(t, test.CustomCABundle, actualCustomCABundle)
 
 			if test.ConfigMapPreReq != nil && len(test.ConfigMapPreReq) > 0 {
 				for _, cfg := range test.ConfigMapPreReq {

controllers/mlmd_test.go

@@ -131,8 +131,9 @@ func TestDeployMLMDV2(t *testing.T) {
 	// Construct DSPA Spec with MLMD Enabled
 	dspa := &dspav1alpha1.DataSciencePipelinesApplication{
 		Spec: dspav1alpha1.DSPASpec{
-			DSPVersion: "v2",
-			APIServer:  &dspav1alpha1.APIServer{},
+			DSPVersion:  "v2",
+			PodToPodTLS: boolPtr(false),
+			APIServer:   &dspav1alpha1.APIServer{},
 			MLMD: &dspav1alpha1.MLMD{
 				Deploy: true,
 			},
@@ -315,8 +316,9 @@ func TestDontDeployMLMDV2(t *testing.T) {
 	// Construct DSPA Spec with MLMD Not Enabled
 	dspa := &dspav1alpha1.DataSciencePipelinesApplication{
 		Spec: dspav1alpha1.DSPASpec{
-			DSPVersion: "v2",
-			APIServer:  &dspav1alpha1.APIServer{},
+			DSPVersion:  "v2",
+			PodToPodTLS: boolPtr(false),
+			APIServer:   &dspav1alpha1.APIServer{},
 			MLMD: &dspav1alpha1.MLMD{
 				Deploy: false,
 			},
@@ -448,8 +450,9 @@ func TestDefaultDeployBehaviorMLMDV2(t *testing.T) {
 	// Construct DSPA Spec with MLMD Spec not defined
 	dspa := &dspav1alpha1.DataSciencePipelinesApplication{
 		Spec: dspav1alpha1.DSPASpec{
-			DSPVersion: "v2",
-			APIServer:  &dspav1alpha1.APIServer{},
+			DSPVersion:  "v2",
+			PodToPodTLS: boolPtr(false),
+			APIServer:   &dspav1alpha1.APIServer{},
 			Database: &dspav1alpha1.Database{
 				DisableHealthCheck: false,
 				MariaDB: &dspav1alpha1.MariaDB{
@@ -608,8 +611,9 @@ func TestDeployEnvoyRouteV2(t *testing.T) {
 	// Construct DSPA Spec with MLMD Enabled
 	dspa := &dspav1alpha1.DataSciencePipelinesApplication{
 		Spec: dspav1alpha1.DSPASpec{
-			DSPVersion: "v2",
-			APIServer:  &dspav1alpha1.APIServer{},
+			DSPVersion:  "v2",
+			PodToPodTLS: boolPtr(false),
+			APIServer:   &dspav1alpha1.APIServer{},
 			MLMD: &dspav1alpha1.MLMD{
 				Deploy: true,
 				Envoy: &dspav1alpha1.Envoy{
@@ -750,8 +754,9 @@ func TestDontDeployEnvoyRouteV2(t *testing.T) {
 	// Construct DSPA Spec with MLMD Enabled
 	dspa := &dspav1alpha1.DataSciencePipelinesApplication{
 		Spec: dspav1alpha1.DSPASpec{
-			DSPVersion: "v2",
-			APIServer:  &dspav1alpha1.APIServer{},
+			DSPVersion:  "v2",
+			PodToPodTLS: boolPtr(false),
+			APIServer:   &dspav1alpha1.APIServer{},
 			MLMD: &dspav1alpha1.MLMD{
 				Deploy: true,
 				Envoy: &dspav1alpha1.Envoy{
@@ -811,3 +816,7 @@ func TestDontDeployEnvoyRouteV2(t *testing.T) {
 	assert.False(t, created)
 	assert.Nil(t, err)
 }
+
+func boolPtr(b bool) *bool {
+	return &b
+}