diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index e331ec63e1..99aedbc643 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -49,8 +49,7 @@ const ( // ClusterExtensionSpec defines the desired state of ClusterExtension type ClusterExtensionSpec struct { // 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 + // It 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. @@ -67,14 +66,13 @@ type ClusterExtensionSpec struct { // +kubebuilder:validation:Required Namespace string `json:"namespace"` - // serviceAccount is a reference to a ServiceAccount used to perform all interactions + // Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + // serviceAccount was a reference to the 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 optional. // - // +kubebuilder:validation:Required - ServiceAccount ServiceAccountReference `json:"serviceAccount"` + // +kubebuilder:validation: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. @@ -369,8 +367,9 @@ type CatalogFilter struct { UpgradeConstraintPolicy UpgradeConstraintPolicy `json:"upgradeConstraintPolicy,omitempty"` } -// ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension. +// Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. type ServiceAccountReference struct { + // Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. // 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. diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index c3241ce632..326e4b5a11 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -62,8 +62,6 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/action" "github.com/operator-framework/operator-controller/internal/operator-controller/applier" - "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" - "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/cache" catalogclient "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/client" "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager" @@ -625,11 +623,8 @@ func setupHelm( if err != nil { return fmt.Errorf("unable to create core client: %w", err) } - tokenGetter := authentication.NewTokenGetter(coreClient, authentication.WithExpirationDuration(1*time.Hour)) - clientRestConfigMapper := action.ServiceAccountRestConfigMapper(tokenGetter) - if features.OperatorControllerFeatureGate.Enabled(features.SyntheticPermissions) { - clientRestConfigMapper = action.SyntheticUserRestConfigMapper(clientRestConfigMapper) - } + + clientRestConfigMapper := action.ClusterAdminRestConfigMapper(mgr.GetConfig()) cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)), @@ -650,12 +645,6 @@ func setupHelm( return fmt.Errorf("unable to create helm action client getter: %w", err) } - // determine if PreAuthorizer should be enabled based on feature gate - var preAuth authorization.PreAuthorizer - if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) { - preAuth = authorization.NewRBACPreAuthorizer(mgr.GetClient()) - } - cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()) err = clusterExtensionFinalizers.Register(controllers.ClusterExtensionCleanupContentManagerCacheFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { ext := obj.(*ocv1.ClusterExtension) @@ -675,7 +664,6 @@ func setupHelm( ManifestProvider: regv1ManifestProvider, }, HelmReleaseToObjectsConverter: &applier.HelmReleaseToObjectsConverter{}, - PreAuthorizer: preAuth, Watcher: ceController, Manager: cm, } diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 1b1ad66565..f820ec64f2 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -339,8 +339,8 @@ _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: \{\}
| +| `namespace` _string_ | namespace is a reference to a Kubernetes namespace.
It 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)_ | Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release.
serviceAccount was a reference to the ServiceAccount used to perform all interactions
with the cluster that are required to manage the extension.
serviceAccount is optional. | | Optional: \{\}
| | `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 contains optional configuration values applied during rendering of the
ClusterExtension's manifests. Values can be specified inline.

config is optional. When not specified, the default configuration of the resolved bundle will be used.

| | | @@ -440,7 +440,7 @@ _Appears in:_ -ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension. +Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. @@ -449,7 +449,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_ | Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release.
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: \{\}
| #### SourceConfig diff --git a/go.mod b/go.mod index 8b94dc19a5..cb69461d44 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,6 @@ require ( k8s.io/client-go v0.34.1 k8s.io/component-base v0.34.1 k8s.io/klog/v2 v2.130.1 - k8s.io/kubernetes v1.34.0 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 pkg.package-operator.run/boxcutter v0.7.1 sigs.k8s.io/controller-runtime v0.22.1 @@ -49,10 +48,7 @@ require ( sigs.k8s.io/yaml v1.6.0 ) -require ( - k8s.io/component-helpers v0.34.0 // indirect - k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect -) +require k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect require ( cel.dev/expr v0.24.0 // indirect @@ -242,7 +238,6 @@ require ( gopkg.in/warnings.v0 v0.1.2 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/controller-manager v0.33.2 // indirect k8s.io/kubectl v0.34.0 // indirect oras.land/oras-go/v2 v2.6.0 // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.33.0 // indirect diff --git a/go.sum b/go.sum index fed9184675..6f834ef909 100644 --- a/go.sum +++ b/go.sum @@ -765,18 +765,12 @@ k8s.io/client-go v0.34.0 h1:YoWv5r7bsBfb0Hs2jh8SOvFbKzzxyNo0nSb0zC19KZo= k8s.io/client-go v0.34.0/go.mod h1:ozgMnEKXkRjeMvBZdV1AijMHLTh3pbACPvK7zFR+QQY= k8s.io/component-base v0.34.0 h1:bS8Ua3zlJzapklsB1dZgjEJuJEeHjj8yTu1gxE2zQX8= k8s.io/component-base v0.34.0/go.mod h1:RSCqUdvIjjrEm81epPcjQ/DS+49fADvGSCkIP3IC6vg= -k8s.io/component-helpers v0.34.0 h1:5T7P9XGMoUy1JDNKzHf0p/upYbeUf8ZaSf9jbx0QlIo= -k8s.io/component-helpers v0.34.0/go.mod h1:kaOyl5tdtnymriYcVZg4uwDBe2d1wlIpXyDkt6sVnt4= -k8s.io/controller-manager v0.34.0 h1:oCHoqS8dcFp7zDSu7HUvTpakq3isSxil3GprGGlJMsE= -k8s.io/controller-manager v0.34.0/go.mod h1:XFto21U+Mm9BT8r/Jd5E4tHCGtwjKAUFOuDcqaj2VK0= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b h1:MloQ9/bdJyIu9lb1PzujOPolHyvO06MXG5TUIj2mNAA= k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b/go.mod h1:UZ2yyWbFTpuhSbFhv24aGNOdoRdJZgsIObGBUaYVsts= k8s.io/kubectl v0.34.0 h1:NcXz4TPTaUwhiX4LU+6r6udrlm0NsVnSkP3R9t0dmxs= k8s.io/kubectl v0.34.0/go.mod h1:bmd0W5i+HuG7/p5sqicr0Li0rR2iIhXL0oUyLF3OjR4= -k8s.io/kubernetes v1.34.0 h1:NvUrwPAVB4W3mSOpJ/RtNGHWWYyUP/xPaX5rUSpzA0w= -k8s.io/kubernetes v1.34.0/go.mod h1:iu+FhII+Oc/1gGWLJcer6wpyih441aNFHl7Pvm8yPto= k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 h1:hwvWFiBzdWw1FhfY1FooPn3kzWuJ8tmbZBHi4zVsl1Y= k8s.io/utils v0.0.0-20250604170112-4c0f3b243397/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go/v2 v2.6.0 h1:X4ELRsiGkrbeox69+9tzTu492FMUu7zJQW6eJU+I2oc= diff --git a/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml b/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml deleted file mode 100644 index 7eb5a7082b..0000000000 --- a/hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: olm.operatorframework.io/v1 -kind: ClusterExtension -metadata: - name: argocd-operator -spec: - namespace: argocd-system - serviceAccount: - name: "olm.synthetic-user" - source: - sourceType: Catalog - catalog: - packageName: argocd-operator - version: 0.6.0 diff --git a/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml b/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml deleted file mode 100644 index d0ab570f7b..0000000000 --- a/hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: clusterextensions-group-admin-binding -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: cluster-admin -subjects: - - kind: Group - name: "olm:clusterextensions" diff --git a/hack/demo/synthetic-user-cluster-admin-demo-script.sh b/hack/demo/synthetic-user-cluster-admin-demo-script.sh deleted file mode 100755 index 4790e46e77..0000000000 --- a/hack/demo/synthetic-user-cluster-admin-demo-script.sh +++ /dev/null @@ -1,30 +0,0 @@ -#!/usr/bin/env bash - -# -# Welcome to the SingleNamespace install mode demo -# -trap "trap - SIGTERM && kill -- -$$" SIGINT SIGTERM EXIT - -# enable 'SyntheticPermissions' feature -kubectl kustomize config/overlays/featuregate/synthetic-user-permissions | kubectl apply -f - - -# wait for operator-controller to become available -kubectl rollout status -n olmv1-system deployment/operator-controller-controller-manager - -# create install namespace -kubectl create ns argocd-system - -# give cluster extension group cluster admin privileges - all cluster extensions installer users will be cluster admin -bat --style=plain ${DEMO_RESOURCE_DIR}/synthetic-user-perms/cegroup-admin-binding.yaml - -# apply cluster role binding -kubectl apply -f ${DEMO_RESOURCE_DIR}/synthetic-user-perms/cegroup-admin-binding.yaml - -# install cluster extension - for now .spec.serviceAccount = "olm.synthetic-user" -bat --style=plain ${DEMO_RESOURCE_DIR}/synthetic-user-perms/argocd-clusterextension.yaml - -# apply cluster extension -kubectl apply -f ${DEMO_RESOURCE_DIR}/synthetic-user-perms/argocd-clusterextension.yaml - -# wait for cluster extension installation to succeed -kubectl wait --for=condition=Installed clusterextension/argocd-operator --timeout="60s" diff --git a/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go b/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go index 8e134a3429..026319e7dc 100644 --- a/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go +++ b/hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go @@ -44,8 +44,7 @@ const ( // ClusterExtensionSpec defines the desired state of ClusterExtension type ClusterExtensionSpec struct { // 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 + // It 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. @@ -57,20 +56,18 @@ type ClusterExtensionSpec struct { // [RFC 1123]: https://tools.ietf.org/html/rfc1123 // // +kubebuilder:validation:MaxLength:=63 - // - // + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="namespace is immutable" // +kubebuilder:validation:XValidation:rule="self.matches(\"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\")",message="namespace must be a valid DNS1123 label" // +kubebuilder:validation:Required Namespace string `json:"namespace"` - // serviceAccount is a reference to a ServiceAccount used to perform all interactions + // Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + // serviceAccount was a reference to the 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 optional. // - // +kubebuilder:validation:Required - ServiceAccount ServiceAccountReference `json:"serviceAccount"` + // +kubebuilder:validation: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. @@ -112,28 +109,15 @@ type SourceConfig struct { // When using the Catalog sourceType, the catalog field must also be set. // // +unionDiscriminator - // - // + // +kubebuilder:validation:Enum:="Catalog" // +kubebuilder:validation:Required SourceType string `json:"sourceType"` // catalog is used to configure how information is sourced from a catalog. // This field is required when sourceType is "Catalog", and forbidden otherwise. // - // - // This is the experimental description for Catalog - // - // - // - // No one should see this! - // - // // +optional Catalog *CatalogFilter `json:"catalog,omitempty"` - - // test is a required parameter - // - Test string `json:"test"` } // ClusterExtensionInstallConfig is a union which selects the clusterExtension installation config. @@ -341,8 +325,9 @@ type CatalogFilter struct { UpgradeConstraintPolicy UpgradeConstraintPolicy `json:"upgradeConstraintPolicy,omitempty"` } -// ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension. +// Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. type ServiceAccountReference struct { + // Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. // 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. diff --git a/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml b/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml index 9866f68bbe..49e687dcac 100644 --- a/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -57,6 +57,40 @@ spec: description: spec is an optional field that defines the desired state of the ClusterExtension. properties: + config: + description: |- + config contains optional configuration values applied during rendering of the + ClusterExtension's manifests. Values can be specified inline. + + config is optional. When not specified, the default configuration of the resolved bundle will be used. + properties: + configType: + description: |- + configType is a required reference to the type of configuration source. + + Allowed values are "Inline" + + When this field is set to "Inline", the cluster extension configuration is defined inline within the + ClusterExtension resource. + enum: + - Inline + type: string + inline: + description: |- + inline contains JSON or YAML values specified directly in the + ClusterExtension. + + inline must be set if configType is 'Inline'. + type: object + x-kubernetes-preserve-unknown-fields: true + required: + - configType + type: object + x-kubernetes-validations: + - message: inline is required when configType is Inline, and forbidden + otherwise + rule: 'has(self.configType) && self.configType == ''Inline'' ?has(self.inline) + : !has(self.inline)' install: description: |- install is an optional field used to configure the installation options @@ -112,8 +146,7 @@ spec: namespace: description: |- 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 + It 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. @@ -126,20 +159,20 @@ spec: maxLength: 63 type: string x-kubernetes-validations: + - message: namespace is immutable + rule: self == oldSelf - message: namespace must be a valid DNS1123 label rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") - - message: self == oldSelf - rule: namespace really is immutable serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -195,8 +228,6 @@ spec: description: |- catalog is used to configure how information is sourced from a catalog. This field is required when sourceType is "Catalog", and forbidden otherwise. - - This is the experimental description for Catalog properties: channels: description: |- @@ -450,14 +481,9 @@ spec: When using the Catalog sourceType, the catalog field must also be set. enum: - Catalog - - NotCatalog - type: string - test: - description: test is a required parameter type: string required: - sourceType - - test type: object x-kubernetes-validations: - message: catalog is required when sourceType is Catalog, and forbidden @@ -466,7 +492,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml b/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml index 8dcb4beed1..d274c42634 100644 --- a/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml @@ -112,8 +112,7 @@ spec: namespace: description: |- 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 + It 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. @@ -126,20 +125,20 @@ spec: maxLength: 63 type: string x-kubernetes-validations: + - message: namespace is immutable + rule: self == oldSelf - message: namespace must be a valid DNS1123 label rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") - - message: self == oldSelf - rule: namespace is immutable serviceAccount: description: |- - serviceAccount is a reference to a ServiceAccount used to perform all interactions + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -451,7 +450,6 @@ spec: type: string required: - sourceType - - test type: object x-kubernetes-validations: - message: catalog is required when sourceType is Catalog, and forbidden @@ -460,7 +458,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/helm/experimental.yaml b/helm/experimental.yaml index fd7d9702ef..5b363c637f 100644 --- a/helm/experimental.yaml +++ b/helm/experimental.yaml @@ -8,7 +8,6 @@ operatorControllerFeatures: - WebhookProviderCertManager - SingleOwnNamespaceInstallSupport - - PreflightPermissions - HelmChartSupport - BoxcutterRuntime 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 4cae796a6e..49e687dcac 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 @@ -146,8 +146,7 @@ spec: namespace: description: |- 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 + It 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. @@ -166,14 +165,14 @@ 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 + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -493,7 +492,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..d274c42634 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 @@ -112,8 +112,7 @@ spec: namespace: description: |- 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 + It 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. @@ -132,14 +131,14 @@ 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 + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -459,7 +458,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: diff --git a/helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml b/helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml deleted file mode 100644 index 84f221003c..0000000000 --- a/helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml +++ /dev/null @@ -1,75 +0,0 @@ -{{- if and .Values.options.operatorController.enabled (not (has "BoxcutterRuntime" .Values.operatorConrollerFeatures)) }} -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: operator-controller-manager-role - labels: - app.kubernetes.io/name: operator-controller - {{- include "olmv1.labels" . | nindent 4 }} - annotations: - {{- include "olmv1.annotations" . | nindent 4 }} -rules: - - apiGroups: - - "" - resources: - - serviceaccounts/token - verbs: - - create - - apiGroups: - - apiextensions.k8s.io - resources: - - customresourcedefinitions - verbs: - - get - - apiGroups: - - olm.operatorframework.io - resources: - - clustercatalogs - verbs: - - get - - list - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions - verbs: - - get - - list - - patch - - update - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/finalizers - verbs: - - update - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/status - verbs: - - patch - - update - - apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterrolebindings - - clusterroles - - rolebindings - - roles - verbs: - - list - - watch - {{- if .Values.options.openshift.enabled }} - - apiGroups: - - security.openshift.io - resources: - - securitycontextconstraints - resourceNames: - - privileged - verbs: - - use - {{- end }} -{{- end }} 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 a779301c77..5ffaf72b5b 100644 --- a/helm/olmv1/templates/rbac/clusterrolebinding-operator-controller-manager-rolebinding.yml +++ b/helm/olmv1/templates/rbac/clusterrolebinding-operator-controller-manager-rolebinding.yml @@ -16,11 +16,7 @@ metadata: roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole -{{- if has "BoxcutterRuntime" .Values.operatorControllerFeatures }} name: cluster-admin -{{- else }} - name: operator-controller-manager-role -{{- end }} subjects: - kind: ServiceAccount name: operator-controller-controller-manager diff --git a/helm/tilt.yaml b/helm/tilt.yaml index 367ab0c291..9076e6bcc7 100644 --- a/helm/tilt.yaml +++ b/helm/tilt.yaml @@ -15,7 +15,6 @@ options: operatorControllerFeatures: - WebhookProviderCertManager - SingleOwnNamespaceInstallSupport - - PreflightPermissions - HelmChartSupport catalogdFeatures: diff --git a/internal/operator-controller/action/restconfig.go b/internal/operator-controller/action/restconfig.go index 05e25f707d..a86449bfc1 100644 --- a/internal/operator-controller/action/restconfig.go +++ b/internal/operator-controller/action/restconfig.go @@ -2,73 +2,15 @@ package action import ( "context" - "fmt" - "net/http" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" - "k8s.io/client-go/transport" "sigs.k8s.io/controller-runtime/pkg/client" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" ) -const syntheticServiceAccountName = "olm.synthetic-user" - -// SyntheticUserRestConfigMapper returns an AuthConfigMapper that that impersonates synthetic users and groups for Object o. -// o is expected to be a ClusterExtension. If the service account defined in o is different from 'olm.synthetic-user', the -// defaultAuthMapper will be used -func SyntheticUserRestConfigMapper(defaultAuthMapper func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error)) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) { - return func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) { - cExt, err := validate(o, c) - if err != nil { - return nil, err - } - if cExt.Spec.ServiceAccount.Name != syntheticServiceAccountName { - return defaultAuthMapper(ctx, cExt, c) - } - cc := rest.CopyConfig(c) - cc.Wrap(func(rt http.RoundTripper) http.RoundTripper { - return transport.NewImpersonatingRoundTripper(authentication.SyntheticImpersonationConfig(*cExt), rt) - }) - return cc, nil - } -} - -// ServiceAccountRestConfigMapper returns an AuthConfigMapper scoped to the service account defined in o, which is expected to -// be a ClusterExtension -func ServiceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) { +// ClusterAdminRestConfigMapper returns a config with cluster-admin permissions +func ClusterAdminRestConfigMapper(adminConfig *rest.Config) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, + error) { return func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) { - cExt, err := validate(o, c) - if err != nil { - return nil, err - } - saConfig := rest.AnonymousClientConfig(c) - saConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper { - return &authentication.TokenInjectingRoundTripper{ - Tripper: rt, - TokenGetter: tokenGetter, - Key: types.NamespacedName{ - Name: cExt.Spec.ServiceAccount.Name, - Namespace: cExt.Spec.Namespace, - }, - } - }) - return saConfig, nil - } -} - -func validate(o client.Object, c *rest.Config) (*ocv1.ClusterExtension, error) { - if c == nil { - return nil, fmt.Errorf("rest config is nil") - } - if o == nil { - return nil, fmt.Errorf("object is nil") - } - cExt, ok := o.(*ocv1.ClusterExtension) - if !ok { - return nil, fmt.Errorf("object is not a ClusterExtension") + return rest.CopyConfig(adminConfig), nil } - return cExt, nil } diff --git a/internal/operator-controller/action/restconfig_test.go b/internal/operator-controller/action/restconfig_test.go index 4c9f786719..ae10748e59 100644 --- a/internal/operator-controller/action/restconfig_test.go +++ b/internal/operator-controller/action/restconfig_test.go @@ -1,177 +1,58 @@ package action_test -import ( - "context" - "errors" - "net/http" - "testing" - - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/client" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/action" - "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" -) - -func Test_ServiceAccountRestConfigMapper(t *testing.T) { - for _, tc := range []struct { - description string - obj client.Object - cfg *rest.Config - expectedError error - }{ - { - description: "return error if object is nil", - cfg: &rest.Config{}, - expectedError: errors.New("object is nil"), - }, { - description: "return error if cfg is nil", - obj: &ocv1.ClusterExtension{}, - expectedError: errors.New("rest config is nil"), - }, { - description: "return error if object is not a ClusterExtension", - obj: &corev1.Secret{}, - cfg: &rest.Config{}, - expectedError: errors.New("object is not a ClusterExtension"), - }, { - description: "succeeds if object is not a ClusterExtension", - obj: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-clusterextension", - }, - Spec: ocv1.ClusterExtensionSpec{ - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "my-service-account", - }, - Namespace: "my-namespace", - }, - }, - cfg: &rest.Config{}, - }, - } { - t.Run(tc.description, func(t *testing.T) { - tokenGetter := &authentication.TokenGetter{} - saMapper := action.ServiceAccountRestConfigMapper(tokenGetter) - actualCfg, err := saMapper(context.Background(), tc.obj, tc.cfg) - if tc.expectedError != nil { - require.Nil(t, actualCfg) - require.EqualError(t, err, tc.expectedError.Error()) - } else { - require.NoError(t, err) - transport, err := rest.TransportFor(actualCfg) - require.NoError(t, err) - require.NotNil(t, transport) - tokenInjectionRoundTripper, ok := transport.(*authentication.TokenInjectingRoundTripper) - require.True(t, ok) - require.Equal(t, tokenGetter, tokenInjectionRoundTripper.TokenGetter) - require.Equal(t, types.NamespacedName{Name: "my-service-account", Namespace: "my-namespace"}, tokenInjectionRoundTripper.Key) - } - }) - } -} - -func Test_SyntheticUserRestConfigMapper_Fails(t *testing.T) { - for _, tc := range []struct { - description string - obj client.Object - cfg *rest.Config - expectedError error - }{ - { - description: "return error if object is nil", - cfg: &rest.Config{}, - expectedError: errors.New("object is nil"), - }, { - description: "return error if cfg is nil", - obj: &ocv1.ClusterExtension{}, - expectedError: errors.New("rest config is nil"), - }, { - description: "return error if object is not a ClusterExtension", - obj: &corev1.Secret{}, - cfg: &rest.Config{}, - expectedError: errors.New("object is not a ClusterExtension"), - }, - } { - t.Run(tc.description, func(t *testing.T) { - tokenGetter := &authentication.TokenGetter{} - saMapper := action.ServiceAccountRestConfigMapper(tokenGetter) - actualCfg, err := saMapper(context.Background(), tc.obj, tc.cfg) - if tc.expectedError != nil { - require.Nil(t, actualCfg) - require.EqualError(t, err, tc.expectedError.Error()) - } else { - require.NoError(t, err) - transport, err := rest.TransportFor(actualCfg) - require.NoError(t, err) - require.NotNil(t, transport) - tokenInjectionRoundTripper, ok := transport.(*authentication.TokenInjectingRoundTripper) - require.True(t, ok) - require.Equal(t, tokenGetter, tokenInjectionRoundTripper.TokenGetter) - require.Equal(t, types.NamespacedName{Name: "my-service-account", Namespace: "my-namespace"}, tokenInjectionRoundTripper.Key) - } - }) - } -} -func Test_SyntheticUserRestConfigMapper_UsesDefaultConfigMapper(t *testing.T) { - isDefaultRequestMapperUsed := false - defaultServiceMapper := func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) { - isDefaultRequestMapperUsed = true - return c, nil - } - syntheticAuthServiceMapper := action.SyntheticUserRestConfigMapper(defaultServiceMapper) - obj := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-clusterextension", - }, - Spec: ocv1.ClusterExtensionSpec{ - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "my-service-account", - }, - Namespace: "my-namespace", - }, - } - actualCfg, err := syntheticAuthServiceMapper(context.Background(), obj, &rest.Config{}) - require.NoError(t, err) - require.NotNil(t, actualCfg) - require.True(t, isDefaultRequestMapperUsed) -} - -func Test_SyntheticUserRestConfigMapper_UsesSyntheticAuthMapper(t *testing.T) { - syntheticAuthServiceMapper := action.SyntheticUserRestConfigMapper(func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) { - return c, nil - }) - obj := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-clusterextension", - }, - Spec: ocv1.ClusterExtensionSpec{ - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "olm.synthetic-user", - }, - Namespace: "my-namespace", - }, - } - actualCfg, err := syntheticAuthServiceMapper(context.Background(), obj, &rest.Config{}) - require.NoError(t, err) - require.NotNil(t, actualCfg) - - // test that the impersonation headers are appropriately injected into the request - // by wrapping a fake round tripper around the returned configurations transport - // nolint:bodyclose - _, _ = actualCfg.WrapTransport(fakeRoundTripper(func(req *http.Request) (*http.Response, error) { - require.Equal(t, "olm:clusterextension:my-clusterextension", req.Header.Get("Impersonate-User")) - require.Equal(t, "olm:clusterextensions", req.Header.Get("Impersonate-Group")) - return &http.Response{}, nil - })).RoundTrip(&http.Request{}) -} - -type fakeRoundTripper func(req *http.Request) (*http.Response, error) - -func (f fakeRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) { - return f(request) -} +// func Test_ServiceAccountRestConfigMapper(t *testing.T) { +// for _, tc := range []struct { +// description string +// obj client.Object +// cfg *rest.Config +// expectedError error +// }{ +// { +// description: "return error if object is nil", +// cfg: &rest.Config{}, +// expectedError: errors.New("object is nil"), +// }, { +// description: "return error if cfg is nil", +// obj: &ocv1.ClusterExtension{}, +// expectedError: errors.New("rest config is nil"), +// }, { +// description: "return error if object is not a ClusterExtension", +// obj: &corev1.Secret{}, +// cfg: &rest.Config{}, +// expectedError: errors.New("object is not a ClusterExtension"), +// }, { +// description: "succeeds if object is not a ClusterExtension", +// obj: &ocv1.ClusterExtension{ +// ObjectMeta: metav1.ObjectMeta{ +// Name: "my-clusterextension", +// }, +// Spec: ocv1.ClusterExtensionSpec{ +// ServiceAccount: ocv1.ServiceAccountReference{ +// Name: "my-service-account", +// }, +// Namespace: "my-namespace", +// }, +// }, +// cfg: &rest.Config{}, +// }, +// } { +// t.Run(tc.description, func(t *testing.T) { +// tokenGetter := &authentication.TokenGetter{} +// saMapper := action.ServiceAccountRestConfigMapper(tokenGetter) +// actualCfg, err := saMapper(context.Background(), tc.obj, tc.cfg) +// if tc.expectedError != nil { +// require.Nil(t, actualCfg) +// require.EqualError(t, err, tc.expectedError.Error()) +// } else { +// require.NoError(t, err) +// transport, err := rest.TransportFor(actualCfg) +// require.NoError(t, err) +// require.NotNil(t, transport) +// tokenInjectionRoundTripper, ok := transport.(*authentication.TokenInjectingRoundTripper) +// require.True(t, ok) +// require.Equal(t, tokenGetter, tokenInjectionRoundTripper.TokenGetter) +// require.Equal(t, types.NamespacedName{Name: "my-service-account", Namespace: "my-namespace"}, tokenInjectionRoundTripper.Key) +// } +// }) +// } +// } diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 4e70268941..4573639b28 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "io/fs" - "slices" "strings" "helm.sh/helm/v3/pkg/action" @@ -16,7 +15,6 @@ import ( "helm.sh/helm/v3/pkg/postrender" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" - rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apimachyaml "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/klog/v2" @@ -26,7 +24,6 @@ import ( helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager" "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" @@ -60,7 +57,6 @@ func (h HelmReleaseToObjectsConverter) GetObjectsFromRelease(rel *release.Releas type Helm struct { ActionClientGetter helmclient.ActionClientGetter Preflights []Preflight - PreAuthorizer authorization.PreAuthorizer HelmChartProvider HelmChartProvider HelmReleaseToObjectsConverter HelmReleaseToObjectsConverterInterface @@ -68,40 +64,6 @@ type Helm struct { Watcher crcontroller.Controller } -// runPreAuthorizationChecks performs pre-authorization checks for a Helm release -// it renders a client-only release, checks permissions using the PreAuthorizer -// and returns an error if authorization fails or required permissions are missing -func (h *Helm) runPreAuthorizationChecks(ctx context.Context, ext *ocv1.ClusterExtension, chart *chart.Chart, values chartutil.Values, post postrender.PostRenderer) error { - tmplRel, err := h.renderClientOnlyRelease(ctx, ext, chart, values, post) - if err != nil { - return fmt.Errorf("error rendering content for pre-authorization checks: %w", err) - } - - missingRules, authErr := h.PreAuthorizer.PreAuthorize(ctx, ext, strings.NewReader(tmplRel.Manifest)) - - var preAuthErrors []error - - if len(missingRules) > 0 { - var missingRuleDescriptions []string - for _, policyRules := range missingRules { - for _, rule := range policyRules.MissingRules { - missingRuleDescriptions = append(missingRuleDescriptions, ruleDescription(policyRules.Namespace, rule)) - } - } - slices.Sort(missingRuleDescriptions) - // This phrase is explicitly checked by external testing - preAuthErrors = append(preAuthErrors, fmt.Errorf("service account requires the following permissions to manage cluster extension:\n %s", strings.Join(missingRuleDescriptions, "\n "))) - } - if authErr != nil { - preAuthErrors = append(preAuthErrors, fmt.Errorf("authorization evaluation error: %w", authErr)) - } - if len(preAuthErrors) > 0 { - // This phrase is explicitly checked by external testing - return fmt.Errorf("pre-authorization failed: %v", errors.Join(preAuthErrors...)) - } - return nil -} - func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) (bool, string, error) { chrt, err := h.buildHelmChart(contentFS, ext) if err != nil { @@ -113,14 +75,6 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte labels: objectLabels, } - if h.PreAuthorizer != nil { - err := h.runPreAuthorizationChecks(ctx, ext, chrt, values, post) - if err != nil { - // Return the pre-authorization error directly - return false, "", err - } - } - ac, err := h.ActionClientGetter.ActionClientFor(ctx, ext) if err != nil { return false, "", err @@ -214,34 +168,6 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char return h.HelmChartProvider.Get(bundleFS, ext) } -func (h *Helm) renderClientOnlyRelease(ctx context.Context, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, error) { - // We need to get a separate action client because our work below - // permanently modifies the underlying action.Configuration for ClientOnly mode. - ac, err := h.ActionClientGetter.ActionClientFor(ctx, ext) - if err != nil { - return nil, err - } - - isUpgrade := false - currentRelease, err := ac.Get(ext.GetName()) - if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) { - return nil, err - } - if currentRelease != nil { - isUpgrade = true - } - - return ac.Install(ext.GetName(), ext.Spec.Namespace, chrt, values, func(i *action.Install) error { - i.DryRun = true - i.ReleaseName = ext.GetName() - i.Replace = true - i.ClientOnly = true - i.IncludeCRDs = true - i.IsUpgrade = isUpgrade - return nil - }, helmclient.AppendInstallPostRenderer(post)) -} - func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, *release.Release, string, error) { currentRelease, err := cl.Get(ext.GetName()) if errors.Is(err, driver.ErrReleaseNotFound) { @@ -306,25 +232,3 @@ func (p *postrenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, erro } return &buf, nil } - -func ruleDescription(ns string, rule rbacv1.PolicyRule) string { - var sb strings.Builder - sb.WriteString(fmt.Sprintf("Namespace:%q", ns)) - - if len(rule.APIGroups) > 0 { - sb.WriteString(fmt.Sprintf(" APIGroups:[%s]", strings.Join(slices.Sorted(slices.Values(rule.APIGroups)), ","))) - } - if len(rule.Resources) > 0 { - sb.WriteString(fmt.Sprintf(" Resources:[%s]", strings.Join(slices.Sorted(slices.Values(rule.Resources)), ","))) - } - if len(rule.ResourceNames) > 0 { - sb.WriteString(fmt.Sprintf(" ResourceNames:[%s]", strings.Join(slices.Sorted(slices.Values(rule.ResourceNames)), ","))) - } - if len(rule.Verbs) > 0 { - sb.WriteString(fmt.Sprintf(" Verbs:[%s]", strings.Join(slices.Sorted(slices.Values(rule.Verbs)), ","))) - } - if len(rule.NonResourceURLs) > 0 { - sb.WriteString(fmt.Sprintf(" NonResourceURLs:[%s]", strings.Join(slices.Sorted(slices.Values(rule.NonResourceURLs)), ","))) - } - return sb.String() -} diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 8d07de9123..f57ae00aab 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -3,7 +3,6 @@ package applier_test import ( "context" "errors" - "io" "io/fs" "os" "testing" @@ -14,14 +13,12 @@ import ( "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" - rbacv1 "k8s.io/api/rbac/v1" "sigs.k8s.io/controller-runtime/pkg/client" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/applier" - "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager" cmcache "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager/cache" ) @@ -69,19 +66,6 @@ type mockPreflight struct { upgradeErr error } -type mockPreAuthorizer struct { - missingRules []authorization.ScopedPolicyRules - returnError error -} - -func (p *mockPreAuthorizer) PreAuthorize( - ctx context.Context, - ext *ocv1.ClusterExtension, - manifestReader io.Reader, -) ([]authorization.ScopedPolicyRules, error) { - return p.missingRules, p.returnError -} - func (mp *mockPreflight) Install(context.Context, []client.Object) error { return mp.installErr } @@ -196,33 +180,6 @@ spec: testCE = &ocv1.ClusterExtension{} testObjectLabels = map[string]string{"object": "label"} testStorageLabels = map[string]string{"storage": "label"} - errPreAuth = errors.New("problem running preauthorization") - missingRBAC = []authorization.ScopedPolicyRules{ - { - Namespace: "", - MissingRules: []rbacv1.PolicyRule{ - { - Verbs: []string{"list", "watch"}, - APIGroups: []string{""}, - Resources: []string{"services"}, - ResourceNames: []string(nil), - NonResourceURLs: []string(nil)}, - }, - }, - { - Namespace: "test-namespace", - MissingRules: []rbacv1.PolicyRule{ - { - Verbs: []string{"create"}, - APIGroups: []string{"*"}, - Resources: []string{"certificates"}}, - }, - }, - } - - errMissingRBAC = `pre-authorization failed: service account requires the following permissions to manage cluster extension: - Namespace:"" APIGroups:[] Resources:[services] Verbs:[list,watch] - Namespace:"test-namespace" APIGroups:[*] Resources:[certificates] Verbs:[create]` ) func TestApply_Base(t *testing.T) { @@ -344,142 +301,6 @@ func TestApply_Installation(t *testing.T) { }) } -func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { - t.Run("fails during dry-run installation", func(t *testing.T) { - mockAcg := &mockActionGetter{ - getClientErr: driver.ErrReleaseNotFound, - dryRunInstallErr: errors.New("failed attempting to dry-run install chart"), - } - helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - HelmChartProvider: DummyHelmChartProvider, - } - - installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) - require.Error(t, err) - require.ErrorContains(t, err, "attempting to dry-run install chart") - require.False(t, installSucceeded) - require.Empty(t, installStatus) - }) - - t.Run("fails during pre-flight installation", func(t *testing.T) { - mockAcg := &mockActionGetter{ - getClientErr: driver.ErrReleaseNotFound, - installErr: errors.New("failed installing chart"), - desiredRel: &release.Release{ - Info: &release.Info{Status: release.StatusDeployed}, - Manifest: validManifest, - }, - } - mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} - helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, - HelmChartProvider: DummyHelmChartProvider, - HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, - } - - installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) - require.Error(t, err) - require.ErrorContains(t, err, "install pre-flight check") - require.False(t, installSucceeded) - require.Empty(t, installStatus) - }) - - t.Run("fails during installation because of pre-authorization failure", func(t *testing.T) { - mockAcg := &mockActionGetter{ - getClientErr: driver.ErrReleaseNotFound, - desiredRel: &release.Release{ - Info: &release.Info{Status: release.StatusDeployed}, - Manifest: validManifest, - }, - } - helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth}, - HelmChartProvider: DummyHelmChartProvider, - } - // Use a ClusterExtension with valid Spec fields. - validCE := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "default", - }, - }, - } - installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, validCE, testObjectLabels, testStorageLabels) - require.Error(t, err) - require.ErrorContains(t, err, "problem running preauthorization") - require.False(t, installSucceeded) - require.Empty(t, installStatus) - }) - - t.Run("fails during installation due to missing RBAC rules", func(t *testing.T) { - mockAcg := &mockActionGetter{ - getClientErr: driver.ErrReleaseNotFound, - desiredRel: &release.Release{ - Info: &release.Info{Status: release.StatusDeployed}, - Manifest: validManifest, - }, - } - helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil}, - HelmChartProvider: DummyHelmChartProvider, - } - // Use a ClusterExtension with valid Spec fields. - validCE := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "default", - }, - }, - } - installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, validCE, testObjectLabels, testStorageLabels) - require.Error(t, err) - require.ErrorContains(t, err, errMissingRBAC) - require.False(t, installSucceeded) - require.Empty(t, installStatus) - }) - - t.Run("successful installation", func(t *testing.T) { - mockAcg := &mockActionGetter{ - getClientErr: driver.ErrReleaseNotFound, - desiredRel: &release.Release{ - Info: &release.Info{Status: release.StatusDeployed}, - Manifest: validManifest, - }, - } - helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, - HelmChartProvider: DummyHelmChartProvider, - HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, - Manager: &mockManagedContentCacheManager{ - cache: &mockManagedContentCache{}, - }, - } - - // Use a ClusterExtension with valid Spec fields. - validCE := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "default", - }, - }, - } - - installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, validCE, testObjectLabels, testStorageLabels) - require.NoError(t, err) - require.Empty(t, installStatus) - require.True(t, installSucceeded) - }) -} - func TestApply_Upgrade(t *testing.T) { testCurrentRelease := &release.Release{ Info: &release.Info{Status: release.StatusDeployed}, diff --git a/internal/operator-controller/authentication/synthetic.go b/internal/operator-controller/authentication/synthetic.go deleted file mode 100644 index 710f2885e8..0000000000 --- a/internal/operator-controller/authentication/synthetic.go +++ /dev/null @@ -1,26 +0,0 @@ -package authentication - -import ( - "fmt" - - "k8s.io/client-go/transport" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" -) - -func syntheticUserName(ext ocv1.ClusterExtension) string { - return fmt.Sprintf("olm:clusterextension:%s", ext.Name) -} - -func syntheticGroups(_ ocv1.ClusterExtension) []string { - return []string{ - "olm:clusterextensions", - } -} - -func SyntheticImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig { - return transport.ImpersonationConfig{ - UserName: syntheticUserName(ext), - Groups: syntheticGroups(ext), - } -} diff --git a/internal/operator-controller/authentication/synthetic_test.go b/internal/operator-controller/authentication/synthetic_test.go deleted file mode 100644 index 2e3f17a07b..0000000000 --- a/internal/operator-controller/authentication/synthetic_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package authentication_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/authentication" -) - -func TestSyntheticImpersonationConfig(t *testing.T) { - config := authentication.SyntheticImpersonationConfig(ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-ext", - }, - }) - require.Equal(t, "olm:clusterextension:my-ext", config.UserName) - require.Equal(t, []string{ - "olm:clusterextensions", - }, config.Groups) - require.Empty(t, config.UID) - require.Empty(t, config.Extra) -} diff --git a/internal/operator-controller/authorization/rbac.go b/internal/operator-controller/authorization/rbac.go deleted file mode 100644 index 357268615c..0000000000 --- a/internal/operator-controller/authorization/rbac.go +++ /dev/null @@ -1,671 +0,0 @@ -package authorization - -import ( - "context" - "errors" - "fmt" - "io" - "maps" - "regexp" - "slices" - "sort" - "strings" - - corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/sets" - apimachyaml "k8s.io/apimachinery/pkg/util/yaml" - "k8s.io/apiserver/pkg/authentication/user" - "k8s.io/apiserver/pkg/authorization/authorizer" - "k8s.io/apiserver/pkg/endpoints/request" - rbacinternal "k8s.io/kubernetes/pkg/apis/rbac" - rbacv1helpers "k8s.io/kubernetes/pkg/apis/rbac/v1" - rbacregistry "k8s.io/kubernetes/pkg/registry/rbac" - "k8s.io/kubernetes/pkg/registry/rbac/validation" - rbac "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" - "sigs.k8s.io/controller-runtime/pkg/client" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" -) - -type PreAuthorizer interface { - PreAuthorize(ctx context.Context, ext *ocv1.ClusterExtension, manifestReader io.Reader) ([]ScopedPolicyRules, error) -} - -type ScopedPolicyRules struct { - Namespace string - MissingRules []rbacv1.PolicyRule -} - -var objectVerbs = []string{"get", "patch", "update", "delete"} - -// Here we are splitting collection verbs based on required scope -// NB: this split is tightly coupled to the requirements of the contentmanager, specifically -// its need for cluster-scoped list/watch permissions. -// TODO: We are accepting this coupling for now, but plan to decouple -// TODO: link for above https://github.com/operator-framework/operator-controller/issues/1911 -var namespacedCollectionVerbs = []string{"create"} -var clusterCollectionVerbs = []string{"list", "watch"} - -type rbacPreAuthorizer struct { - authorizer authorizer.Authorizer - ruleResolver validation.AuthorizationRuleResolver - restMapper meta.RESTMapper -} - -func NewRBACPreAuthorizer(cl client.Client) PreAuthorizer { - return &rbacPreAuthorizer{ - authorizer: newRBACAuthorizer(cl), - ruleResolver: newRBACRulesResolver(cl), - restMapper: cl.RESTMapper(), - } -} - -// PreAuthorize validates whether the current user/request satisfies the necessary permissions -// as defined by the RBAC policy. It examines the user’s roles, resource identifiers, and -// the intended action to determine if the operation is allowed. -// -// Return Value: -// - nil: indicates that the authorization check passed and the operation is permitted. -// - non-nil error: indicates that an error occurred during the permission evaluation process -// (for example, a failure decoding the manifest or other internal issues). If the evaluation -// completes successfully but identifies missing rules, then a nil error is returned along with -// the list (or slice) of missing rules. Note that in some cases the error may encapsulate multiple -// evaluation failures -func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, ext *ocv1.ClusterExtension, manifestReader io.Reader) ([]ScopedPolicyRules, error) { - dm, err := a.decodeManifest(manifestReader) - if err != nil { - return nil, err - } - manifestManager := &user.DefaultInfo{Name: fmt.Sprintf("system:serviceaccount:%s:%s", ext.Spec.Namespace, ext.Spec.ServiceAccount.Name)} - attributesRecords := dm.asAuthorizationAttributesRecordsForUser(manifestManager, ext) - - var preAuthEvaluationErrors []error - missingRules, err := a.authorizeAttributesRecords(ctx, attributesRecords) - if err != nil { - preAuthEvaluationErrors = append(preAuthEvaluationErrors, err) - } - - ec := a.escalationCheckerFor(dm) - - var parseErrors []error - for _, obj := range dm.rbacObjects() { - if err := ec.checkEscalation(ctx, manifestManager, obj); err != nil { - result, err := parseEscalationErrorForMissingRules(err) - missingRules[obj.GetNamespace()] = append(missingRules[obj.GetNamespace()], result.MissingRules...) - preAuthEvaluationErrors = append(preAuthEvaluationErrors, result.ResolutionErrors) - parseErrors = append(parseErrors, err) - } - } - allMissingPolicyRules := make([]ScopedPolicyRules, 0, len(missingRules)) - - for ns, nsMissingRules := range missingRules { - // NOTE: Although CompactRules is defined to return an error, its current implementation - // never produces a non-nil error. This is because all operations within the function are - // designed to succeed under current conditions. In the future, if more complex rule validations - // are introduced, this behavior may change and proper error handling will be required. - if compactMissingRules, err := validation.CompactRules(nsMissingRules); err == nil { - missingRules[ns] = compactMissingRules - } - - missingRulesWithDeduplicatedVerbs := make([]rbacv1.PolicyRule, 0, len(missingRules[ns])) - for _, rule := range missingRules[ns] { - verbSet := sets.New[string](rule.Verbs...) - if verbSet.Has("*") { - rule.Verbs = []string{"*"} - } else { - rule.Verbs = sets.List(verbSet) - } - missingRulesWithDeduplicatedVerbs = append(missingRulesWithDeduplicatedVerbs, rule) - } - - sortableRules := rbacv1helpers.SortableRuleSlice(missingRulesWithDeduplicatedVerbs) - - sort.Sort(sortableRules) - allMissingPolicyRules = append(allMissingPolicyRules, ScopedPolicyRules{Namespace: ns, MissingRules: sortableRules}) - } - - // sort allMissingPolicyRules alphabetically by namespace - slices.SortFunc(allMissingPolicyRules, func(a, b ScopedPolicyRules) int { - return strings.Compare(a.Namespace, b.Namespace) - }) - - var errs []error - if parseErr := errors.Join(parseErrors...); parseErr != nil { - errs = append(errs, fmt.Errorf("failed to parse escalation check error strings: %v", parseErr)) - } - if preAuthEvaluationErrors := errors.Join(preAuthEvaluationErrors...); preAuthEvaluationErrors != nil { - errs = append(errs, fmt.Errorf("failed to resolve or evaluate permissions: %v", preAuthEvaluationErrors)) - } - if len(errs) > 0 { - return allMissingPolicyRules, fmt.Errorf("missing rules may be incomplete: %w", errors.Join(errs...)) - } - return allMissingPolicyRules, nil -} - -func (a *rbacPreAuthorizer) escalationCheckerFor(dm *decodedManifest) escalationChecker { - ec := escalationChecker{ - authorizer: a.authorizer, - ruleResolver: a.ruleResolver, - extraClusterRoles: dm.clusterRoles, - extraRoles: dm.roles, - } - return ec -} - -func (a *rbacPreAuthorizer) decodeManifest(manifestReader io.Reader) (*decodedManifest, error) { - dm := &decodedManifest{ - gvrs: map[schema.GroupVersionResource][]types.NamespacedName{}, - clusterRoles: map[client.ObjectKey]rbacv1.ClusterRole{}, - roles: map[client.ObjectKey]rbacv1.Role{}, - clusterRoleBindings: map[client.ObjectKey]rbacv1.ClusterRoleBinding{}, - roleBindings: map[client.ObjectKey]rbacv1.RoleBinding{}, - } - var ( - i int - errs []error - decoder = apimachyaml.NewYAMLOrJSONDecoder(manifestReader, 1024) - ) - for { - var uObj unstructured.Unstructured - err := decoder.Decode(&uObj) - if errors.Is(err, io.EOF) { - break - } - if err != nil { - errs = append(errs, fmt.Errorf("could not decode object %d in manifest: %w", i, err)) - continue - } - gvk := uObj.GroupVersionKind() - restMapping, err := a.restMapper.RESTMapping(gvk.GroupKind(), gvk.Version) - if err != nil { - var objName string - if name := uObj.GetName(); name != "" { - objName = fmt.Sprintf(" (name: %s)", name) - } - - errs = append( - errs, - fmt.Errorf("could not get REST mapping for object %d in manifest with GVK %s%s: %w", i, gvk, objName, err), - ) - continue - } - - gvr := restMapping.Resource - dm.gvrs[gvr] = append(dm.gvrs[gvr], client.ObjectKeyFromObject(&uObj)) - - switch restMapping.Resource.GroupResource() { - case schema.GroupResource{Group: rbacv1.GroupName, Resource: "clusterroles"}: - obj := &rbacv1.ClusterRole{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil { - errs = append(errs, fmt.Errorf("could not decode object %d in manifest as ClusterRole: %w", i, err)) - continue - } - dm.clusterRoles[client.ObjectKeyFromObject(obj)] = *obj - case schema.GroupResource{Group: rbacv1.GroupName, Resource: "clusterrolebindings"}: - obj := &rbacv1.ClusterRoleBinding{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil { - errs = append(errs, fmt.Errorf("could not decode object %d in manifest as ClusterRoleBinding: %w", i, err)) - continue - } - dm.clusterRoleBindings[client.ObjectKeyFromObject(obj)] = *obj - case schema.GroupResource{Group: rbacv1.GroupName, Resource: "roles"}: - obj := &rbacv1.Role{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil { - errs = append(errs, fmt.Errorf("could not decode object %d in manifest as Role: %w", i, err)) - continue - } - dm.roles[client.ObjectKeyFromObject(obj)] = *obj - case schema.GroupResource{Group: rbacv1.GroupName, Resource: "rolebindings"}: - obj := &rbacv1.RoleBinding{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil { - errs = append(errs, fmt.Errorf("could not decode object %d in manifest as RoleBinding: %w", i, err)) - continue - } - dm.roleBindings[client.ObjectKeyFromObject(obj)] = *obj - } - } - if len(errs) > 0 { - return nil, errors.Join(errs...) - } - return dm, nil -} - -func (a *rbacPreAuthorizer) authorizeAttributesRecords(ctx context.Context, attributesRecords []authorizer.AttributesRecord) (map[string][]rbacv1.PolicyRule, error) { - var ( - missingRules = map[string][]rbacv1.PolicyRule{} - errs []error - ) - for _, ar := range attributesRecords { - allow, err := a.attributesAllowed(ctx, ar) - if err != nil { - errs = append(errs, err) - continue - } - if !allow { - missingRules[ar.Namespace] = append(missingRules[ar.Namespace], policyRuleFromAttributesRecord(ar)) - } - } - return missingRules, errors.Join(errs...) -} - -func (a *rbacPreAuthorizer) attributesAllowed(ctx context.Context, attributesRecord authorizer.AttributesRecord) (bool, error) { - decision, reason, err := a.authorizer.Authorize(ctx, attributesRecord) - if err != nil { - if reason != "" { - return false, fmt.Errorf("%s: %w", reason, err) - } - return false, err - } - return decision == authorizer.DecisionAllow, nil -} - -func policyRuleFromAttributesRecord(attributesRecord authorizer.AttributesRecord) rbacv1.PolicyRule { - pr := rbacv1.PolicyRule{} - if attributesRecord.Verb != "" { - pr.Verbs = []string{attributesRecord.Verb} - } - if !attributesRecord.ResourceRequest { - pr.NonResourceURLs = []string{attributesRecord.Path} - return pr - } - - pr.APIGroups = []string{attributesRecord.APIGroup} - if attributesRecord.Name != "" { - pr.ResourceNames = []string{attributesRecord.Name} - } - - r := attributesRecord.Resource - if attributesRecord.Subresource != "" { - r += "/" + attributesRecord.Subresource - } - pr.Resources = []string{r} - - return pr -} - -type decodedManifest struct { - gvrs map[schema.GroupVersionResource][]types.NamespacedName - clusterRoles map[client.ObjectKey]rbacv1.ClusterRole - roles map[client.ObjectKey]rbacv1.Role - clusterRoleBindings map[client.ObjectKey]rbacv1.ClusterRoleBinding - roleBindings map[client.ObjectKey]rbacv1.RoleBinding -} - -func (dm *decodedManifest) rbacObjects() []client.Object { - objects := make([]client.Object, 0, len(dm.clusterRoles)+len(dm.roles)+len(dm.clusterRoleBindings)+len(dm.roleBindings)) - for obj := range maps.Values(dm.clusterRoles) { - objects = append(objects, &obj) - } - for obj := range maps.Values(dm.roles) { - objects = append(objects, &obj) - } - for obj := range maps.Values(dm.clusterRoleBindings) { - objects = append(objects, &obj) - } - for obj := range maps.Values(dm.roleBindings) { - objects = append(objects, &obj) - } - return objects -} - -func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info, ext *ocv1.ClusterExtension) []authorizer.AttributesRecord { - var attributeRecords []authorizer.AttributesRecord - - for gvr, keys := range dm.gvrs { - namespaces := sets.New[string]() - for _, k := range keys { - namespaces.Insert(k.Namespace) - // generate records for object-specific verbs (get, update, patch, delete) in their respective namespaces - for _, v := range objectVerbs { - attributeRecords = append(attributeRecords, authorizer.AttributesRecord{ - User: manifestManager, - Namespace: k.Namespace, - Name: k.Name, - APIGroup: gvr.Group, - APIVersion: gvr.Version, - Resource: gvr.Resource, - ResourceRequest: true, - Verb: v, - }) - } - } - // generate records for namespaced collection verbs (create) for each relevant namespace - for _, ns := range sets.List(namespaces) { - for _, v := range namespacedCollectionVerbs { - attributeRecords = append(attributeRecords, authorizer.AttributesRecord{ - User: manifestManager, - Namespace: ns, - APIGroup: gvr.Group, - APIVersion: gvr.Version, - Resource: gvr.Resource, - ResourceRequest: true, - Verb: v, - }) - } - } - // generate records for cluster-scoped collection verbs (list, watch) required by contentmanager - for _, v := range clusterCollectionVerbs { - attributeRecords = append(attributeRecords, authorizer.AttributesRecord{ - User: manifestManager, - Namespace: corev1.NamespaceAll, // check cluster scope - APIGroup: gvr.Group, - APIVersion: gvr.Version, - Resource: gvr.Resource, - ResourceRequest: true, - Verb: v, - }) - } - - for _, verb := range []string{"update"} { - attributeRecords = append(attributeRecords, authorizer.AttributesRecord{ - User: manifestManager, - Name: ext.Name, - APIGroup: ext.GroupVersionKind().Group, - APIVersion: ext.GroupVersionKind().Version, - Resource: "clusterextensions/finalizers", - ResourceRequest: true, - Verb: verb, - }) - } - } - return attributeRecords -} - -func newRBACAuthorizer(cl client.Client) authorizer.Authorizer { - rg := &rbacGetter{cl: cl} - return rbac.New(rg, rg, rg, rg) -} - -type rbacGetter struct { - cl client.Client -} - -func (r rbacGetter) ListClusterRoleBindings(ctx context.Context) ([]*rbacv1.ClusterRoleBinding, error) { - var clusterRoleBindingsList rbacv1.ClusterRoleBindingList - if err := r.cl.List(ctx, &clusterRoleBindingsList); err != nil { - return nil, err - } - return toPtrSlice(clusterRoleBindingsList.Items), nil -} - -func (r rbacGetter) GetClusterRole(ctx context.Context, name string) (*rbacv1.ClusterRole, error) { - var clusterRole rbacv1.ClusterRole - if err := r.cl.Get(ctx, client.ObjectKey{Name: name}, &clusterRole); err != nil { - return nil, err - } - return &clusterRole, nil -} - -func (r rbacGetter) ListRoleBindings(ctx context.Context, namespace string) ([]*rbacv1.RoleBinding, error) { - var roleBindingsList rbacv1.RoleBindingList - if err := r.cl.List(ctx, &roleBindingsList, client.InNamespace(namespace)); err != nil { - return nil, err - } - return toPtrSlice(roleBindingsList.Items), nil -} - -func (r rbacGetter) GetRole(ctx context.Context, namespace, name string) (*rbacv1.Role, error) { - var role rbacv1.Role - if err := r.cl.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, &role); err != nil { - return nil, err - } - return &role, nil -} - -func newRBACRulesResolver(cl client.Client) validation.AuthorizationRuleResolver { - rg := &rbacGetter{cl: cl} - return validation.NewDefaultRuleResolver(rg, rg, rg, rg) -} - -type escalationChecker struct { - authorizer authorizer.Authorizer - ruleResolver validation.AuthorizationRuleResolver - extraRoles map[types.NamespacedName]rbacv1.Role - extraClusterRoles map[types.NamespacedName]rbacv1.ClusterRole -} - -func (ec *escalationChecker) checkEscalation(ctx context.Context, manifestManager user.Info, obj client.Object) error { - ctx = request.WithUser(request.WithNamespace(ctx, obj.GetNamespace()), manifestManager) - switch v := obj.(type) { - case *rbacv1.Role: - ctx = request.WithRequestInfo(ctx, &request.RequestInfo{APIGroup: rbacv1.GroupName, Resource: "roles", IsResourceRequest: true}) - return ec.checkRoleEscalation(ctx, v) - case *rbacv1.RoleBinding: - ctx = request.WithRequestInfo(ctx, &request.RequestInfo{APIGroup: rbacv1.GroupName, Resource: "rolebindings", IsResourceRequest: true}) - return ec.checkRoleBindingEscalation(ctx, v) - case *rbacv1.ClusterRole: - ctx = request.WithRequestInfo(ctx, &request.RequestInfo{APIGroup: rbacv1.GroupName, Resource: "clusterroles", IsResourceRequest: true}) - return ec.checkClusterRoleEscalation(ctx, v) - case *rbacv1.ClusterRoleBinding: - ctx = request.WithRequestInfo(ctx, &request.RequestInfo{APIGroup: rbacv1.GroupName, Resource: "clusterrolebindings", IsResourceRequest: true}) - return ec.checkClusterRoleBindingEscalation(ctx, v) - default: - return fmt.Errorf("unknown object type %T", v) - } -} - -func (ec *escalationChecker) checkClusterRoleEscalation(ctx context.Context, clusterRole *rbacv1.ClusterRole) error { - if rbacregistry.EscalationAllowed(ctx) || rbacregistry.RoleEscalationAuthorized(ctx, ec.authorizer) { - return nil - } - - // to set the aggregation rule, since it can gather anything, requires * on *.* - if hasAggregationRule(clusterRole) { - if err := validation.ConfirmNoEscalation(ctx, ec.ruleResolver, fullAuthority); err != nil { - return fmt.Errorf("must have cluster-admin privileges to use an aggregationRule: %w", err) - } - } - - if err := validation.ConfirmNoEscalation(ctx, ec.ruleResolver, clusterRole.Rules); err != nil { - return err - } - return nil -} - -func (ec *escalationChecker) checkClusterRoleBindingEscalation(ctx context.Context, clusterRoleBinding *rbacv1.ClusterRoleBinding) error { - if rbacregistry.EscalationAllowed(ctx) { - return nil - } - - roleRef := rbacinternal.RoleRef{} - err := rbacv1helpers.Convert_v1_RoleRef_To_rbac_RoleRef(&clusterRoleBinding.RoleRef, &roleRef, nil) - if err != nil { - return err - } - - if rbacregistry.BindingAuthorized(ctx, roleRef, metav1.NamespaceNone, ec.authorizer) { - return nil - } - - rules, err := ec.ruleResolver.GetRoleReferenceRules(ctx, clusterRoleBinding.RoleRef, metav1.NamespaceNone) - if err != nil && !apierrors.IsNotFound(err) { - return err - } - - if clusterRoleBinding.RoleRef.Kind == "ClusterRole" { - if manifestClusterRole, ok := ec.extraClusterRoles[types.NamespacedName{Name: clusterRoleBinding.RoleRef.Name}]; ok { - rules = append(rules, manifestClusterRole.Rules...) - } - } - - if err := validation.ConfirmNoEscalation(ctx, ec.ruleResolver, rules); err != nil { - return err - } - return nil -} - -func (ec *escalationChecker) checkRoleEscalation(ctx context.Context, role *rbacv1.Role) error { - if rbacregistry.EscalationAllowed(ctx) || rbacregistry.RoleEscalationAuthorized(ctx, ec.authorizer) { - return nil - } - - rules := role.Rules - if err := validation.ConfirmNoEscalation(ctx, ec.ruleResolver, rules); err != nil { - return err - } - return nil -} - -func (ec *escalationChecker) checkRoleBindingEscalation(ctx context.Context, roleBinding *rbacv1.RoleBinding) error { - if rbacregistry.EscalationAllowed(ctx) { - return nil - } - - roleRef := rbacinternal.RoleRef{} - err := rbacv1helpers.Convert_v1_RoleRef_To_rbac_RoleRef(&roleBinding.RoleRef, &roleRef, nil) - if err != nil { - return err - } - if rbacregistry.BindingAuthorized(ctx, roleRef, roleBinding.Namespace, ec.authorizer) { - return nil - } - - rules, err := ec.ruleResolver.GetRoleReferenceRules(ctx, roleBinding.RoleRef, roleBinding.Namespace) - if err != nil && !apierrors.IsNotFound(err) { - return err - } - - switch roleRef.Kind { - case "ClusterRole": - if manifestClusterRole, ok := ec.extraClusterRoles[types.NamespacedName{Name: roleBinding.RoleRef.Name}]; ok { - rules = append(rules, manifestClusterRole.Rules...) - } - case "Role": - if manifestRole, ok := ec.extraRoles[types.NamespacedName{Namespace: roleBinding.Namespace, Name: roleBinding.RoleRef.Name}]; ok { - rules = append(rules, manifestRole.Rules...) - } - } - - if err := validation.ConfirmNoEscalation(ctx, ec.ruleResolver, rules); err != nil { - return err - } - return nil -} - -var fullAuthority = []rbacv1.PolicyRule{ - {Verbs: []string{"*"}, APIGroups: []string{"*"}, Resources: []string{"*"}}, - {Verbs: []string{"*"}, NonResourceURLs: []string{"*"}}, -} - -var ( - errRegex = regexp.MustCompile(`(?s)^user ".*" \(groups=.*\) is attempting to grant RBAC permissions not currently held:\n([^;]+)(?:; resolution errors: (.*))?$`) - ruleRegex = regexp.MustCompile(`{([^}]*)}`) - itemRegex = regexp.MustCompile(`"[^"]*"`) -) - -type parseResult struct { - MissingRules []rbacv1.PolicyRule - ResolutionErrors error -} - -// TODO: Investigate replacing this regex parsing with structured error handling once there are -// -// structured RBAC errors introduced by https://github.com/kubernetes/kubernetes/pull/130955. -// -// parseEscalationErrorForMissingRules attempts to extract specific RBAC permissions -// that were denied due to escalation prevention from a given error's text. -// It returns the list of extracted PolicyRules and an error detailing the escalation attempt -// and any resolution errors found. -// Note: If parsing is successful, the returned error is derived from the *input* error's -// message, not an error encountered during the parsing process itself. If parsing fails due to an unexpected -// error format, a distinct parsing error is returned. -func parseEscalationErrorForMissingRules(ecError error) (*parseResult, error) { - var ( - result = &parseResult{} - parseErrors []error - ) - - // errRegex captures the missing permissions and optionally resolution errors from an escalation error message - // Group 1: The list of missing permissions - // Group 2: Optional resolution errors - errString := ecError.Error() - errMatches := errRegex.FindStringSubmatch(errString) // Use FindStringSubmatch for single match expected - - // Check if the main error message pattern was matched and captured the required groups - // We expect at least 3 elements: full match, missing permissions, resolution errors (can be empty) - if len(errMatches) != 3 { - // The error format doesn't match the expected pattern for escalation errors - return &parseResult{}, fmt.Errorf("unexpected format of escalation check error string: %q", errString) - } - missingPermissionsStr := errMatches[1] - if resolutionErrorsStr := errMatches[2]; resolutionErrorsStr != "" { - result.ResolutionErrors = errors.New(resolutionErrorsStr) - } - - // Extract permissions using permRegex from the captured permissions string (Group 1) - for _, rule := range ruleRegex.FindAllString(missingPermissionsStr, -1) { - pr, err := parseCompactRuleString(rule) - if err != nil { - parseErrors = append(parseErrors, err) - continue - } - result.MissingRules = append(result.MissingRules, *pr) - } - // Return the extracted permissions and the constructed error message - return result, errors.Join(parseErrors...) -} - -func parseCompactRuleString(rule string) (*rbacv1.PolicyRule, error) { - var fields []string - if ruleText := rule[1 : len(rule)-1]; ruleText != "" { - fields = mapSlice(strings.Split(ruleText, ","), func(in string) string { - return strings.TrimSpace(in) - }) - } - var pr rbacv1.PolicyRule - for _, item := range fields { - field, valuesStr, ok := strings.Cut(item, ":") - if !ok { - return nil, fmt.Errorf("unexpected item %q: expected :[...]", item) - } - values := mapSlice(itemRegex.FindAllString(valuesStr, -1), func(in string) string { - return strings.Trim(in, `"`) - }) - switch field { - case "APIGroups": - pr.APIGroups = values - case "Resources": - pr.Resources = values - case "ResourceNames": - pr.ResourceNames = values - case "NonResourceURLs": - pr.NonResourceURLs = values - case "Verbs": - pr.Verbs = values - default: - return nil, fmt.Errorf("unexpected item %q: unknown field: %q", item, field) - } - } - return &pr, nil -} - -func hasAggregationRule(clusterRole *rbacv1.ClusterRole) bool { - // Currently, an aggregation rule is considered present only if it has one or more selectors. - // An empty slice of ClusterRoleSelectors means no selectors were provided, - // which does NOT imply "match all." - return clusterRole.AggregationRule != nil && len(clusterRole.AggregationRule.ClusterRoleSelectors) > 0 -} - -func mapSlice[I, O any](in []I, f func(I) O) []O { - out := make([]O, len(in)) - for i := range in { - out[i] = f(in[i]) - } - return out -} - -func toPtrSlice[V any](in []V) []*V { - out := make([]*V, len(in)) - for i := range in { - out[i] = &in[i] - } - return out -} diff --git a/internal/operator-controller/authorization/rbac_test.go b/internal/operator-controller/authorization/rbac_test.go deleted file mode 100644 index 8e3d47687d..0000000000 --- a/internal/operator-controller/authorization/rbac_test.go +++ /dev/null @@ -1,748 +0,0 @@ -package authorization - -import ( - "context" - "errors" - "fmt" - "strings" - "testing" - - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/api/meta/testrestmapper" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apiserver/pkg/authentication/user" - "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/kubernetes/pkg/registry/rbac/validation" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" -) - -var ( - testManifest = `apiVersion: v1 -kind: Service -metadata: - name: test-service - namespace: test-namespace -spec: - clusterIP: None ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: test-extension-role - namespace: test-namespace -rules: -- apiGroups: ["*"] - resources: [serviceaccounts] - verbs: [watch] -- apiGroups: ["*"] - resources: [certificates] - verbs: [create] ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: test-extension-binding - namespace: test-namespace -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: test-extension-role -subjects: -- kind: ServiceAccount - name: test-serviceaccount - namespace: test-namespace - ` - - testManifestMultiNamespace = `apiVersion: v1 -kind: Service -metadata: - name: test-service - namespace: test-namespace -spec: - clusterIP: None ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: test-extension-role - namespace: test-namespace -rules: -- apiGroups: ["*"] - resources: [serviceaccounts] - verbs: [watch] -- apiGroups: ["*"] - resources: [certificates] - verbs: [create] ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: test-extension-binding - namespace: test-namespace -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: test-extension-role -subjects: -- kind: ServiceAccount - name: test-serviceaccount - namespace: test-namespace ---- -kind: Service -metadata: - name: test-service - namespace: a-test-namespace -spec: - clusterIP: None ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: test-extension-role - namespace: a-test-namespace -rules: -- apiGroups: ["*"] - resources: [serviceaccounts] - verbs: [watch] -- apiGroups: ["*"] - resources: [certificates] - verbs: [create] ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: test-extension-binding - namespace: a-test-namespace -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: test-extension-role -subjects: -- kind: ServiceAccount - name: test-serviceaccount - namespace: a-test-namespace - ` - - saName = "test-serviceaccount" - ns = "test-namespace" - exampleClusterExtension = ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-extension"}, - Spec: ocv1.ClusterExtensionSpec{ - Namespace: ns, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: saName, - }, - }, - } - - objects = []client.Object{ - &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-namespace", - }, - }, - &rbacv1.ClusterRoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "admin-clusterrole-binding", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - Name: saName, - Namespace: ns, - }, - }, - RoleRef: rbacv1.RoleRef{ - Name: "admin-clusterrole", - Kind: "ClusterRole", - }, - }, - &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-serviceaccount", - Namespace: "test-namespace", - }, - }, - } - - privilegedClusterRole = &rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "admin-clusterrole", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"*"}, - Resources: []string{"*"}, - Verbs: []string{"*"}, - }, - }, - } - - limitedClusterRole = &rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "admin-clusterrole", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{""}, - Verbs: []string{""}, - }, - }, - } - - escalatingClusterRole = &rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "admin-clusterrole", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"*"}, - Resources: []string{"serviceaccounts", "services", "clusterextensions/finalizers"}, - Verbs: []string{"*"}, - }, - { - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles", "clusterroles", "rolebindings", "clusterrolebindings"}, - Verbs: []string{"get", "patch", "watch", "list", "create", "update", "delete", "escalate", "bind"}, - }, - }, - } - - expectedSingleNamespaceMissingRules = []ScopedPolicyRules{ - { - Namespace: "", - MissingRules: []rbacv1.PolicyRule{ - { - Verbs: []string{"list", "watch"}, - APIGroups: []string{""}, - Resources: []string{"services"}, - ResourceNames: []string(nil), - NonResourceURLs: []string(nil)}, - { - Verbs: []string{"list", "watch"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}, - ResourceNames: []string(nil), - NonResourceURLs: []string(nil)}, - { - Verbs: []string{"list", "watch"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}, - ResourceNames: []string(nil), - NonResourceURLs: []string(nil)}, - { - Verbs: []string{"update"}, - APIGroups: []string{""}, - Resources: []string{"clusterextensions/finalizers"}, - ResourceNames: []string{"test-cluster-extension"}, - NonResourceURLs: []string(nil), - }, - }, - }, - { - Namespace: "test-namespace", - MissingRules: []rbacv1.PolicyRule{ - { - Verbs: []string{"create"}, - APIGroups: []string{"*"}, - Resources: []string{"certificates"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{""}, - Resources: []string{"services"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{""}, - Resources: []string{"services"}, - ResourceNames: []string{"test-service"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}, - ResourceNames: []string{"test-extension-binding"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}, - ResourceNames: []string{"test-extension-role"}}, - { - Verbs: []string{"watch"}, - APIGroups: []string{"*"}, - Resources: []string{"serviceaccounts"}, - }, - }, - }, - } - - expectedMultiNamespaceMissingRules = []ScopedPolicyRules{ - { - Namespace: "", - MissingRules: []rbacv1.PolicyRule{ - { - Verbs: []string{"list", "watch"}, - APIGroups: []string{""}, - Resources: []string{"services"}, - ResourceNames: []string(nil), - NonResourceURLs: []string(nil)}, - { - Verbs: []string{"list", "watch"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}, - ResourceNames: []string(nil), - NonResourceURLs: []string(nil)}, - { - Verbs: []string{"list", "watch"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}, - ResourceNames: []string(nil), - NonResourceURLs: []string(nil)}, - { - Verbs: []string{"update"}, - APIGroups: []string{""}, - Resources: []string{"clusterextensions/finalizers"}, - ResourceNames: []string{"test-cluster-extension"}, - NonResourceURLs: []string(nil), - }, - }, - }, - { - Namespace: "a-test-namespace", - MissingRules: []rbacv1.PolicyRule{ - { - Verbs: []string{"create"}, - APIGroups: []string{"*"}, - Resources: []string{"certificates"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{""}, - Resources: []string{"services"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{""}, - Resources: []string{"services"}, - ResourceNames: []string{"test-service"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}, - ResourceNames: []string{"test-extension-binding"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}, - ResourceNames: []string{"test-extension-role"}}, - { - Verbs: []string{"watch"}, - APIGroups: []string{"*"}, - Resources: []string{"serviceaccounts"}, - }, - }, - }, - { - Namespace: "test-namespace", - MissingRules: []rbacv1.PolicyRule{ - { - Verbs: []string{"create"}, - APIGroups: []string{"*"}, - Resources: []string{"certificates"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{""}, - Resources: []string{"services"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}}, - { - Verbs: []string{"create"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{""}, - Resources: []string{"services"}, - ResourceNames: []string{"test-service"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"rolebindings"}, - ResourceNames: []string{"test-extension-binding"}}, - { - Verbs: []string{"delete", "get", "patch", "update"}, - APIGroups: []string{"rbac.authorization.k8s.io"}, - Resources: []string{"roles"}, - ResourceNames: []string{"test-extension-role"}}, - { - Verbs: []string{"watch"}, - APIGroups: []string{"*"}, - Resources: []string{"serviceaccounts"}, - }, - }, - }, - } -) - -func setupFakeClient(role client.Object) client.Client { - s := runtime.NewScheme() - _ = corev1.AddToScheme(s) - _ = rbacv1.AddToScheme(s) - restMapper := testrestmapper.TestOnlyStaticRESTMapper(s) - fakeClientBuilder := fake.NewClientBuilder().WithObjects(append(objects, role)...).WithRESTMapper(restMapper) - return fakeClientBuilder.Build() -} - -func TestPreAuthorize_Success(t *testing.T) { - t.Run("preauthorize succeeds with no missing rbac rules", func(t *testing.T) { - fakeClient := setupFakeClient(privilegedClusterRole) - preAuth := NewRBACPreAuthorizer(fakeClient) - missingRules, err := preAuth.PreAuthorize(context.TODO(), &exampleClusterExtension, strings.NewReader(testManifest)) - require.NoError(t, err) - require.Equal(t, []ScopedPolicyRules{}, missingRules) - }) -} - -func TestPreAuthorize_MissingRBAC(t *testing.T) { - t.Run("preauthorize fails and finds missing rbac rules", func(t *testing.T) { - fakeClient := setupFakeClient(limitedClusterRole) - preAuth := NewRBACPreAuthorizer(fakeClient) - missingRules, err := preAuth.PreAuthorize(context.TODO(), &exampleClusterExtension, strings.NewReader(testManifest)) - require.NoError(t, err) - require.Equal(t, expectedSingleNamespaceMissingRules, missingRules) - }) -} - -func TestPreAuthorizeMultiNamespace_MissingRBAC(t *testing.T) { - t.Run("preauthorize fails and finds missing rbac rules in multiple namespaces", func(t *testing.T) { - fakeClient := setupFakeClient(limitedClusterRole) - preAuth := NewRBACPreAuthorizer(fakeClient) - missingRules, err := preAuth.PreAuthorize(context.TODO(), &exampleClusterExtension, strings.NewReader(testManifestMultiNamespace)) - require.NoError(t, err) - require.Equal(t, expectedMultiNamespaceMissingRules, missingRules) - }) -} - -func TestPreAuthorize_CheckEscalation(t *testing.T) { - t.Run("preauthorize succeeds with no missing rbac rules", func(t *testing.T) { - fakeClient := setupFakeClient(escalatingClusterRole) - preAuth := NewRBACPreAuthorizer(fakeClient) - missingRules, err := preAuth.PreAuthorize(context.TODO(), &exampleClusterExtension, strings.NewReader(testManifest)) - require.NoError(t, err) - require.Equal(t, []ScopedPolicyRules{}, missingRules) - }) -} - -// TestParseEscalationErrorForMissingRules Are tests with respect to https://github.com/kubernetes/api/blob/e8d4d542f6a9a16a694bfc8e3b8cd1557eecfc9d/rbac/v1/types.go#L49-L74 -// Goal is: prove the regex works as planned AND that if the error messages ever change we'll learn about it with these tests -func TestParseEscalationErrorForMissingRules_ParsingLogic(t *testing.T) { - testCases := []struct { - name string - inputError error - expectedResult *parseResult - expectError require.ErrorAssertionFunc - }{ - { - name: "One Missing Resource Rule", - inputError: errors.New(`user "test-user" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:["apps"], Resources:["deployments"], Verbs:["get"]}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{"apps"}, Resources: []string{"deployments"}, Verbs: []string{"get"}}, - }, - }, - expectError: require.NoError, - }, - { - name: "Multiple Missing Rules (Resource + NonResource)", - inputError: errors.New(`user "sa" (groups=["system:authenticated"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:[""], Resources:["pods"], Verbs:["list" "watch"]} -{NonResourceURLs:["/healthz"], Verbs:["get"]}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"pods"}, Verbs: []string{"list", "watch"}}, - {NonResourceURLs: []string{"/healthz"}, Verbs: []string{"get"}}, - }, - }, - expectError: require.NoError, - }, - { - name: "One Missing Rule with Resolution Errors", - inputError: errors.New(`user "test-admin" (groups=["system:masters"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:["batch"], Resources:["jobs"], Verbs:["create"]}; resolution errors: [role "missing-role" not found]`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{"batch"}, Resources: []string{"jobs"}, Verbs: []string{"create"}}, - }, - ResolutionErrors: errors.New(`[role "missing-role" not found]`), - }, - expectError: require.NoError, - }, - { - name: "Multiple Missing Rules with Resolution Errors", - inputError: errors.New(`user "another-user" (groups=[]) is attempting to grant RBAC permissions not currently held: -{APIGroups:[""], Resources:["secrets"], Verbs:["get"]} -{APIGroups:[""], Resources:["configmaps"], Verbs:["list"]}; resolution errors: [clusterrole "missing-clusterrole" not found, role "other-missing" not found]`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"secrets"}, Verbs: []string{"get"}}, - {APIGroups: []string{""}, Resources: []string{"configmaps"}, Verbs: []string{"list"}}, - }, - ResolutionErrors: errors.New(`[clusterrole "missing-clusterrole" not found, role "other-missing" not found]`), - }, - expectError: require.NoError, - }, - { - name: "Missing Rule (All Resource Fields)", - inputError: errors.New(`user "resource-name-user" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:["extensions"], Resources:["ingresses"], ResourceNames:["my-ingress"], Verbs:["update" "patch"]}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{"extensions"}, Resources: []string{"ingresses"}, ResourceNames: []string{"my-ingress"}, Verbs: []string{"update", "patch"}}, - }, - }, - expectError: require.NoError, - }, - { - name: "Missing Rule (No ResourceNames)", - inputError: errors.New(`user "no-res-name-user" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:["networking.k8s.io"], Resources:["networkpolicies"], Verbs:["watch"]}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{"networking.k8s.io"}, Resources: []string{"networkpolicies"}, Verbs: []string{"watch"}}, - }, - }, - expectError: require.NoError, - }, - { - name: "Missing Rule (NonResourceURLs only)", - inputError: errors.New(`user "url-user" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{NonResourceURLs:["/version" "/apis"], Verbs:["get"]}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {NonResourceURLs: []string{"/version", "/apis"}, Verbs: []string{"get"}}, - }, - }, - expectError: require.NoError, - }, - { - name: "Unexpected Format", - inputError: errors.New("some completely different error message that doesn't match"), - expectedResult: &parseResult{}, - expectError: func(t require.TestingT, err error, i ...interface{}) { - require.ErrorContains(t, err, "unexpected format of escalation check error string") - }, - }, - { - name: "Empty Permissions String", - inputError: errors.New(`user "empty-perms" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -`), - expectedResult: &parseResult{}, - expectError: func(t require.TestingT, err error, i ...interface{}) { - require.ErrorContains(t, err, "unexpected format of escalation check error string") - }, - }, - { - name: "Rule with Empty Strings in lists", - inputError: errors.New(`user "empty-strings" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:["" "apps"], Resources:["" "deployments"], Verbs:["get" ""]}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{"", "apps"}, Resources: []string{"", "deployments"}, Verbs: []string{"get", ""}}, - }, - }, - expectError: require.NoError, - }, - { - name: "Rule with Only Empty Verb", - inputError: errors.New(`user "empty-verb" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:[""], Resources:["pods"], Verbs:[""]}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"pods"}, Verbs: []string{""}}, - }, - }, - expectError: require.NoError, - }, - { - name: "Rule with no fields", - inputError: errors.New(`user "empty-verb" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{}`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{{}}, - }, - expectError: require.NoError, - }, - { - name: "Rule with no colon separator", - inputError: errors.New(`user "empty-verb" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:[""], Resources, Verbs:["get"]} -`), - expectedResult: &parseResult{}, - expectError: func(t require.TestingT, err error, i ...interface{}) { - require.ErrorContains(t, err, `unexpected item "Resources": expected :[...]`) - }, - }, - { - name: "Rule with unknown field", - inputError: errors.New(`user "empty-verb" (groups=["test"]) is attempting to grant RBAC permissions not currently held: -{FooBar:["baz"]} -{APIGroups:[""], Resources:["secrets"], Verbs:["get"]} -`), - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"secrets"}, Verbs: []string{"get"}}, - }, - }, - expectError: func(t require.TestingT, err error, i ...interface{}) { - require.ErrorContains(t, err, `unknown field: "FooBar"`) - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - rules, err := parseEscalationErrorForMissingRules(tc.inputError) - tc.expectError(t, err) - require.Equal(t, tc.expectedResult, rules) - }) - } -} - -func TestParseEscalationErrorForMissingRules_KubernetesCompatibility(t *testing.T) { - testCases := []struct { - name string - ruleResolver validation.AuthorizationRuleResolver - wantRules []rbacv1.PolicyRule - expectedErrorString string - expectedResult *parseResult - }{ - { - name: "missing rules", - ruleResolver: mockRulesResolver{ - rules: []rbacv1.PolicyRule{}, - err: nil, - }, - wantRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"secrets"}, Verbs: []string{"get"}, ResourceNames: []string{"test-secret"}}, - {APIGroups: []string{""}, Resources: []string{"configmaps"}, Verbs: []string{"get", "list", "watch"}}, - {APIGroups: []string{"apps"}, Resources: []string{"deployments", "replicasets"}, Verbs: []string{"create", "update", "patch", "delete"}}, - {NonResourceURLs: []string{"/healthz", "/livez"}, Verbs: []string{"get", "post"}}, - }, - expectedErrorString: `user "user" (groups=["a" "b"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:[""], Resources:["configmaps"], Verbs:["get" "list" "watch"]} -{APIGroups:[""], Resources:["secrets"], ResourceNames:["test-secret"], Verbs:["get"]} -{APIGroups:["apps"], Resources:["deployments"], Verbs:["create" "update" "patch" "delete"]} -{APIGroups:["apps"], Resources:["replicasets"], Verbs:["create" "update" "patch" "delete"]} -{NonResourceURLs:["/healthz"], Verbs:["get"]} -{NonResourceURLs:["/healthz"], Verbs:["post"]} -{NonResourceURLs:["/livez"], Verbs:["get"]} -{NonResourceURLs:["/livez"], Verbs:["post"]}`, - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"configmaps"}, Verbs: []string{"get", "list", "watch"}}, - {APIGroups: []string{""}, Resources: []string{"secrets"}, Verbs: []string{"get"}, ResourceNames: []string{"test-secret"}}, - {APIGroups: []string{"apps"}, Resources: []string{"deployments"}, Verbs: []string{"create", "update", "patch", "delete"}}, - {APIGroups: []string{"apps"}, Resources: []string{"replicasets"}, Verbs: []string{"create", "update", "patch", "delete"}}, - {NonResourceURLs: []string{"/healthz"}, Verbs: []string{"get"}}, - {NonResourceURLs: []string{"/healthz"}, Verbs: []string{"post"}}, - {NonResourceURLs: []string{"/livez"}, Verbs: []string{"get"}}, - {NonResourceURLs: []string{"/livez"}, Verbs: []string{"post"}}, - }, - }, - }, - { - name: "resolution failure", - ruleResolver: mockRulesResolver{ - rules: []rbacv1.PolicyRule{}, - err: errors.New("resolution error"), - }, - wantRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"secrets"}, Verbs: []string{"get"}, ResourceNames: []string{"test-secret"}}, - {APIGroups: []string{""}, Resources: []string{"configmaps"}, Verbs: []string{"get", "list", "watch"}}, - {APIGroups: []string{"apps"}, Resources: []string{"deployments", "replicasets"}, Verbs: []string{"create", "update", "patch", "delete"}}, - {NonResourceURLs: []string{"/healthz", "/livez"}, Verbs: []string{"get", "post"}}, - }, - expectedErrorString: `user "user" (groups=["a" "b"]) is attempting to grant RBAC permissions not currently held: -{APIGroups:[""], Resources:["configmaps"], Verbs:["get" "list" "watch"]} -{APIGroups:[""], Resources:["secrets"], ResourceNames:["test-secret"], Verbs:["get"]} -{APIGroups:["apps"], Resources:["deployments"], Verbs:["create" "update" "patch" "delete"]} -{APIGroups:["apps"], Resources:["replicasets"], Verbs:["create" "update" "patch" "delete"]} -{NonResourceURLs:["/healthz"], Verbs:["get"]} -{NonResourceURLs:["/healthz"], Verbs:["post"]} -{NonResourceURLs:["/livez"], Verbs:["get"]} -{NonResourceURLs:["/livez"], Verbs:["post"]}; resolution errors: [resolution error]`, - expectedResult: &parseResult{ - MissingRules: []rbacv1.PolicyRule{ - {APIGroups: []string{""}, Resources: []string{"configmaps"}, Verbs: []string{"get", "list", "watch"}}, - {APIGroups: []string{""}, Resources: []string{"secrets"}, Verbs: []string{"get"}, ResourceNames: []string{"test-secret"}}, - {APIGroups: []string{"apps"}, Resources: []string{"deployments"}, Verbs: []string{"create", "update", "patch", "delete"}}, - {APIGroups: []string{"apps"}, Resources: []string{"replicasets"}, Verbs: []string{"create", "update", "patch", "delete"}}, - {NonResourceURLs: []string{"/healthz"}, Verbs: []string{"get"}}, - {NonResourceURLs: []string{"/healthz"}, Verbs: []string{"post"}}, - {NonResourceURLs: []string{"/livez"}, Verbs: []string{"get"}}, - {NonResourceURLs: []string{"/livez"}, Verbs: []string{"post"}}, - }, - ResolutionErrors: errors.New("[resolution error]"), - }, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - ctx := request.WithUser(request.WithNamespace(context.Background(), "namespace"), &user.DefaultInfo{ - Name: "user", - Groups: []string{"a", "b"}, - }) - - // Let's actually call the upstream function that generates and returns the - // error message that we are attempting to parse correctly. The hope is that - // these tests will start failing if we bump to a new version of kubernetes - // that causes our parsing logic to be incorrect. - err := validation.ConfirmNoEscalation(ctx, tc.ruleResolver, tc.wantRules) - require.Error(t, err) - require.Equal(t, tc.expectedErrorString, err.Error()) - - res, err := parseEscalationErrorForMissingRules(err) - require.NoError(t, err) - require.Equal(t, tc.expectedResult, res) - }) - } -} - -type mockRulesResolver struct { - rules []rbacv1.PolicyRule - err error -} - -func (m mockRulesResolver) GetRoleReferenceRules(ctx context.Context, roleRef rbacv1.RoleRef, namespace string) ([]rbacv1.PolicyRule, error) { - panic("unimplemented") -} - -func (m mockRulesResolver) RulesFor(ctx context.Context, user user.Info, namespace string) ([]rbacv1.PolicyRule, error) { - return m.rules, m.err -} - -func (m mockRulesResolver) VisitRulesFor(ctx context.Context, user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool) { - panic("unimplemented") -} diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go index 7e1fb930c1..b6baa4f74d 100644 --- a/internal/operator-controller/controllers/clusterextension_admission_test.go +++ b/internal/operator-controller/controllers/clusterextension_admission_test.go @@ -44,7 +44,7 @@ func TestClusterExtensionSourceConfig(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "default", }, })) @@ -55,7 +55,7 @@ func TestClusterExtensionSourceConfig(t *testing.T) { SourceType: tc.sourceType, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "default", }, })) @@ -114,7 +114,7 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "default", }, })) @@ -212,7 +212,7 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "default", }, })) @@ -267,7 +267,7 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "default", }, })) @@ -320,7 +320,7 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) { }, }, Namespace: tc.namespace, - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "default", }, })) @@ -348,7 +348,8 @@ 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}, + // An empty service account name passes since serviceAccount is omitted if zero-valued + {"no service account name", "", ""}, {"spaces", "spaces spaces", regexMismatchError}, {"capitalized", "Capitalized", regexMismatchError}, {"camel case", "camelCase", regexMismatchError}, @@ -374,7 +375,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: tc.serviceAccount, }, })) @@ -433,7 +434,7 @@ func TestClusterExtensionAdmissionInstall(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "default", }, Install: tc.installConfig, diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 437f62dcec..11f248bfee 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -103,9 +103,6 @@ func TestClusterExtensionShortCircuitsReconcileDuringDeletion(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "default", - }, }, } require.NoError(t, cl.Create(ctx, clusterExtension)) @@ -142,9 +139,6 @@ func TestClusterExtensionResolutionFails(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "default", - }, }, } require.NoError(t, cl.Create(ctx, clusterExtension)) @@ -204,7 +198,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { pkgVer := "1.0.0" pkgChan := "beta" namespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, @@ -218,9 +211,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { }, }, Namespace: namespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount, - }, }, } err := cl.Create(ctx, clusterExtension) @@ -284,7 +274,6 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) pkgVer := "1.0.0" pkgChan := "beta" namespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, @@ -298,9 +287,6 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) }, }, Namespace: namespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount, - }, }, } err := cl.Create(ctx, clusterExtension) @@ -368,7 +354,7 @@ func TestClusterExtensionServiceAccountNotFound(t *testing.T) { }, }, Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{ + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck Name: "missing-sa", }, }, @@ -415,7 +401,6 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { pkgVer := "1.0.0" pkgChan := "beta" namespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, @@ -429,9 +414,6 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { }, }, Namespace: namespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount, - }, }, } err := cl.Create(ctx, clusterExtension) @@ -510,7 +492,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) { pkgVer := "1.0.0" pkgChan := "beta" namespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, @@ -524,9 +505,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) { }, }, Namespace: namespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount, - }, }, } err := cl.Create(ctx, clusterExtension) @@ -586,7 +564,6 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) { pkgVer := "1.0.0" pkgChan := "beta" installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, @@ -601,9 +578,6 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) { }, }, Namespace: installNamespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount, - }, }, } err := cl.Create(ctx, clusterExtension) @@ -663,7 +637,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) { pkgVer := "1.0.0" pkgChan := "beta" namespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, @@ -677,9 +650,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) { }, }, Namespace: namespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount, - }, }, } err := cl.Create(ctx, clusterExtension) @@ -738,7 +708,6 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) { pkgVer := "1.0.0" pkgChan := "beta" namespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - serviceAccount := fmt.Sprintf("test-sa-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, @@ -752,9 +721,6 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) { }, }, Namespace: namespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount, - }, }, } err := cl.Create(ctx, clusterExtension) diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go index 694bd4d4af..5dd16be4c2 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller_test.go @@ -649,9 +649,6 @@ func newTestClusterExtension() *ocv1.ClusterExtension { }, Spec: ocv1.ClusterExtensionSpec{ Namespace: "some-namespace", - ServiceAccount: ocv1.ServiceAccountReference{ - Name: "service-account", - }, Source: ocv1.SourceConfig{ SourceType: ocv1.SourceTypeCatalog, Catalog: &ocv1.CatalogFilter{ diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go index 1abdf0a18a..9d85c1c27f 100644 --- a/internal/operator-controller/features/features.go +++ b/internal/operator-controller/features/features.go @@ -11,9 +11,7 @@ import ( const ( // Add new feature gates constants (strings) // Ex: SomeFeature featuregate.Feature = "SomeFeature" - PreflightPermissions featuregate.Feature = "PreflightPermissions" SingleOwnNamespaceInstallSupport featuregate.Feature = "SingleOwnNamespaceInstallSupport" - SyntheticPermissions featuregate.Feature = "SyntheticPermissions" WebhookProviderCertManager featuregate.Feature = "WebhookProviderCertManager" WebhookProviderOpenshiftServiceCA featuregate.Feature = "WebhookProviderOpenshiftServiceCA" HelmChartSupport featuregate.Feature = "HelmChartSupport" @@ -21,14 +19,6 @@ const ( ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ - // Add new feature gate definitions - // Ex: SomeFeature: {...} - PreflightPermissions: { - Default: false, - PreRelease: featuregate.Alpha, - LockToDefault: false, - }, - // SingleOwnNamespaceInstallSupport enables support for installing // registry+v1 cluster extensions with single or own namespaces modes // i.e. with a single watch namespace. @@ -38,14 +28,6 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature LockToDefault: false, }, - // SyntheticPermissions enables support for a synthetic user permission - // model to manage operator permission boundaries - SyntheticPermissions: { - Default: false, - PreRelease: featuregate.Alpha, - LockToDefault: false, - }, - // WebhookProviderCertManager enables support for installing // registry+v1 cluster extensions that include validating, // mutating, and/or conversion webhooks with CertManager diff --git a/internal/operator-controller/resolve/catalog_test.go b/internal/operator-controller/resolve/catalog_test.go index 21232bc4df..5319edffa7 100644 --- a/internal/operator-controller/resolve/catalog_test.go +++ b/internal/operator-controller/resolve/catalog_test.go @@ -585,8 +585,7 @@ func buildFooClusterExtension(pkg string, channels []string, version string, upg Name: pkg, }, Spec: ocv1.ClusterExtensionSpec{ - Namespace: "default", - ServiceAccount: ocv1.ServiceAccountReference{Name: "default"}, + Namespace: "default", Source: ocv1.SourceConfig{ SourceType: "Catalog", Catalog: &ocv1.CatalogFilter{ diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 1bc93321ef..6b8a6e770f 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -944,8 +944,7 @@ spec: namespace: description: |- 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 + It 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. @@ -964,14 +963,14 @@ 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 + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -1291,7 +1290,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: @@ -1559,71 +1557,6 @@ rules: - list - watch --- -# Source: olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: operator-controller-manager-role - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental-e2e -rules: - - apiGroups: - - "" - resources: - - serviceaccounts/token - verbs: - - create - - apiGroups: - - apiextensions.k8s.io - resources: - - customresourcedefinitions - verbs: - - get - - apiGroups: - - olm.operatorframework.io - resources: - - clustercatalogs - verbs: - - get - - list - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions - verbs: - - get - - list - - patch - - update - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/finalizers - verbs: - - update - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/status - verbs: - - patch - - update - - apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterrolebindings - - clusterroles - - rolebindings - - roles - verbs: - - list - - watch ---- # Source: olmv1/templates/rbac/clusterrolebinding-catalogd-manager-rolebinding.yml apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -2168,7 +2101,6 @@ spec: - --leader-elect - --feature-gates=WebhookProviderCertManager=true - --feature-gates=SingleOwnNamespaceInstallSupport=true - - --feature-gates=PreflightPermissions=true - --feature-gates=HelmChartSupport=true - --feature-gates=BoxcutterRuntime=true - --tls-cert=/var/certs/tls.crt diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 69128a8b7b..61f83564c0 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -909,8 +909,7 @@ spec: namespace: description: |- 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 + It 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. @@ -929,14 +928,14 @@ 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 + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -1256,7 +1255,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: @@ -1524,71 +1522,6 @@ rules: - list - watch --- -# Source: olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: operator-controller-manager-role - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental -rules: - - apiGroups: - - "" - resources: - - serviceaccounts/token - verbs: - - create - - apiGroups: - - apiextensions.k8s.io - resources: - - customresourcedefinitions - verbs: - - get - - apiGroups: - - olm.operatorframework.io - resources: - - clustercatalogs - verbs: - - get - - list - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions - verbs: - - get - - list - - patch - - update - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/finalizers - verbs: - - update - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/status - verbs: - - patch - - update - - apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterrolebindings - - clusterroles - - rolebindings - - roles - verbs: - - list - - watch ---- # Source: olmv1/templates/rbac/clusterrolebinding-catalogd-manager-rolebinding.yml apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -2081,7 +2014,6 @@ spec: - --leader-elect - --feature-gates=WebhookProviderCertManager=true - --feature-gates=SingleOwnNamespaceInstallSupport=true - - --feature-gates=PreflightPermissions=true - --feature-gates=HelmChartSupport=true - --feature-gates=BoxcutterRuntime=true - --tls-cert=/var/certs/tls.crt diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 72f8b82dcf..92ef0782f5 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -703,8 +703,7 @@ spec: namespace: description: |- 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 + It 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. @@ -723,14 +722,14 @@ 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 + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -1050,7 +1049,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: @@ -1318,71 +1316,6 @@ rules: - list - watch --- -# Source: olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: operator-controller-manager-role - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard-e2e -rules: - - apiGroups: - - "" - resources: - - serviceaccounts/token - verbs: - - create - - apiGroups: - - apiextensions.k8s.io - resources: - - customresourcedefinitions - verbs: - - get - - apiGroups: - - olm.operatorframework.io - resources: - - clustercatalogs - verbs: - - get - - list - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions - verbs: - - get - - list - - patch - - update - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/finalizers - verbs: - - update - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/status - verbs: - - patch - - update - - apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterrolebindings - - clusterroles - - rolebindings - - roles - verbs: - - list - - watch ---- # Source: olmv1/templates/rbac/clusterrolebinding-catalogd-manager-rolebinding.yml apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -1453,7 +1386,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 75ee176f7f..a825ae6740 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -668,8 +668,7 @@ spec: namespace: description: |- 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 + It 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. @@ -688,14 +687,14 @@ 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 + Deprecated: ServiceAccount is ignored by OLM and will be removed in a future release. + serviceAccount was a reference to the 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 optional. properties: name: description: |- + Deprecated: ServiceAccount.Name is ignored by OLM and will be removed in a future release. 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. @@ -1015,7 +1014,6 @@ spec: has(self.catalog) : !has(self.catalog)' required: - namespace - - serviceAccount - source type: object status: @@ -1283,71 +1281,6 @@ rules: - list - watch --- -# Source: olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: operator-controller-manager-role - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard -rules: - - apiGroups: - - "" - resources: - - serviceaccounts/token - verbs: - - create - - apiGroups: - - apiextensions.k8s.io - resources: - - customresourcedefinitions - verbs: - - get - - apiGroups: - - olm.operatorframework.io - resources: - - clustercatalogs - verbs: - - get - - list - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions - verbs: - - get - - list - - patch - - update - - watch - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/finalizers - verbs: - - update - - apiGroups: - - olm.operatorframework.io - resources: - - clusterextensions/status - verbs: - - patch - - update - - apiGroups: - - rbac.authorization.k8s.io - resources: - - clusterrolebindings - - clusterroles - - rolebindings - - roles - verbs: - - list - - watch ---- # Source: olmv1/templates/rbac/clusterrolebinding-catalogd-manager-rolebinding.yml apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -1418,7 +1351,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 26b0cb7a0e..c595a7207f 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -380,9 +380,6 @@ func TestClusterExtensionInstallRegistry(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } t.Log("It resolves the specified package with correct bundle path") t.Log("By creating the ClusterExtension resource") @@ -450,9 +447,6 @@ func TestClusterExtensionInstallRegistryDynamic(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } t.Log("It updates the registries.conf file contents") cm := corev1.ConfigMap{ @@ -527,9 +521,6 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } t.Log("It resolves to multiple bundle paths") t.Log("By creating the ClusterExtension resource") @@ -570,9 +561,6 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } require.NoError(t, c.Create(context.Background(), clusterExtension)) t.Log("By eventually reporting a successful installation") @@ -630,9 +618,6 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } require.NoError(t, c.Create(context.Background(), clusterExtension)) t.Log("By eventually reporting a successful resolution") @@ -677,9 +662,6 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } require.NoError(t, c.Create(context.Background(), clusterExtension)) t.Log("By eventually reporting a successful resolution") @@ -730,9 +712,6 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } t.Log("It resolves the specified package with correct bundle path") t.Log("By creating the ClusterExtension resource") @@ -810,9 +789,6 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) { }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } t.Log("It resolves the specified package with correct bundle path") t.Log("By creating the ClusterExtension resource") @@ -868,9 +844,6 @@ func TestClusterExtensionInstallReResolvesWhenManagedContentChanged(t *testing.T }, }, Namespace: ns.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, } t.Log("It installs the specified package with correct bundle path") t.Log("By creating the ClusterExtension resource") @@ -922,9 +895,6 @@ func TestClusterExtensionRecoversFromNoNamespaceWhenFailureFixed(t *testing.T) { }, }, Namespace: clusterExtension.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: clusterExtension.Name, - }, } t.Log("It resolves the specified package with correct bundle path") @@ -997,9 +967,6 @@ func TestClusterExtensionRecoversFromExistingDeploymentWhenFailureFixed(t *testi }, }, Namespace: clusterExtension.Name, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: clusterExtension.Name, - }, } t.Log("By creating a new Deployment that can not be adopted") diff --git a/test/experimental-e2e/experimental_e2e_test.go b/test/experimental-e2e/experimental_e2e_test.go index 234d73d8db..7850bda0a1 100644 --- a/test/experimental-e2e/experimental_e2e_test.go +++ b/test/experimental-e2e/experimental_e2e_test.go @@ -155,9 +155,6 @@ func TestWebhookSupport(t *testing.T) { }, }, Namespace: namespace.GetName(), - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount.GetName(), - }, }, } require.NoError(t, c.Create(t.Context(), clusterExtension)) @@ -352,9 +349,6 @@ func TestClusterExtensionConfigSupport(t *testing.T) { }, }, Namespace: namespace.GetName(), - ServiceAccount: ocv1.ServiceAccountReference{ - Name: serviceAccount.GetName(), - }, Config: &ocv1.ClusterExtensionConfig{ ConfigType: ocv1.ClusterExtensionConfigTypeInline, Inline: &apiextensionsv1.JSON{ diff --git a/test/extension-developer-e2e/extension_developer_test.go b/test/extension-developer-e2e/extension_developer_test.go index a71f1f83d6..ab03e5eb86 100644 --- a/test/extension-developer-e2e/extension_developer_test.go +++ b/test/extension-developer-e2e/extension_developer_test.go @@ -77,9 +77,6 @@ func TestExtensionDeveloper(t *testing.T) { }, }, Namespace: installNamespace, - ServiceAccount: ocv1.ServiceAccountReference{ - Name: sa.Name, - }, }, }