From 435dc41a6d1beaa32e9edfecdf5c1271921cead4 Mon Sep 17 00:00:00 2001 From: Tayler Geiger Date: Tue, 11 Nov 2025 12:39:37 -0600 Subject: [PATCH] Use operator-controller SA by default, make SA field optional Changes the ClusterExtension API field spec.ServiceAccount to be optional. Operator-controller will use its own service account by default unless the spec.ServiceAccount field is set. RBAC PreAuthorization only happens if the optional SA field is set, as well. Give operator-controller's SA cluster-admin by default. --- api/v1/clusterextension_types.go | 14 +++++++------- ...olm.operatorframework.io_clusterextensions.yaml | 9 +++------ ...olm.operatorframework.io_clusterextensions.yaml | 9 +++------ ...ing-operator-controller-manager-rolebinding.yml | 8 -------- internal/operator-controller/action/restconfig.go | 6 ++++++ internal/operator-controller/applier/helm.go | 3 ++- .../controllers/clusterextension_admission_test.go | 4 +++- manifests/experimental-e2e.yaml | 9 +++------ manifests/experimental.yaml | 9 +++------ manifests/standard-e2e.yaml | 9 +++------ manifests/standard.yaml | 9 +++------ 11 files changed, 36 insertions(+), 53 deletions(-) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6de62b0e12..13b38ab458 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -67,14 +67,14 @@ type ClusterExtensionSpec struct { // +kubebuilder:validation:Required Namespace string `json:"namespace"` - // serviceAccount is a reference to a ServiceAccount used to perform all interactions - // with the cluster that are required to manage the extension. + // serviceAccount is an optional field that references a ServiceAccount used to + // perform all interactions with the cluster that are required to manage the extension. + // If not set, operator-controller will use its own ServiceAccount for extension management. // The ServiceAccount must be configured with the necessary permissions to perform these interactions. // The ServiceAccount must exist in the namespace referenced in the spec. - // serviceAccount is required. // - // +kubebuilder:validation:Required - ServiceAccount ServiceAccountReference `json:"serviceAccount"` + // +optional + ServiceAccount ServiceAccountReference `json:"serviceAccount,omitzero"` // source is a required field which selects the installation source of content // for this ClusterExtension. Selection is performed by setting the sourceType. @@ -374,7 +374,7 @@ type CatalogFilter struct { UpgradeConstraintPolicy UpgradeConstraintPolicy `json:"upgradeConstraintPolicy,omitempty"` } -// ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension. +// ServiceAccountReference identifies the serviceAccount used to install a ClusterExtension. type ServiceAccountReference struct { // name is a required, immutable reference to the name of the ServiceAccount // to be used for installation and management of the content for the package @@ -403,7 +403,7 @@ type ServiceAccountReference struct { // +kubebuilder:validation:MaxLength:=253 // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="name is immutable" // +kubebuilder:validation:XValidation:rule="self.matches(\"^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\")",message="name must be a valid DNS1123 subdomain. It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.), start and end with an alphanumeric character, and be no longer than 253 characters" - // +kubebuilder:validation:Required + // +kubebuilder:validation:Optional Name string `json:"name"` } diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 1038b7fdf0..3e49ec691c 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -171,11 +171,11 @@ spec: rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions - with the cluster that are required to manage the extension. + serviceAccount is an optional field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. + If not set, operator-controller will use its own ServiceAccount for extension management. The ServiceAccount must be configured with the necessary permissions to perform these interactions. The ServiceAccount must exist in the namespace referenced in the spec. - serviceAccount is required. properties: name: description: |- @@ -212,8 +212,6 @@ spec: (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") - required: - - name type: object source: description: |- @@ -498,7 +496,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index a0983e41f9..465f93425f 100644 --- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -132,11 +132,11 @@ spec: rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions - with the cluster that are required to manage the extension. + serviceAccount is an optional field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. + If not set, operator-controller will use its own ServiceAccount for extension management. The ServiceAccount must be configured with the necessary permissions to perform these interactions. The ServiceAccount must exist in the namespace referenced in the spec. - serviceAccount is required. properties: name: description: |- @@ -173,8 +173,6 @@ spec: (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") - required: - - name type: object source: description: |- @@ -459,7 +457,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/helm/olmv1/templates/rbac/clusterrolebinding-operator-controller-manager-rolebinding.yml b/helm/olmv1/templates/rbac/clusterrolebinding-operator-controller-manager-rolebinding.yml index 9817337dff..91a3aeb315 100644 --- a/helm/olmv1/templates/rbac/clusterrolebinding-operator-controller-manager-rolebinding.yml +++ b/helm/olmv1/templates/rbac/clusterrolebinding-operator-controller-manager-rolebinding.yml @@ -8,19 +8,11 @@ metadata: labels: app.kubernetes.io/name: operator-controller {{- include "olmv1.labels" $ | nindent 4 }} -{{- if has "BoxcutterRuntime" .Values.options.operatorController.features.enabled }} - name: operator-controller-manager-admin-rolebinding -{{- else }} name: operator-controller-manager-rolebinding -{{- end }} roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole -{{- if has "BoxcutterRuntime" .Values.options.operatorController.features.enabled }} name: cluster-admin -{{- else }} - name: operator-controller-manager-role -{{- end }} subjects: - kind: ServiceAccount name: operator-controller-controller-manager diff --git a/internal/operator-controller/action/restconfig.go b/internal/operator-controller/action/restconfig.go index 05e25f707d..30b2a9a5d7 100644 --- a/internal/operator-controller/action/restconfig.go +++ b/internal/operator-controller/action/restconfig.go @@ -44,6 +44,12 @@ func ServiceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) fun if err != nil { return nil, err } + + // If ServiceAccount is not set, just use operator-controller's service account + if cExt.Spec.ServiceAccount.Name == "" { + return c, nil + } + saConfig := rest.AnonymousClientConfig(c) saConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper { return &authentication.TokenInjectingRoundTripper{ diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 4e70268941..fc22473f6c 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -113,7 +113,8 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte labels: objectLabels, } - if h.PreAuthorizer != nil { + // Only run pre-Authorization if optional ServiceAccount field name is set + if h.PreAuthorizer != nil && ext.Spec.ServiceAccount.Name != "" { err := h.runPreAuthorizationChecks(ctx, ext, chrt, values, post) if err != nil { // Return the pre-authorization error directly diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go index 38c6c60d41..51e86aae20 100644 --- a/internal/operator-controller/controllers/clusterextension_admission_test.go +++ b/internal/operator-controller/controllers/clusterextension_admission_test.go @@ -349,7 +349,9 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) { {"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}, + // This test case passes now that the ServiceAccount field is optional. + // The empty serialized version of the ServiceAccount field has an empty string for Name. + {"no service account name", "", ""}, {"spaces", "spaces spaces", regexMismatchError}, {"capitalized", "Capitalized", regexMismatchError}, {"camel case", "camelCase", regexMismatchError}, diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index db03c11a8d..ed8b10648a 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -976,11 +976,11 @@ spec: rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions - with the cluster that are required to manage the extension. + serviceAccount is an optional field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. + If not set, operator-controller will use its own ServiceAccount for extension management. The ServiceAccount must be configured with the necessary permissions to perform these interactions. The ServiceAccount must exist in the namespace referenced in the spec. - serviceAccount is required. properties: name: description: |- @@ -1017,8 +1017,6 @@ spec: (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") - required: - - name type: object source: description: |- @@ -1303,7 +1301,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 664f8599cc..95a688168d 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -941,11 +941,11 @@ spec: rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions - with the cluster that are required to manage the extension. + serviceAccount is an optional field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. + If not set, operator-controller will use its own ServiceAccount for extension management. The ServiceAccount must be configured with the necessary permissions to perform these interactions. The ServiceAccount must exist in the namespace referenced in the spec. - serviceAccount is required. properties: name: description: |- @@ -982,8 +982,6 @@ spec: (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") - required: - - name type: object source: description: |- @@ -1268,7 +1266,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 5c95907841..69a0bd83d0 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -723,11 +723,11 @@ spec: rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions - with the cluster that are required to manage the extension. + serviceAccount is an optional field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. + If not set, operator-controller will use its own ServiceAccount for extension management. The ServiceAccount must be configured with the necessary permissions to perform these interactions. The ServiceAccount must exist in the namespace referenced in the spec. - serviceAccount is required. properties: name: description: |- @@ -764,8 +764,6 @@ spec: (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") - required: - - name type: object source: description: |- @@ -1050,7 +1048,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 95e400c264..b3f8b74ead 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -688,11 +688,11 @@ spec: rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions - with the cluster that are required to manage the extension. + serviceAccount is an optional field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. + If not set, operator-controller will use its own ServiceAccount for extension management. The ServiceAccount must be configured with the necessary permissions to perform these interactions. The ServiceAccount must exist in the namespace referenced in the spec. - serviceAccount is required. properties: name: description: |- @@ -729,8 +729,6 @@ spec: (.), start and end with an alphanumeric character, and be no longer than 253 characters rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$") - required: - - name type: object source: description: |- @@ -1015,7 +1013,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: