Deprecate spec.ServiceAccount and remove synthetic permissions feature

Make spec.ServiceAccount an optional field and note that it's now
deprecated and does not perform any function. Make OLM use cluster-admin
by default for managing ClusterExtensions.

Remove the permissions preflight experimental feature flag.

Remove the synthetic permissions experimental feature flag.

Author: trgeiger

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
+	// serviceAccount is deprecated and ignored by OLM.
+	// 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,omitempty"`
 
 	// source is a required field which selects the installation source of content
 	// for this ClusterExtension. Selection is performed by setting the sourceType.
@@ -369,7 +367,8 @@ type CatalogFilter struct {
 	UpgradeConstraintPolicy UpgradeConstraintPolicy `json:"upgradeConstraintPolicy,omitempty"`
 }
 
-// ServiceAccountReference identifies the serviceAccount used fo install a ClusterExtension.
+// ServiceAccountReference identifies the serviceAccount used to install a ClusterExtension.
+// Note: The serviceAccount field is deprecated and ignored by OLM.
 type ServiceAccountReference struct {
 	// name is a required, immutable reference to the name of the ServiceAccount
 	// to be used for installation and management of the content for the package

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"
@@ -606,11 +604,9 @@ 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)
-	}
+	// tokenGetter := authentication.NewTokenGetter(coreClient, authentication.WithExpirationDuration(1*time.Hour))
+	// clientRestConfigMapper := action.ServiceAccountRestConfigMapper(tokenGetter)
+	clientRestConfigMapper := action.ClusterAdminRestConfigMapper(mgr.GetConfig())
 
 	cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
 		helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)),
@@ -631,12 +627,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)
@@ -660,7 +650,6 @@ func setupHelm(
 			IsWebhookSupportEnabled: certProvider != nil,
 		},
 		HelmReleaseToObjectsConverter: &applier.HelmReleaseToObjectsConverter{},
-		PreAuthorizer:                 preAuth,
 		Watcher:                       ceController,
 		Manager:                       cm,
 	}

hack/demo/resources/synthetic-user-perms/argocd-clusterextension.yaml

@@ -63,13 +63,13 @@ type ClusterExtensionSpec struct {
 	// +kubebuilder:validation:Required
 	Namespace string `json:"namespace"`
 
-	// serviceAccount is a reference to a ServiceAccount used to perform all interactions
+	// serviceAccount is deprecated and ignored by OLM.
 	// 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 deprecated and optional.
 	//
-	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:Optional
 	ServiceAccount ServiceAccountReference `json:"serviceAccount"`
 
 	// source is a required field which selects the installation source of content

hack/demo/resources/synthetic-user-perms/cegroup-admin-binding.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,11 +165,10 @@ 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
+                  serviceAccount is deprecated and ignored by OLM.
+                  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: |-
@@ -493,7 +491,6 @@ spec:
                     has(self.catalog) : !has(self.catalog)'
             required:
             - namespace
-            - serviceAccount
             - source
             type: object
           status:

hack/demo/synthetic-user-cluster-admin-demo-script.sh

@@ -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,11 +131,10 @@ 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
+                  serviceAccount is deprecated and ignored by OLM.
+                  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: |-
@@ -459,7 +457,6 @@ spec:
                     has(self.catalog) : !has(self.catalog)'
             required:
             - namespace
-            - serviceAccount
             - source
             type: object
           status:

hack/tools/crd-generator/testdata/api/v1/clusterextension_types.go

@@ -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

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml

@@ -7,35 +7,12 @@ import (
 
 	"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) {
@@ -59,6 +36,14 @@ func ServiceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) fun
 	}
 }
 
+// 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) {
+		return rest.CopyConfig(adminConfig), nil
+	}
+}
+
 func validate(o client.Object, c *rest.Config) (*ocv1.ClusterExtension, error) {
 	if c == nil {
 		return nil, fmt.Errorf("rest config is nil")