Make webhook annotations a global configuration

mprahl · mprahl · commit 050ca1f0612b · 2025-06-16T12:33:05.000-04:00
The webhook annotations shouldn't be a DSPA configuration since the
webhook is global to the cluster.

Signed-off-by: mprahl &lt;mprahl@users.noreply.github.com&gt;
diff --git a/Makefile b/Makefile
@@ -121,7 +121,7 @@ vet: ## Run go vet against code.
 
 .PHONY: test
 test: manifests generate fmt vet envtest ## Run tests.
-	export SSL_CERT_FILE=${ROOT_DIR}/controllers/testdata/tls/ca-bundle.crt && KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... --tags=test_all -coverprofile cover.out
+	export SSL_CERT_FILE=${ROOT_DIR}/controllers/testdata/tls/ca-bundle.crt && export DSPO_NAMESPACE=$(DSPO_NAMESPACE) && KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... --tags=test_all -coverprofile cover.out
 
 .PHONY: unittest
 unittest: manifests generate fmt vet envtest ## Run tests.
diff --git a/api/v1/dspipeline_types.go b/api/v1/dspipeline_types.go
@@ -162,10 +162,6 @@ type APIServer struct {
 	// Enable/disable caching in the DSP API server. Default: true
 	// +kubebuilder:default:=true
 	CacheEnabled *bool `json:"cacheEnabled,omitempty"`
-
-	// WebhookAnnotations is a map of annotations to add to the validating and mutating webhooks.
-	// +kubebuilder:validation:Optional
-	WebhookAnnotations map[string]string `json:"webhookAnnotations,omitempty"`
 }
 
 type CABundle struct {
diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go
diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go
@@ -214,10 +214,6 @@ type APIServer struct {
 	// Enable/disable caching in the DSP API server. Default: true
 	// +kubebuilder:default:=true
 	CacheEnabled *bool `json:"cacheEnabled,omitempty"`
-
-	// WebhookAnnotations is a map of annotations to add to the validating and mutating webhooks.
-	// +kubebuilder:validation:Optional
-	WebhookAnnotations map[string]string `json:"webhookAnnotations,omitempty"`
 }
 
 type CABundle struct {
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/base/params.env b/config/base/params.env
@@ -22,3 +22,4 @@ DSPO_APISERVER_INCLUDE_OWNERREFERENCE=true
 MANAGEDPIPELINES="{}"
 PLATFORMVERSION="v0.0.0"
 FIPSENABLED=false
+WEBHOOK_ANNOTATIONS=""
diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml
@@ -241,12 +241,6 @@ spec:
                     description: Toolbox image used for basic container spec runtime
                       operations in managed pipelines.
                     type: string
-                  webhookAnnotations:
-                    additionalProperties:
-                      type: string
-                    description: WebhookAnnotations is a map of annotations to add
-                      to the validating and mutating webhooks.
-                    type: object
                 type: object
               database:
                 default:
@@ -1250,12 +1244,6 @@ spec:
                     description: 'Default: true Deprecated: DSP V1 only, will be removed
                       in the future.'
                     type: boolean
-                  webhookAnnotations:
-                    additionalProperties:
-                      type: string
-                    description: WebhookAnnotations is a map of annotations to add
-                      to the validating and mutating webhooks.
-                    type: object
                 type: object
               database:
                 default:
diff --git a/config/internal/webhook/mutating_webhook.yaml.tmpl b/config/internal/webhook/mutating_webhook.yaml.tmpl
@@ -4,8 +4,8 @@ metadata:
   name: pipelineversions.pipelines.kubeflow.org
   annotations:
     service.beta.openshift.io/inject-cabundle: "true"
-    {{ if .APIServer.WebhookAnnotations }}
-    {{ range $key, $value := .APIServer.WebhookAnnotations }}
+    {{ if .WebhookAnnotations }}
+    {{ range $key, $value := .WebhookAnnotations }}
     {{ printf "%q" $key }}: {{ printf "%q" $value }}
     {{ end }}
     {{end}}
diff --git a/config/internal/webhook/validating_webhook.yaml.tmpl b/config/internal/webhook/validating_webhook.yaml.tmpl
@@ -4,8 +4,8 @@ metadata:
   name: pipelineversions.pipelines.kubeflow.org
   annotations:
     service.beta.openshift.io/inject-cabundle: "true"
-    {{ if .APIServer.WebhookAnnotations }}
-    {{ range $key, $value := .APIServer.WebhookAnnotations }}
+    {{ if .WebhookAnnotations }}
+    {{ range $key, $value := .WebhookAnnotations }}
     {{ printf "%q" $key }}: {{ printf "%q" $value }}
     {{ end }}
     {{end}}
diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -85,6 +85,8 @@ spec:
             value: $(PLATFORMVERSION)
           - name: DSPO_FIPSENABLED
             value: $(FIPSENABLED)
+          - name: WEBHOOK_ANNOTATIONS
+            value: $(WEBHOOK_ANNOTATIONS)
         securityContext:
           allowPrivilegeEscalation: false
           capabilities:
diff --git a/config/overlays/kind-tests/env_patch.yaml b/config/overlays/kind-tests/env_patch.yaml
@@ -0,0 +1,13 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: controller-manager
+  namespace: datasciencepipelinesapplications-controller
+spec:
+  template:
+    spec:
+      containers:
+        - name: manager
+          env:
+            - name: WEBHOOK_ANNOTATIONS
+              value: '{"cert-manager.io/inject-ca-from": "opendatahub/dspa-webhook-cert"}'
diff --git a/config/overlays/kind-tests/kustomization.yaml b/config/overlays/kind-tests/kustomization.yaml
@@ -7,6 +7,7 @@ patchesStrategicMerge:
   - img_patch.yaml
   - res_patch.yaml
   - user_patch.yaml
+  - env_patch.yaml
 images:
   - name: controller
     newName: quay.io/opendatahub/data-science-pipelines-operator
diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go
@@ -59,6 +59,7 @@ type DSPAReconciler struct {
 	Log                     logr.Logger
 	TemplatesPath           string
 	MaxConcurrentReconciles int
+	WebhookAnnotations      map[string]string
 }
 
 func (r *DSPAReconciler) ApplyDir(owner mf.Owner, params *DSPAParams, directory string, fns ...mf.Transformer) error {
@@ -184,7 +185,9 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	log.V(1).Info("DataSciencePipelinesApplication Reconciler called.")
 
-	params := &DSPAParams{}
+	params := &DSPAParams{
+		WebhookAnnotations: r.WebhookAnnotations,
+	}
 
 	dspa := &dspav1.DataSciencePipelinesApplication{}
 	err := r.Get(ctx, req.NamespacedName, dspa)
diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go
@@ -74,6 +74,7 @@ type DSPAParams struct {
 	WebhookName                          string
 	WorkflowController                   *dspa.WorkflowController
 	CustomKfpLauncherConfigMapData       string
+	WebhookAnnotations                   map[string]string
 	DBConnection
 	ObjectStorageConnection
 
diff --git a/controllers/webhook_test.go b/controllers/webhook_test.go
@@ -25,6 +25,7 @@ import (
 	"github.com/opendatahub-io/data-science-pipelines-operator/controllers/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	admv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -204,4 +205,39 @@ func TestWebhookLifecycle(t *testing.T) {
 		assert.True(t, created)
 		require.NoError(t, err)
 	})
+
+	t.Run("WebhookAnnotationsAreSet", func(t *testing.T) {
+		dspa := testutil.CreateTestDSPA()
+
+		ctx, params, reconciler := CreateNewTestObjects()
+		err := params.ExtractParams(ctx, dspa, reconciler.Client, reconciler.Log)
+		require.NoError(t, err)
+
+		dspoDeployment := testutil.CreateTestDSPODeployment(params.DSPONamespace)
+		err = reconciler.Client.Create(ctx, dspoDeployment)
+		assert.NoError(t, err)
+
+		params.WebhookAnnotations = map[string]string{
+			"test.annotation/key1": "value1",
+			"test.annotation/key2": "value2",
+		}
+
+		// Run test reconciliation
+		err = reconciler.ReconcileWebhook(ctx, params)
+		require.NoError(t, err)
+
+		// Verify ValidatingWebhookConfiguration has annotations
+		validatingWebhook := &admv1.ValidatingWebhookConfiguration{}
+		err = reconciler.Client.Get(ctx, types.NamespacedName{Name: "pipelineversions.pipelines.kubeflow.org"}, validatingWebhook)
+		require.NoError(t, err)
+		assert.Equal(t, "value1", validatingWebhook.Annotations["test.annotation/key1"])
+		assert.Equal(t, "value2", validatingWebhook.Annotations["test.annotation/key2"])
+
+		// Verify MutatingWebhookConfiguration has annotations
+		mutatingWebhook := &admv1.MutatingWebhookConfiguration{}
+		err = reconciler.Client.Get(ctx, types.NamespacedName{Name: "pipelineversions.pipelines.kubeflow.org"}, mutatingWebhook)
+		require.NoError(t, err)
+		assert.Equal(t, "value1", mutatingWebhook.Annotations["test.annotation/key1"])
+		assert.Equal(t, "value2", mutatingWebhook.Annotations["test.annotation/key2"])
+	})
 }
diff --git a/main.go b/main.go
@@ -17,6 +17,7 @@ limitations under the License.
 package main
 
 import (
+	"encoding/json"
 	"flag"
 	"fmt"
 	"os"
@@ -163,12 +164,23 @@ func main() {
 		os.Exit(1)
 	}
 
+	webhookAnnotations := map[string]string{}
+
+	if os.Getenv("WEBHOOK_ANNOTATIONS") != "" {
+		err := json.Unmarshal([]byte(os.Getenv("WEBHOOK_ANNOTATIONS")), &webhookAnnotations)
+		if err != nil {
+			setupLog.Error(err, "the WEBHOOK_ANNOTATIONS environment variable is not a valid JSON object")
+			os.Exit(1)
+		}
+	}
+
 	if err = (&controllers.DSPAReconciler{
 		Client:                  mgr.GetClient(),
 		Scheme:                  mgr.GetScheme(),
 		Log:                     ctrl.Log,
 		TemplatesPath:           "config/internal/",
 		MaxConcurrentReconciles: maxConcurrentReconciles,
+		WebhookAnnotations:      webhookAnnotations,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "DSPAParams")
 		os.Exit(1)
diff --git a/tests/resources/dspa-k8s.yaml b/tests/resources/dspa-k8s.yaml
@@ -13,8 +13,6 @@ spec:
       configMapName: nginx-tls-config
       configMapKey: rootCA.crt
     pipelineStore: kubernetes
-    webhookAnnotations:
-      cert-manager.io/inject-ca-from: opendatahub/dspa-webhook-cert
     resources:
       limits:
         cpu: 20m
