diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6de62b0e12..c1fea6aa35 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -67,14 +67,21 @@ 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 a required field that references 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"` + // + // + 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 +381,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,8 +410,8 @@ 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 - Name string `json:"name"` + // +optional + Name string `json:"name,omitempty"` } // PreflightConfig holds the configuration for the preflight checks. If used, at least one preflight check must be non-nil. diff --git a/api/v1/clusterextension_types_test.go b/api/v1/clusterextension_types_test.go index feb9b4e224..d8c11cf17a 100644 --- a/api/v1/clusterextension_types_test.go +++ b/api/v1/clusterextension_types_test.go @@ -1,6 +1,7 @@ package v1_test import ( + "encoding/json" "fmt" "go/ast" "go/parser" @@ -11,6 +12,7 @@ import ( "golang.org/x/exp/slices" // replace with "slices" in go 1.21 + v1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets" ) @@ -100,3 +102,62 @@ func parseConstants(prefix string) ([]string, error) { } return constValues, nil } + +func TestServiceAccountMarshaling(t *testing.T) { + tests := []struct { + name string + spec v1.ClusterExtensionSpec + expectField string + unexpectField string + }{ + { + name: "ServiceAccount with name is marshaled", + spec: v1.ClusterExtensionSpec{ + Namespace: "test-ns", + ServiceAccount: v1.ServiceAccountReference{ + Name: "test-sa", + }, + Source: v1.SourceConfig{ + SourceType: "Catalog", + Catalog: &v1.CatalogFilter{ + PackageName: "test-package", + }, + }, + }, + expectField: "serviceAccount", + }, + { + name: "ServiceAccount with empty name is omitted", + spec: v1.ClusterExtensionSpec{ + Namespace: "test-ns", + ServiceAccount: v1.ServiceAccountReference{}, + Source: v1.SourceConfig{ + SourceType: "Catalog", + Catalog: &v1.CatalogFilter{ + PackageName: "test-package", + }, + }, + }, + unexpectField: "serviceAccount", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data, err := json.Marshal(tt.spec) + if err != nil { + t.Fatalf("failed to marshal spec: %v", err) + } + + jsonStr := string(data) + + if tt.expectField != "" && !strings.Contains(jsonStr, tt.expectField) { + t.Errorf("expected field %q to be present in JSON output, got: %s", tt.expectField, jsonStr) + } + + if tt.unexpectField != "" && strings.Contains(jsonStr, tt.unexpectField) { + t.Errorf("expected field %q to be omitted from JSON output, got: %s", tt.unexpectField, jsonStr) + } + }) + } +} diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 317b46a00c..2ae26efeb6 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -340,7 +340,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `namespace` _string_ | namespace is a reference to a Kubernetes namespace.
This is the namespace in which the provided ServiceAccount must exist.
It also designates the default namespace where namespace-scoped resources
for the extension are applied to the cluster.
Some extensions may contain namespace-scoped resources to be applied in other namespaces.
This namespace must exist.
namespace is required, immutable, and follows the DNS label standard
as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-),
start and end with an alphanumeric character, and be no longer than 63 characters
[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 63
Required: \{\}
| -| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions
with the cluster that are required to manage the extension.
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. | | Required: \{\}
| +| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ |
serviceAccount is a required field that references 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.

| | | | `source` _[SourceConfig](#sourceconfig)_ | source is a required field which selects the installation source of content
for this ClusterExtension. Selection is performed by setting the sourceType.
Catalog is currently the only implemented sourceType, and setting the
sourcetype to "Catalog" requires the catalog field to also be defined.
Below is a minimal example of a source definition (in yaml):
source:
sourceType: Catalog
catalog:
packageName: example-package | | Required: \{\}
| | `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is an optional field used to configure the installation options
for the ClusterExtension such as the pre-flight check configuration. | | | | `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is an optional field used to specify bundle specific configuration
used to configure the bundle. Configuration is bundle specific and a bundle may provide
a configuration schema. When not specified, the default configuration of the resolved bundle will be used.
config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide
a configuration schema the final manifests will be derived on a best-effort basis. More information on how
to configure the bundle should be found in its end-user documentation.
| | | @@ -439,7 +439,7 @@ _Appears in:_ -ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension. +ServiceAccountReference identifies the serviceAccount used to install a ClusterExtension. @@ -448,7 +448,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `name` _string_ | 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
specified in the packageName field.
This ServiceAccount must exist in the installNamespace.
name follows the DNS subdomain standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters,
hyphens (-) or periods (.), start and end with an alphanumeric character,
and be no longer than 253 characters.
Some examples of valid values are:
- some-serviceaccount
- 123-serviceaccount
- 1-serviceaccount-2
- someserviceaccount
- some.serviceaccount
Some examples of invalid values are:
- -some-serviceaccount
- some-serviceaccount-
[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 253
Required: \{\}
| +| `name` _string_ | 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
specified in the packageName field.
This ServiceAccount must exist in the installNamespace.
name follows the DNS subdomain standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters,
hyphens (-) or periods (.), start and end with an alphanumeric character,
and be no longer than 253 characters.
Some examples of valid values are:
- some-serviceaccount
- 123-serviceaccount
- 1-serviceaccount-2
- someserviceaccount
- some.serviceaccount
Some examples of invalid values are:
- -some-serviceaccount
- some-serviceaccount-
[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 253
| #### SourceConfig diff --git a/hack/tools/crd-generator/main.go b/hack/tools/crd-generator/main.go index 9687489f45..c9a3744b51 100644 --- a/hack/tools/crd-generator/main.go +++ b/hack/tools/crd-generator/main.go @@ -23,6 +23,7 @@ import ( "log" "os" "regexp" + "slices" "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -136,7 +137,7 @@ func runGenerator(args ...string) { if channel == StandardChannel && strings.Contains(version.Name, "alpha") { channelCrd.Spec.Versions[i].Served = false } - version.Schema.OpenAPIV3Schema.Properties = opconTweaksMap(channel, version.Schema.OpenAPIV3Schema.Properties) + channelCrd.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties, channelCrd.Spec.Versions[i].Schema.OpenAPIV3Schema.Required = opconTweaksMap(channel, version.Schema.OpenAPIV3Schema.Properties, version.Schema.OpenAPIV3Schema.Required) } conv, err := crd.AsVersion(*channelCrd, apiextensionsv1.SchemeGroupVersion) @@ -179,25 +180,43 @@ func runGenerator(args ...string) { } } -func opconTweaksMap(channel string, props map[string]apiextensionsv1.JSONSchemaProps) map[string]apiextensionsv1.JSONSchemaProps { +func opconTweaksMap(channel string, props map[string]apiextensionsv1.JSONSchemaProps, existingRequired []string) (map[string]apiextensionsv1.JSONSchemaProps, []string) { + // Start with existing required fields (from kubebuilder markers) + requiredFields := slices.Clone(existingRequired) + for name := range props { jsonProps := props[name] - p := opconTweaks(channel, name, jsonProps) + p, reqStatus := opconTweaks(channel, name, jsonProps) if p == nil { delete(props, name) + // Remove from required list if present + requiredFields = slices.DeleteFunc(requiredFields, func(s string) bool { return s == name }) } else { props[name] = *p + // Update required list based on tag + switch reqStatus { + case "required": + if !slices.Contains(requiredFields, name) { + requiredFields = append(requiredFields, name) + } + case "optional": + requiredFields = slices.DeleteFunc(requiredFields, func(s string) bool { return s == name }) + // "" (unspecified) means keep existing status + } } } - return props + return props, requiredFields } // Custom Opcon API Tweaks for tags prefixed with `") { - return nil + return nil, "" } } @@ -237,6 +256,22 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche Rule: celMatch[2], }) } + + optReqRe := regexp.MustCompile(validationPrefix + "(Optional|Required)>") + optReqMatches := optReqRe.FindAllStringSubmatch(jsonProps.Description, 64) + for _, optReqMatch := range optReqMatches { + if len(optReqMatch) != 2 { + log.Fatalf("Invalid %s Optional/Required tag for %s", validationPrefix, name) + } + + numValid++ + switch optReqMatch[1] { + case "Optional": + requiredStatus = "optional" + case "Required": + requiredStatus = "required" + } + } } if numValid < numExpressions { @@ -246,34 +281,42 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche jsonProps.Description = formatDescription(jsonProps.Description, channel, name) if len(jsonProps.Properties) > 0 { - jsonProps.Properties = opconTweaksMap(channel, jsonProps.Properties) + jsonProps.Properties, jsonProps.Required = opconTweaksMap(channel, jsonProps.Properties, jsonProps.Required) } else if jsonProps.Items != nil && jsonProps.Items.Schema != nil { - jsonProps.Items.Schema = opconTweaks(channel, name, *jsonProps.Items.Schema) + jsonProps.Items.Schema, _ = opconTweaks(channel, name, *jsonProps.Items.Schema) } - return &jsonProps + return &jsonProps, requiredStatus } func formatDescription(description string, channel string, name string) string { - startTag := "" - endTag := "" - if channel == StandardChannel && strings.Contains(description, startTag) { - regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*` - re := regexp.MustCompile(regexPattern) - match := re.FindStringSubmatch(description) - if len(match) != 2 { - log.Fatalf("Invalid tag for %s", name) + tagset := []struct { + channel string + start string + end string + }{ + {channel: ExperimentalChannel, start: "", end: ""}, + {channel: StandardChannel, start: "", end: ""}, + } + for _, ts := range tagset { + if channel == ts.channel && strings.Contains(description, ts.start) { + regexPattern := `\n*` + regexp.QuoteMeta(ts.start) + `(?s:(.*?))` + regexp.QuoteMeta(ts.end) + `\n*` + re := regexp.MustCompile(regexPattern) + match := re.FindStringSubmatch(description) + if len(match) != 2 { + log.Fatalf("Invalid tag for %s", name) + } + description = re.ReplaceAllString(description, "\n\n") + } else { + description = strings.ReplaceAll(description, ts.start, "") + description = strings.ReplaceAll(description, ts.end, "") } - description = re.ReplaceAllString(description, "\n\n") - } else { - description = strings.ReplaceAll(description, startTag, "") - description = strings.ReplaceAll(description, endTag, "") } // Comments within "opcon:util:excludeFromCRD" tag are not included in the generated CRD and all trailing \n operators before // and after the tags are removed and replaced with three \n operators. - startTag = "" - endTag = "" + startTag := "" + endTag := "" if strings.Contains(description, startTag) { regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*` re := regexp.MustCompile(regexPattern) 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..38ec6a8b4c 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,8 @@ 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. - 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. + serviceAccount is a required field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. properties: name: description: |- @@ -173,8 +170,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: |- 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..19b7e8df4e 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: @@ -1702,7 +1699,7 @@ metadata: labels: app.kubernetes.io/name: operator-controller app.kubernetes.io/part-of: olm - name: operator-controller-manager-admin-rolebinding + name: operator-controller-manager-rolebinding roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 664f8599cc..d1c7c124bb 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: @@ -1667,7 +1664,7 @@ metadata: labels: app.kubernetes.io/name: operator-controller app.kubernetes.io/part-of: olm - name: operator-controller-manager-admin-rolebinding + name: operator-controller-manager-rolebinding roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 5c95907841..e918cabe71 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -723,11 +723,8 @@ 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. - 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. + serviceAccount is a required field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. properties: name: description: |- @@ -764,8 +761,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: |- @@ -1453,7 +1448,7 @@ metadata: roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: operator-controller-manager-role + name: cluster-admin subjects: - kind: ServiceAccount name: operator-controller-controller-manager diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 95e400c264..5ec65eaaeb 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -688,11 +688,8 @@ 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. - 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. + serviceAccount is a required field that references a ServiceAccount used to + perform all interactions with the cluster that are required to manage the extension. properties: name: description: |- @@ -729,8 +726,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: |- @@ -1418,7 +1413,7 @@ metadata: roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: operator-controller-manager-role + name: cluster-admin subjects: - kind: ServiceAccount name: operator-controller-controller-manager diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index ab0bf48b1c..4e7afbe9de 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -13,6 +13,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -789,3 +790,129 @@ func TestClusterExtensionRecoversFromExistingDeploymentWhenFailureFixed(t *testi require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) }, pollDuration, pollInterval) } + +func TestClusterExtensionInstallWithoutServiceAccount(t *testing.T) { + t.Log("When a cluster extension is installed without specifying a ServiceAccount") + t.Log("It should use the operator-controller ServiceAccount with synthetic user permissions") + + clusterExtension, extensionCatalog := TestInitClusterExtensionClusterCatalog(t) + defer TestCleanup(t, extensionCatalog, clusterExtension, nil, nil) + defer utils.CollectTestArtifacts(t, artifactName, c, cfg) + + // Create only the namespace (no ServiceAccount) + ns, err := CreateNamespace(context.Background(), clusterExtension.Name) + require.NoError(t, err) + defer TestCleanup(t, nil, nil, nil, ns) + + // Create a ClusterRole for the synthetic user with necessary permissions + syntheticUserName := fmt.Sprintf("olm:clusterextension:%s", clusterExtension.Name) + clusterRoleName := fmt.Sprintf("synthetic-user-cr-%s", clusterExtension.Name) + cr := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterRoleName, + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"olm.operatorframework.io"}, + Resources: []string{"clusterextensions/finalizers"}, + Verbs: []string{"update"}, + ResourceNames: []string{clusterExtension.Name}, + }, + { + APIGroups: []string{""}, + Resources: []string{"configmaps", "secrets", "services", "serviceaccounts"}, + Verbs: []string{"create", "update", "delete", "patch", "get", "list", "watch"}, + }, + { + APIGroups: []string{"apiextensions.k8s.io"}, + Resources: []string{"customresourcedefinitions"}, + Verbs: []string{"create", "update", "delete", "patch", "get", "list", "watch"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"deployments"}, + Verbs: []string{"create", "update", "delete", "patch", "get", "list", "watch"}, + }, + { + APIGroups: []string{"rbac.authorization.k8s.io"}, + Resources: []string{"clusterroles", "roles", "clusterrolebindings", "rolebindings"}, + Verbs: []string{"create", "update", "delete", "patch", "get", "list", "watch", "bind", "escalate"}, + }, + { + APIGroups: []string{"networking.k8s.io"}, + Resources: []string{"networkpolicies"}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, + }, + } + require.NoError(t, c.Create(context.Background(), cr)) + defer func() { + require.NoError(t, c.Delete(context.Background(), cr)) + }() + + // Create a ClusterRoleBinding for the synthetic user + clusterRoleBindingName := fmt.Sprintf("synthetic-user-crb-%s", clusterExtension.Name) + crb := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterRoleBindingName, + }, + Subjects: []rbacv1.Subject{ + { + Kind: "User", + Name: syntheticUserName, + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: clusterRoleName, + }, + } + require.NoError(t, c.Create(context.Background(), crb)) + defer func() { + require.NoError(t, c.Delete(context.Background(), crb)) + }() + + clusterExtension.Spec = ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{ + PackageName: "test", + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name}, + }, + }, + }, + Namespace: ns.Name, + // ServiceAccount intentionally omitted - should use operator-controller SA with synthetic user + } + + t.Log("By creating the ClusterExtension resource without a ServiceAccount") + require.NoError(t, c.Create(context.Background(), clusterExtension)) + + t.Log("By eventually reporting Progressing == True with Reason Succeeded") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + }, pollDuration, pollInterval) + + t.Log("By eventually installing the package successfully") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) + cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled) + require.NotNil(ct, cond) + require.Equal(ct, metav1.ConditionTrue, cond.Status) + require.Equal(ct, ocv1.ReasonSucceeded, cond.Reason) + require.Contains(ct, cond.Message, "Installed bundle") + require.NotEmpty(ct, clusterExtension.Status.Install.Bundle) + }, pollDuration, pollInterval) + + t.Log("By verifying the NetworkPolicy was created successfully") + require.EventuallyWithT(t, func(ct *assert.CollectT) { + var np networkingv1.NetworkPolicy + require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: "test-operator-network-policy", Namespace: ns.Name}, &np)) + }, pollDuration, pollInterval) +}