Update CA Cert Conditional #12572

alyssacgoins · 2025-12-22T14:34:01Z

Description of your changes:
Update the CA cert logic on driver/launcher so that if CABUNDLE_SECRET_NAME is set, even if pod-to-pod TLS is not enabled, the cert will be passed into the launcher.

backend/test/compiler/argo_ginkgo_test.go is updated to fix env var testing, in order to validate the change above.

Checklist:

You have signed off your commits
The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

google-oss-prow · 2025-12-22T14:34:05Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

mprahl · 2025-12-22T19:04:52Z

backend/src/v2/compiler/argocompiler/container.go


 	setCABundle := false
-	if common.GetCaBundleSecretName() != "" && (c.mlPipelineTLSEnabled || common.GetMetadataTLSEnabled()) {
+	if common.GetCaBundleSecretName() != "" || c.mlPipelineTLSEnabled || common.GetMetadataTLSEnabled() {


I think we still need to guarantee that the secret name is set.

Suggested change

if common.GetCaBundleSecretName() != "" || c.mlPipelineTLSEnabled || common.GetMetadataTLSEnabled() {

if common.GetCaBundleSecretName() != "" {

mprahl · 2025-12-22T19:05:02Z

backend/src/v2/compiler/argocompiler/container.go

 	}
 	// If the apiserver is TLS-enabled, add the custom CA bundle to the executor.
-	if common.GetCaBundleSecretName() != "" && (c.mlPipelineTLSEnabled || common.GetMetadataTLSEnabled()) {
+	if common.GetCaBundleSecretName() != "" || c.mlPipelineTLSEnabled || common.GetMetadataTLSEnabled() {


I think we still need to guarantee that the secret name is set.

Suggested change

if common.GetCaBundleSecretName() != "" || c.mlPipelineTLSEnabled || common.GetMetadataTLSEnabled() {

if common.GetCaBundleSecretName() != "" {

hbelmiro

We should do the same in backend/src/v2/compiler/argocompiler/dag.go and backend/src/v2/compiler/argocompiler/importer.go.

mprahl · 2026-01-05T16:16:39Z

backend/src/v2/compiler/argocompiler/common.go

+		volumeSource.Secret = &k8score.SecretVolumeSource{SecretName: caBundleSecretName}
+	} else if caBundleConfigMapName != "" {
+		caBundleConfigMapKey := common.GetCaBundleConfigMapKey()
+		if caBundleConfigMapKey == "" {


I think we should align the behavior between Secret and ConfigMap such that:

Prefer Secret over ConfigMap (like your code)

We have one configuration option of CABUNDLE_KEY_NAME that applies to both Secret and ConfigMap.

If CABUNDLE_KEY_NAME is not set, then don't set SubPath on either the Secret or ConfigMap (matches existing Secret behavior).

mprahl · 2026-01-05T18:40:36Z

backend/src/apiserver/common/config.go

 	return GetStringConfigWithDefault(CaBundleSecretName, "")
 }

+func GetCaBundleConfigMapKey() string {


Suggested change

func GetCaBundleConfigMapKey() string {

func GetCABundleKey() string {

mprahl · 2026-01-05T19:54:00Z

backend/src/v2/compiler/argocompiler/common.go

+	caBundleKeyName := common.GetCABundleKey()
+	volumeMount := k8score.VolumeMount{Name: "custom-ca", MountPath: common.CABundleDir}
+	if caBundleKeyName != "" {
+		volumeMount.SubPath = caBundleKeyName


I don't think this logic is quite right. I think we want to do something like this instead where the key can change but the mounted path is always the same. Consider using a constant variable for ca.crt.

diff --git a/backend/src/v2/compiler/argocompiler/common.go b/backend/src/v2/compiler/argocompiler/common.go index 8e4c8e081..6112679e3 100644 --- a/backend/src/v2/compiler/argocompiler/common.go +++ b/backend/src/v2/compiler/argocompiler/common.go @@ -58,12 +58,32 @@ func ConfigureCustomCABundle(tmpl *wfapi.Template) { caBundleSecretName := common.GetCaBundleSecretName() caBundleConfigMapName := common.GetCaBundleConfigMapName() volumeSource := k8score.VolumeSource{} + caBundleKeyName := common.GetCABundleKey() + if caBundleKeyName == "" { + caBundleKeyName = "ca.crt" + } // CABUNDLE_SECRET_NAME is prioritized above CABUNDLE_CONFIGMAP_NAME. if caBundleSecretName != "" { // nolint:gocritic // ifElseChain is preferred here for clarity over a switch - volumeSource.Secret = &k8score.SecretVolumeSource{SecretName: caBundleSecretName} + volumeSource.Secret = &k8score.SecretVolumeSource{ + SecretName: caBundleSecretName, + Items: []k8score.KeyToPath{ + { + Key: caBundleKeyName, + Path: "ca.crt", + }, + }, + } } else if caBundleConfigMapName != "" { - volumeSource.ConfigMap = &k8score.ConfigMapVolumeSource{LocalObjectReference: k8score.LocalObjectReference{Name: caBundleConfigMapName}} + volumeSource.ConfigMap = &k8score.ConfigMapVolumeSource{ + LocalObjectReference: k8score.LocalObjectReference{Name: caBundleConfigMapName}, + Items: []k8score.KeyToPath{ + { + Key: caBundleKeyName, + Path: "ca.crt", + }, + }, + } } else { glog.Error("Neither CABUNDLE_SECRET_NAME nor CABUNDLE_CONFIGMAP_NAME is set. Failed to configure custom CA bundle.") return @@ -75,13 +95,8 @@ func ConfigureCustomCABundle(tmpl *wfapi.Template) { tmpl.Volumes = append(tmpl.Volumes, volume) // If CABUNDLE_KEY_NAME is set, set its value to the VolumeMount subpath. - caBundleKeyName := common.GetCABundleKey() volumeMount := k8score.VolumeMount{Name: "custom-ca", MountPath: common.CABundleDir} - if caBundleKeyName != "" { - volumeMount.SubPath = caBundleKeyName - } tmpl.Container.VolumeMounts = append(tmpl.Container.VolumeMounts, volumeMount) - } // addExitTask adds an exit lifecycle hook to a task if exitTemplate is not empty.

Signed-off-by: alyssacgoins <agoins@redhat.com>

mprahl

/approve
/lgtm

google-oss-prow · 2026-01-05T21:03:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mprahl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mprahl]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

nsingla · 2026-01-05T21:17:56Z

backend/test/compiler/argo_ginkgo_test.go

+		{
+			compilerOptions:   argocompiler.Options{CacheDisabled: true},
+			envVars:           map[string]string{"PIPELINE_RUN_AS_USER": "1001", "PIPELINE_LOG_LEVEL": "3"},
+			pipelineFilePaths: []string{filepath.Join(pipelineFilesRootDir, pipelineDirectory, "run_as_user_cache_disabled.yaml")},


I am confused about this, why do you need a separate yaml? can the existing pipelines not run as a user and with a different log level? if so, we can always set the required values in the expected compiled workflow object

A separate YAML is necessary because the environment variables update the contents of the YAML - for example, the variable PIPELINE_RUN_AS_USER adds securityContext: runAsUser: 1001 to the compiled workflow. But we can't update the general YAML with this specialized case, because then the test cases with no env variables set will fail.

in that case, we can check if we need to update or not based on the environment variable

nsingla · 2026-01-05T21:18:03Z

backend/test/compiler/argo_ginkgo_test.go

-			envVars:         map[string]string{"PIPELINE_RUN_AS_USER": "1001", "PIPELINE_LOG_LEVEL": "3"},
+			compilerOptions:   argocompiler.Options{CacheDisabled: false},
+			envVars:           map[string]string{"PIPELINE_RUN_AS_USER": "1001", "PIPELINE_LOG_LEVEL": "3"},
+			pipelineFilePaths: []string{filepath.Join(pipelineFilesRootDir, pipelineDirectory, "run_as_user_cache_enabled.yaml")},


Addressed above ^

google-oss-prow bot added the do-not-merge/work-in-progress label Dec 22, 2025

google-oss-prow bot requested review from rimolive and zazulam December 22, 2025 14:34

google-oss-prow bot added the size/XS label Dec 22, 2025

alyssacgoins force-pushed the ca_cert_conditional branch 2 times, most recently from a3ef196 to 930b21b Compare December 22, 2025 17:05

alyssacgoins marked this pull request as ready for review December 22, 2025 17:10

google-oss-prow bot removed the do-not-merge/work-in-progress label Dec 22, 2025

google-oss-prow bot requested a review from hbelmiro December 22, 2025 17:10

mprahl reviewed Dec 22, 2025

View reviewed changes

hbelmiro reviewed Dec 26, 2025

View reviewed changes

alyssacgoins force-pushed the ca_cert_conditional branch from 3827a61 to f17feda Compare January 2, 2026 20:57

google-oss-prow bot added size/L and removed size/XS labels Jan 2, 2026

alyssacgoins force-pushed the ca_cert_conditional branch from f17feda to 1f9e91d Compare January 5, 2026 15:31

google-oss-prow bot added size/XL and removed size/L labels Jan 5, 2026

mprahl reviewed Jan 5, 2026

View reviewed changes

alyssacgoins force-pushed the ca_cert_conditional branch from 1f9e91d to 0a96f60 Compare January 5, 2026 16:46

google-oss-prow bot added size/XXL and removed size/XL labels Jan 5, 2026

alyssacgoins force-pushed the ca_cert_conditional branch 3 times, most recently from 1beaca0 to 90a3f40 Compare January 5, 2026 18:32

mprahl reviewed Jan 5, 2026

View reviewed changes

alyssacgoins force-pushed the ca_cert_conditional branch from 90a3f40 to d1f64ae Compare January 5, 2026 19:02

mprahl reviewed Jan 5, 2026

View reviewed changes

Update CA Cert Conditional

a5a226e

Signed-off-by: alyssacgoins <agoins@redhat.com>

alyssacgoins force-pushed the ca_cert_conditional branch from d1f64ae to a5a226e Compare January 5, 2026 20:17

mprahl approved these changes Jan 5, 2026

View reviewed changes

google-oss-prow bot assigned mprahl Jan 5, 2026

google-oss-prow bot added the lgtm label Jan 5, 2026

google-oss-prow bot added the approved label Jan 5, 2026

nsingla reviewed Jan 5, 2026

View reviewed changes

google-oss-prow bot merged commit 460002b into kubeflow:master Jan 5, 2026
178 of 181 checks passed

	if common.GetCaBundleSecretName() != "" \|\| c.mlPipelineTLSEnabled \|\| common.GetMetadataTLSEnabled() {
	if common.GetCaBundleSecretName() != "" {

	func GetCaBundleConfigMapKey() string {
	func GetCABundleKey() string {

Update CA Cert Conditional #12572

Update CA Cert Conditional #12572

Uh oh!

Conversation

alyssacgoins commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-oss-prow bot commented Dec 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hbelmiro left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mprahl left a comment

Choose a reason for hiding this comment

Uh oh!

google-oss-prow bot commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alyssacgoins commented Dec 22, 2025 •

edited

Loading