UPSTREAM: <carry>: Enforce secure MLflow TLS with required caBundlePath

VaniHaripriya · VaniHaripriya · commit cbf6433dd6b6 · 2026-06-25T14:36:52.000-05:00
Signed-off-by: Vani Haripriya Mudadla &lt;vmudadla@redhat.com&gt;
diff --git a/.github/resources/scripts/configure-mlflow.sh b/.github/resources/scripts/configure-mlflow.sh
@@ -29,6 +29,10 @@ KFP_NAMESPACE="${1:?KFP namespace required}"
 MLFLOW_NAMESPACE="${2:?MLflow namespace required}"
 CONFIG_JSON_PATH="${3:?Path to source config.json required}"
 
+MLFLOW_CA_CONFIGMAP="mlflow-ca-cert"
+KFP_CA_BUNDLE_DIR="/kfp/certs"
+KFP_CA_BUNDLE_PATH="${KFP_CA_BUNDLE_DIR}/ca.crt"
+
 echo "Services in ${MLFLOW_NAMESPACE} namespace:"
 kubectl get svc -n "$MLFLOW_NAMESPACE" --no-headers
 MLFLOW_SVC=$(kubectl get svc -n "$MLFLOW_NAMESPACE" --no-headers -o custom-columns=":metadata.name" | grep -i mlflow | head -1)
@@ -43,7 +47,6 @@ MLFLOW_ENDPOINT="https://${MLFLOW_HOST}:${MLFLOW_PORT}${MLFLOW_STATIC_PREFIX}"
 echo "MLflow service: $MLFLOW_SVC port=$MLFLOW_PORT endpoint=$MLFLOW_ENDPOINT"
 
 # --- Extract CA certificate from the MLflow TLS secret ---
-CA_MOUNT_PATH="/etc/mlflow/tls/ca.crt"
 CA_CERT_FILE="/tmp/mlflow-ca.crt"
 
 echo "Extracting MLflow CA certificate..."
@@ -70,71 +73,12 @@ fi
 echo "$CA_DATA" | base64 -d > "$CA_CERT_FILE"
 echo "CA certificate extracted to $CA_CERT_FILE ($(wc -l < "$CA_CERT_FILE") lines)"
 
-# Create a ConfigMap with the CA cert in the KFP namespace for workflow pods
-kubectl create configmap mlflow-ca-cert -n "$KFP_NAMESPACE" \
+# ConfigMap consumed by the API server (plugin config) and compiler (driver/launcher pods).
+kubectl create configmap "$MLFLOW_CA_CONFIGMAP" -n "$KFP_NAMESPACE" \
   --from-file=ca.crt="$CA_CERT_FILE" --dry-run=client -o yaml | kubectl apply -f -
 
-# --- Patch Argo workflow-controller-configmap to mount the CA cert into all workflow pods ---
-echo "Patching workflow-controller-configmap to inject MLflow CA into workflow pods..."
-
-EXISTING_MAIN_CONTAINER=$(kubectl get configmap workflow-controller-configmap -n "$KFP_NAMESPACE" \
-  -o jsonpath='{.data.mainContainer}' 2>/dev/null || echo "")
-if [ -n "$EXISTING_MAIN_CONTAINER" ]; then
-  MAIN_CONTAINER_PATCH=$(echo "$EXISTING_MAIN_CONTAINER" | \
-    python3 -c "
-import sys, json
-raw = sys.stdin.read().strip()
-try:
-    cfg = json.loads(raw)
-except json.JSONDecodeError:
-    import yaml
-    cfg = yaml.safe_load(raw) or {}
-vms = cfg.get('volumeMounts', [])
-vms = [vm for vm in vms if vm.get('name') != 'mlflow-ca']
-vms.append({'name': 'mlflow-ca', 'mountPath': '/etc/mlflow/tls', 'readOnly': True})
-cfg['volumeMounts'] = vms
-print(json.dumps(cfg))
-")
-else
-  MAIN_CONTAINER_PATCH='{"volumeMounts":[{"name":"mlflow-ca","mountPath":"/etc/mlflow/tls","readOnly":true}]}'
-fi
-
-EXISTING_WF_DEFAULTS=$(kubectl get configmap workflow-controller-configmap -n "$KFP_NAMESPACE" \
-  -o jsonpath='{.data.workflowDefaults}' 2>/dev/null || echo "")
-if [ -n "$EXISTING_WF_DEFAULTS" ]; then
-  WF_DEFAULTS_PATCH=$(echo "$EXISTING_WF_DEFAULTS" | \
-    python3 -c "
-import sys, yaml
-cfg = yaml.safe_load(sys.stdin) or {}
-spec = cfg.setdefault('spec', {})
-volumes = spec.setdefault('volumes', [])
-volumes = [v for v in volumes if v.get('name') != 'mlflow-ca']
-volumes.append({'name': 'mlflow-ca', 'configMap': {'name': 'mlflow-ca-cert'}})
-spec['volumes'] = volumes
-cfg['spec'] = spec
-print(yaml.dump(cfg, default_flow_style=False))
-")
-else
-  WF_DEFAULTS_PATCH=$(cat <<'YAML'
-spec:
-  volumes:
-  - name: mlflow-ca
-    configMap:
-      name: mlflow-ca-cert
-YAML
-)
-fi
-
-kubectl patch configmap workflow-controller-configmap -n "$KFP_NAMESPACE" --type=merge \
-  -p "$(jq -n --arg mc "$MAIN_CONTAINER_PATCH" --arg wd "$WF_DEFAULTS_PATCH" \
-    '{"data":{"mainContainer":$mc,"workflowDefaults":$wd}}')"
-
-# Restart workflow controller to pick up configmap changes
-kubectl rollout restart deployment/workflow-controller -n "$KFP_NAMESPACE"
-kubectl rollout status deployment/workflow-controller -n "$KFP_NAMESPACE" --timeout=120s
-
 # --- Build the MLflow plugin config with caBundlePath ---
-MLFLOW_PATCH=$(jq -n --arg endpoint "$MLFLOW_ENDPOINT" --arg caBundlePath "$CA_MOUNT_PATH" '{
+MLFLOW_PATCH=$(jq -n --arg endpoint "$MLFLOW_ENDPOINT" --arg caBundlePath "$KFP_CA_BUNDLE_PATH" '{
   endpoint: $endpoint,
   tls: { caBundlePath: $caBundlePath },
   settings: { workspacesEnabled: true }
@@ -146,11 +90,11 @@ jq --argjson mlflow "$MLFLOW_PATCH" '. + { plugins: { mlflow: $mlflow } }' \
 echo "Patched config.json plugins.mlflow:"
 jq '.plugins.mlflow' /tmp/kfp-config.json
 
-# --- Deploy the config and mount the CA cert into the API server ---
+# --- Deploy plugin config and wire CA trust on the API server + compiler ---
 kubectl create configmap kfp-mlflow-config -n "$KFP_NAMESPACE" \
   --from-file=config.json=/tmp/kfp-config.json --dry-run=client -o yaml | kubectl apply -f -
 kubectl patch deployment ml-pipeline -n "$KFP_NAMESPACE" --type=strategic -p \
-  '{"spec":{"template":{"spec":{"volumes":[{"name":"mlflow-cfg","configMap":{"name":"kfp-mlflow-config"}},{"name":"mlflow-ca","configMap":{"name":"mlflow-ca-cert"}}],"containers":[{"name":"ml-pipeline-api-server","volumeMounts":[{"name":"mlflow-cfg","mountPath":"/config/config.json","subPath":"config.json"},{"name":"mlflow-ca","mountPath":"/etc/mlflow/tls","readOnly":true}]}]}}}}'
+  '{"spec":{"template":{"spec":{"volumes":[{"name":"mlflow-cfg","configMap":{"name":"kfp-mlflow-config"}},{"name":"mlflow-ca","configMap":{"name":"'"$MLFLOW_CA_CONFIGMAP"'"}}],"containers":[{"name":"ml-pipeline-api-server","env":[{"name":"CABUNDLE_CONFIGMAP_NAME","value":"'"$MLFLOW_CA_CONFIGMAP"'"}],"volumeMounts":[{"name":"mlflow-cfg","mountPath":"/config/config.json","subPath":"config.json"},{"name":"mlflow-ca","mountPath":"'"$KFP_CA_BUNDLE_DIR"'","readOnly":true}]}]}}}}'
 kubectl rollout status deployment/ml-pipeline -n "$KFP_NAMESPACE" --timeout=180s
 
 pkill -f "kubectl port-forward.*ml-pipeline.*8888" || true
diff --git a/backend/src/v2/common/plugins/mlflow/config.go b/backend/src/v2/common/plugins/mlflow/config.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"strings"
 
-	commonplugins "github.com/kubeflow/pipelines/backend/src/common/plugins"
 	commonmlflow "github.com/kubeflow/pipelines/backend/src/common/plugins/mlflow"
 	"github.com/spf13/viper"
 )
@@ -59,22 +58,11 @@ func ParseKfpMLflowRuntimeConfig() (*commonmlflow.MLflowRuntimeConfig, error) {
 	if cfg.AuthType != "kubernetes" {
 		return nil, fmt.Errorf("unsupported auth type: %s", cfg.AuthType)
 	}
-	// Disabling TLS verification is not supported in the driver/launcher
-	// to prevent CWE-295 (improper certificate validation).
-	if cfg.InsecureSkipVerify {
+	if cfg.InsecureSkipVerify || (cfg.TLS != nil && cfg.TLS.InsecureSkipVerify) {
 		return nil, fmt.Errorf("insecureSkipVerify is not supported")
 	}
-	// Preserve the CABundlePath from the runtime config (propagated by the
-	// API server) so the driver/launcher can verify certificates signed by
-	// an internal CA (e.g., cert-manager). InsecureSkipVerify is always
-	// forced to false.
-	caBundlePath := ""
-	if cfg.TLS != nil {
-		caBundlePath = cfg.TLS.CABundlePath
-	}
-	cfg.TLS = &commonplugins.TLSConfig{
-		InsecureSkipVerify: false,
-		CABundlePath:       caBundlePath,
+	if cfg.TLS == nil || cfg.TLS.CABundlePath == "" {
+		return nil, fmt.Errorf("tls.caBundlePath is required")
 	}
 	return &cfg, nil
 }
diff --git a/backend/src/v2/common/plugins/mlflow/config_test.go b/backend/src/v2/common/plugins/mlflow/config_test.go
@@ -27,13 +27,18 @@ func TestExecutionStateToMLflowTerminalStatus(t *testing.T) {
 	assert.Equal(t, "FAILED", ExecutionStateToMLflowTerminalStatus("UNKNOWN"))
 }
 
+const testCABundlePath = "/etc/pki/tls/certs/ca-bundle.crt"
+
 func TestParseKfpMLflowRuntimeConfig_Success(t *testing.T) {
 	cfg := commonmlflow.MLflowRuntimeConfig{
 		Endpoint:     "http://localhost",
 		ParentRunID:  "test-parent-run-id",
 		ExperimentID: "test-exp",
 		AuthType:     "kubernetes",
 		Timeout:      "10s",
+		TLS: &commonplugins.TLSConfig{
+			CABundlePath: testCABundlePath,
+		},
 	}
 	expectedCfg := &commonmlflow.MLflowRuntimeConfig{
 		Endpoint:           "http://localhost",
@@ -46,7 +51,7 @@ func TestParseKfpMLflowRuntimeConfig_Success(t *testing.T) {
 		InsecureSkipVerify: false,
 		InjectUserEnvVars:  false,
 		TLS: &commonplugins.TLSConfig{
-			InsecureSkipVerify: false,
+			CABundlePath: testCABundlePath,
 		},
 	}
 
@@ -171,6 +176,39 @@ func TestParseKfpMLflowRuntimeConfig_InsecureSkipVerify_Failure(t *testing.T) {
 	assert.Equal(t, "insecureSkipVerify is not supported", err.Error())
 }
 
+func TestParseKfpMLflowRuntimeConfig_MissingTLS_Failure(t *testing.T) {
+	cfg := commonmlflow.MLflowRuntimeConfig{
+		Endpoint:     "http://localhost",
+		ParentRunID:  "test-parent-run-id",
+		ExperimentID: "test-exp",
+		AuthType:     "kubernetes",
+		Timeout:      "10s",
+	}
+	setRuntimeCfg(cfg)
+	runtimeCfg, err := ParseKfpMLflowRuntimeConfig()
+
+	assert.Nil(t, runtimeCfg)
+	assert.Error(t, err)
+	assert.Equal(t, "tls.caBundlePath is required", err.Error())
+}
+
+func TestParseKfpMLflowRuntimeConfig_EmptyCABundlePath_Failure(t *testing.T) {
+	cfg := commonmlflow.MLflowRuntimeConfig{
+		Endpoint:     "http://localhost",
+		ParentRunID:  "test-parent-run-id",
+		ExperimentID: "test-exp",
+		AuthType:     "kubernetes",
+		Timeout:      "10s",
+		TLS:          &commonplugins.TLSConfig{},
+	}
+	setRuntimeCfg(cfg)
+	runtimeCfg, err := ParseKfpMLflowRuntimeConfig()
+
+	assert.Nil(t, runtimeCfg)
+	assert.Error(t, err)
+	assert.Equal(t, "tls.caBundlePath is required", err.Error())
+}
+
 func TestParseKfpMLflowRuntimeConfig_CABundlePath_Preserved(t *testing.T) {
 	cfg := commonmlflow.MLflowRuntimeConfig{
 		Endpoint:     "http://localhost",
diff --git a/backend/src/v2/common/plugins/mlflow/factory_test.go b/backend/src/v2/common/plugins/mlflow/factory_test.go
@@ -3,6 +3,7 @@ package mlflow
 import (
 	"testing"
 
+	commonplugins "github.com/kubeflow/pipelines/backend/src/common/plugins"
 	commonmlflow "github.com/kubeflow/pipelines/backend/src/common/plugins/mlflow"
 	"github.com/kubeflow/pipelines/backend/src/v2/common/plugins"
 	"github.com/spf13/viper"
@@ -43,6 +44,9 @@ func TestMlflowHandlerFactory_Create_Success(t *testing.T) {
 		ExperimentID: "exp-1",
 		AuthType:     "kubernetes",
 		Timeout:      "10s",
+		TLS: &commonplugins.TLSConfig{
+			CABundlePath: testCABundlePath,
+		},
 	})
 	t.Cleanup(func() { viper.Set(commonmlflow.EnvMLflowConfig, "") })
 
@@ -65,6 +69,24 @@ func TestMlflowHandlerFactory_Create_MissingConfig(t *testing.T) {
 	assert.Contains(t, err.Error(), "KFP_MLFLOW_CONFIG env var not set")
 }
 
+func TestMlflowHandlerFactory_Create_MissingCABundlePath(t *testing.T) {
+	setRuntimeCfg(commonmlflow.MLflowRuntimeConfig{
+		Endpoint:     "http://localhost",
+		ParentRunID:  "parent-run-1",
+		ExperimentID: "exp-1",
+		AuthType:     "kubernetes",
+		Timeout:      "10s",
+	})
+	t.Cleanup(func() { viper.Set(commonmlflow.EnvMLflowConfig, "") })
+
+	factory := &mlflowHandlerFactory{}
+	handler, err := factory.Create()
+
+	require.Error(t, err)
+	assert.Nil(t, handler)
+	assert.Contains(t, err.Error(), "tls.caBundlePath is required")
+}
+
 func TestMLflowHandlerFactory_Create_MissingConfigField(t *testing.T) {
 	setRuntimeCfg(commonmlflow.MLflowRuntimeConfig{
 		Endpoint: "http://localhost",
