feat: add ValidatingAdmissionPolicy for serviceAccount deprecation warning

Create VAP and VAPB resources that emit a warning when
spec.serviceAccount is set on a ClusterExtension. Add integration
test infrastructure for warning collection.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: pedjak

helm/olmv1/templates/validatingadmissionpolicy-clusterextension-serviceaccount-deprecated.yml

@@ -0,0 +1,26 @@
+{{- if .Values.options.operatorController.enabled }}
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicy
+metadata:
+  name: clusterextension-serviceaccount-deprecated
+  labels:
+    app.kubernetes.io/name: operator-controller
+    {{- include "olmv1.labels" . | nindent 4 }}
+  annotations:
+    {{- include "olmv1.annotations" . | nindent 4 }}
+spec:
+  matchConstraints:
+    resourceRules:
+      - apiGroups:
+          - olm.operatorframework.io
+        apiVersions:
+          - v1
+        operations:
+          - CREATE
+          - UPDATE
+        resources:
+          - clusterextensions
+  validations:
+    - expression: "!has(object.spec.serviceAccount) || !has(object.spec.serviceAccount.name) || object.spec.serviceAccount.name == ''"
+      message: "spec.serviceAccount is deprecated, ignored, and will be removed in a future release. The operator-controller's cluster-admin service account is used for all cluster interactions."
+{{- end }}

helm/olmv1/templates/validatingadmissionpolicybinding-clusterextension-serviceaccount-deprecated.yml

@@ -0,0 +1,15 @@
+{{- if .Values.options.operatorController.enabled }}
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  name: clusterextension-serviceaccount-deprecated
+  labels:
+    app.kubernetes.io/name: operator-controller
+    {{- include "olmv1.labels" . | nindent 4 }}
+  annotations:
+    {{- include "olmv1.annotations" . | nindent 4 }}
+spec:
+  policyName: clusterextension-serviceaccount-deprecated
+  validationActions:
+    - Warn
+{{- end }}

internal/operator-controller/controllers/clusterextension_admission_test.go

@@ -45,9 +45,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
 						},
 					},
 					Namespace: "default",
-					ServiceAccount: ocv1.ServiceAccountReference{
-						Name: "default",
-					},
 				}))
 			}
 			if tc.unionField == "" {
@@ -56,9 +53,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
 						SourceType: tc.sourceType,
 					},
 					Namespace: "default",
-					ServiceAccount: ocv1.ServiceAccountReference{
-						Name: "default",
-					},
 				}))
 			}
 
@@ -123,9 +117,6 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -221,9 +212,6 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -245,7 +233,7 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
 		errMsg   string
 	}{
 		{"no channel name", []string{""}, regexMismatchError},
-		{"hypen-separated", []string{"hyphenated-name"}, ""},
+		{"hyphen-separated", []string{"hyphenated-name"}, ""},
 		{"dot-separated", []string{"dotted.name"}, ""},
 		{"includes version", []string{"channel-has-version-1.0.1"}, ""},
 		{"long channel name", []string{strings.Repeat("x", 254)}, tooLongError},
@@ -276,9 +264,6 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for channel %q: %w", tc.channels, err)
@@ -300,7 +285,7 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 		errMsg    string
 	}{
 		{"just alphanumeric", "justalphanumberic1", ""},
-		{"hypen-separated", "hyphenated-name", ""},
+		{"hyphen-separated", "hyphenated-name", ""},
 		{"no install namespace", "", regexMismatchError},
 		{"dot-separated", "dotted.name", regexMismatchError},
 		{"longest valid install namespace", strings.Repeat("x", 63), ""},
@@ -329,9 +314,6 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 					},
 				},
 				Namespace: tc.namespace,
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for namespace %q: %w", tc.namespace, err)
@@ -343,38 +325,43 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 	}
 }
 
+// TestClusterExtensionAdmissionServiceAccount validates the deprecated spec.serviceAccount field:
+// - CRD-level validation (format, length) still works
+// - ValidatingAdmissionPolicy emits a deprecation warning for valid non-empty values
 func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
 	tooLongError := "spec.serviceAccount.name: Too long: may not be more than 253"
 	regexMismatchError := "name must be a valid DNS1123 subdomain"
+	deprecationWarning := "spec.serviceAccount is deprecated"
 
 	testCases := []struct {
 		name           string
 		serviceAccount string
 		errMsg         string
+		warnMsg        string
 	}{
-		{"just alphanumeric", "justalphanumeric1", ""},
-		{"hypen-separated", "hyphenated-name", ""},
-		{"dot-separated", "dotted.name", ""},
-		{"longest valid service account name", strings.Repeat("x", 253), ""},
-		{"too long service account name", strings.Repeat("x", 254), tooLongError},
-		{"no service account name", "", regexMismatchError},
-		{"spaces", "spaces spaces", regexMismatchError},
-		{"capitalized", "Capitalized", regexMismatchError},
-		{"camel case", "camelCase", regexMismatchError},
-		{"invalid characters", "many/invalid$characters+in_name", regexMismatchError},
-		{"starts with hyphen", "-start-with-hyphen", regexMismatchError},
-		{"ends with hyphen", "end-with-hyphen-", regexMismatchError},
-		{"starts with period", ".start-with-period", regexMismatchError},
-		{"ends with period", "end-with-period.", regexMismatchError},
-		{"multiple sequential separators", "a.-b", regexMismatchError},
+		{"just alphanumeric", "justalphanumeric1", "", deprecationWarning},
+		{"hyphen-separated", "hyphenated-name", "", deprecationWarning},
+		{"dot-separated", "dotted.name", "", deprecationWarning},
+		{"longest valid service account name", strings.Repeat("x", 253), "", deprecationWarning},
+		{"too long service account name", strings.Repeat("x", 254), tooLongError, ""},
+		{"no service account name", "", "", ""},
+		{"spaces", "spaces spaces", regexMismatchError, ""},
+		{"capitalized", "Capitalized", regexMismatchError, ""},
+		{"camel case", "camelCase", regexMismatchError, ""},
+		{"invalid characters", "many/invalid$characters+in_name", regexMismatchError, ""},
+		{"starts with hyphen", "-start-with-hyphen", regexMismatchError, ""},
+		{"ends with hyphen", "end-with-hyphen-", regexMismatchError, ""},
+		{"starts with period", ".start-with-period", regexMismatchError, ""},
+		{"ends with period", "end-with-period.", regexMismatchError, ""},
+		{"multiple sequential separators", "a.-b", regexMismatchError, ""},
 	}
 
 	t.Parallel()
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			cl := newClient(t)
+			cl, collector := newWarningCapturingClient(t)
 			err := cl.Create(context.Background(), buildClusterExtension(ocv1.ClusterExtensionSpec{
 				Source: ocv1.SourceConfig{
 					SourceType: "Catalog",
@@ -383,7 +370,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
+				ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // testing deprecated field
 					Name: tc.serviceAccount,
 				},
 			}))
@@ -393,6 +380,9 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
 				require.Error(t, err)
 				require.Contains(t, err.Error(), tc.errMsg)
 			}
+			if tc.warnMsg != "" {
+				require.True(t, collector.hasWarning(tc.warnMsg), "expected deprecation warning containing %q", tc.warnMsg)
+			}
 		})
 	}
 }
@@ -442,10 +432,7 @@ func TestClusterExtensionAdmissionInstall(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
-				Install: tc.installConfig,
+				Install:   tc.installConfig,
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for install configuration %v: %w", tc.installConfig, err)
@@ -494,9 +481,6 @@ func Test_ClusterExtensionAdmissionInlineConfig(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 				Config: &ocv1.ClusterExtensionConfig{
 					ConfigType: ocv1.ClusterExtensionConfigTypeInline,
 					Inline: &apiextensionsv1.JSON{

internal/operator-controller/controllers/suite_test.go

@@ -17,13 +17,18 @@ limitations under the License.
 package controllers_test
 
 import (
+	"context"
 	"log"
 	"os"
+	"strings"
+	"sync"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apimachineryruntime "k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/rest"
@@ -53,6 +58,37 @@ func newClient(t *testing.T) client.Client {
 	return cl
 }
 
+type warningCollector struct {
+	mu    sync.Mutex
+	items []string
+}
+
+func (w *warningCollector) HandleWarningHeader(code int, agent string, text string) {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+	w.items = append(w.items, text)
+}
+
+func (w *warningCollector) hasWarning(substr string) bool {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+	for _, item := range w.items {
+		if strings.Contains(item, substr) {
+			return true
+		}
+	}
+	return false
+}
+
+func newWarningCapturingClient(t *testing.T) (client.Client, *warningCollector) {
+	collector := &warningCollector{}
+	cfg := rest.CopyConfig(config)
+	cfg.WarningHandler = collector
+	cl, err := client.New(cfg, client.Options{Scheme: newScheme(t)})
+	require.NoError(t, err)
+	return cl, collector
+}
+
 // newMockRevisionStatesGetter creates a gomock-based RevisionStatesGetter
 // that returns fixed values, replacing the hand-written MockRevisionStatesGetter.
 func newMockRevisionStatesGetter(ctrl *gomock.Controller, revisionStates *controllers.RevisionStates, err error) *mockcontrollers.MockRevisionStatesGetter {
@@ -128,6 +164,38 @@ func TestMain(m *testing.M) {
 		log.Panic("expected cfg to not be nil")
 	}
 
+	cl, err := client.New(config, client.Options{})
+	utilruntime.Must(err)
+	ctx := context.Background()
+	utilruntime.Must(cl.Create(ctx, &admissionregistrationv1.ValidatingAdmissionPolicy{
+		ObjectMeta: metav1.ObjectMeta{Name: "clusterextension-serviceaccount-deprecated"},
+		Spec: admissionregistrationv1.ValidatingAdmissionPolicySpec{
+			MatchConstraints: &admissionregistrationv1.MatchResources{
+				ResourceRules: []admissionregistrationv1.NamedRuleWithOperations{{
+					RuleWithOperations: admissionregistrationv1.RuleWithOperations{
+						Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update},
+						Rule: admissionregistrationv1.Rule{
+							APIGroups:   []string{"olm.operatorframework.io"},
+							APIVersions: []string{"v1"},
+							Resources:   []string{"clusterextensions"},
+						},
+					},
+				}},
+			},
+			Validations: []admissionregistrationv1.Validation{{
+				Expression: `!has(object.spec.serviceAccount) || !has(object.spec.serviceAccount.name) || object.spec.serviceAccount.name == ''`,
+				Message:    "spec.serviceAccount is deprecated, ignored, and will be removed in a future release. The operator-controller's cluster-admin service account is used for all cluster interactions.",
+			}},
+		},
+	}))
+	utilruntime.Must(cl.Create(ctx, &admissionregistrationv1.ValidatingAdmissionPolicyBinding{
+		ObjectMeta: metav1.ObjectMeta{Name: "clusterextension-serviceaccount-deprecated"},
+		Spec: admissionregistrationv1.ValidatingAdmissionPolicyBindingSpec{
+			PolicyName:        "clusterextension-serviceaccount-deprecated",
+			ValidationActions: []admissionregistrationv1.ValidationAction{admissionregistrationv1.Warn},
+		},
+	}))
+
 	code := m.Run()
 	// Use Eventually wrapper for graceful test environment teardown
 	// controller-runtime v0.23.0+ requires this to prevent timing-related errors