auth: use synthetic user/group when service account is not defined

Signed-off-by: Joe Lanford <[email protected]>

Author: joelanford

api/v1/clusterextension_types.go

@@ -66,10 +66,19 @@ type ClusterExtensionSpec struct {
 	// 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.
 	//
-	// +kubebuilder:validation:Required
-	ServiceAccount ServiceAccountReference `json:"serviceAccount"`
+	// serviceAccount is optional. If a service account is not defined, requests to the apiserver will instead use
+	// username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups
+	// "olmv1:clusterextensions:admin" and "system:authenticated"
+	//
+	// Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged
+	// to use the synthetic user/groups described above. All of the same RBAC setup is still required with these
+	// synthetic user/groups. However, this mode is preferred because it requires administrators to specifically
+	// configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged
+	// service accounts that already exist on the cluster.
+	//
+	// +optional
+	ServiceAccount *ServiceAccountReference `json:"serviceAccount"`
 
 	// source is a required field which selects the installation source of content
 	// for this ClusterExtension. Selection is performed by setting the sourceType.

api/v1/zz_generated.deepcopy.go

@@ -298,7 +298,7 @@ func run() error {
 		return err
 	}
 	tokenGetter := authentication.NewTokenGetter(coreClient, authentication.WithExpirationDuration(1*time.Hour))
-	clientRestConfigMapper := action.ServiceAccountRestConfigMapper(tokenGetter)
+	clientRestConfigMapper := action.ClusterExtensionUserRestConfigMapper(tokenGetter)
 
 	cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
 		helmclient.StorageDriverMapper(action.ChunkedStorageDriverMapper(coreClient, mgr.GetAPIReader(), cfg.systemNamespace)),

cmd/operator-controller/main.go

@@ -135,7 +135,16 @@ spec:
                   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. If a service account is not defined, requests to the apiserver will instead use
+                  username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups
+                  "olmv1:clusterextensions:admin" and "system:authenticated"
+
+                  Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged
+                  to use the synthetic user/groups described above. All of the same RBAC setup is still required with these
+                  synthetic user/groups. However, this mode is preferred because it requires administrators to specifically
+                  configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged
+                  service accounts that already exist on the cluster.
                 properties:
                   name:
                     description: |-
@@ -458,7 +467,6 @@ spec:
                     has(self.catalog) : !has(self.catalog)'
             required:
             - namespace
-            - serviceAccount
             - source
             type: object
           status:

config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml

@@ -4,6 +4,13 @@ kind: ClusterRole
 metadata:
   name: manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - groups
+  - users
+  verbs:
+  - impersonate
 - apiGroups:
   - ""
   resources:

config/base/operator-controller/rbac/role.yaml

@@ -2,13 +2,7 @@
 apiVersion: v1
 kind: Namespace
 metadata:
-  name: argocd
----
-apiVersion: v1
-kind: ServiceAccount
-metadata:
-  name: argocd-installer
-  namespace: argocd
+  name: argocd-system
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
@@ -19,9 +13,8 @@ roleRef:
   kind: ClusterRole
   name: argocd-installer-clusterrole
 subjects:
-- kind: ServiceAccount
-  name: argocd-installer
-  namespace: argocd
+- kind: User
+  name: "olmv1:clusterextensions:argocd-operator:admin"
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
@@ -76,9 +69,8 @@ roleRef:
   kind: ClusterRole
   name: argocd-installer-rbac-clusterrole
 subjects:
-- kind: ServiceAccount
-  name: argocd-installer
-  namespace: argocd
+- kind: User
+  name: "olmv1:clusterextensions:argocd-operator:admin"
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
@@ -222,7 +214,7 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
   name: argocd-installer-role
-  namespace: argocd
+  namespace: argocd-system
 rules:
 - apiGroups: [""]
   resources: [serviceaccounts]
@@ -260,24 +252,21 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:
   name: argocd-installer-binding
-  namespace: argocd
+  namespace: argocd-system
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: Role
   name: argocd-installer-role
 subjects:
-- kind: ServiceAccount
-  name: argocd-installer
-  namespace: argocd
+- kind: User
+  name: "olmv1:clusterextensions:argocd-operator:admin"
 ---
 apiVersion: olm.operatorframework.io/v1
 kind: ClusterExtension
 metadata:
-  name: argocd
+  name: argocd-operator
 spec:
-  namespace: argocd
-  serviceAccount:
-    name: argocd-installer
+  namespace: argocd-system
   source:
     sourceType: Catalog
     catalog:

config/samples/olm_v1_clusterextension.yaml

@@ -306,7 +306,7 @@ _Appears in:_
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
 | `namespace` _string_ | namespace is a reference to a Kubernetes namespace.<br />This is the namespace in which the provided ServiceAccount must exist.<br />It also designates the default namespace where namespace-scoped resources<br />for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br /><br />namespace is required, immutable, and follows the DNS label standard<br />as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-),<br />start and end with an alphanumeric character, and be no longer than 63 characters<br /><br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 |  | MaxLength: 63 <br />Required: \{\} <br /> |
-| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br />serviceAccount is required. |  | Required: \{\} <br /> |
+| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br /><br />serviceAccount is optional. If a service account is not defined, requests to the apiserver will instead use<br />username "olmv1:clusterextensions:<clusterExtension.metadata.name>:admin" and groups<br />"olmv1:clusterextensions:admin" and "system:authenticated"<br /><br />Deprecated: Use of serviceAccount is not recommended. Instead, administrators are encouraged<br />to use the synthetic user/groups described above. All of the same RBAC setup is still required with these<br />synthetic user/groups. However, this mode is preferred because it requires administrators to specifically<br />configure RBAC for extension management, rather than enabling piggybacking on existing highly privileged<br />service accounts that already exist on the cluster. |  |  |
 | `source` _[SourceConfig](#sourceconfig)_ | source is a required field which selects the installation source of content<br />for this ClusterExtension. Selection is performed by setting the sourceType.<br /><br />Catalog is currently the only implemented sourceType, and setting the<br />sourcetype to "Catalog" requires the catalog field to also be defined.<br /><br />Below is a minimal example of a source definition (in yaml):<br /><br />source:<br />  sourceType: Catalog<br />  catalog:<br />    packageName: example-package |  | Required: \{\} <br /> |
 | `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is an optional field used to configure the installation options<br />for the ClusterExtension such as the pre-flight check configuration. |  |  |
 

docs/api-reference/operator-controller-api-reference.md

@@ -6,17 +6,42 @@ 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"
 )
 
-func ServiceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+func ClusterExtensionUserRestConfigMapper(tokenGetter *authentication.TokenGetter) func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+	saRestConfigMapper := serviceAccountRestConfigMapper(tokenGetter)
+	synthRestConfigMapper := sythenticUserRestConfigMapper()
+
+	return func(ctx context.Context, o client.Object, c *rest.Config) (*rest.Config, error) {
+		cExt := o.(*ocv1.ClusterExtension)
+		if cExt.Spec.ServiceAccount != nil { //nolint:staticcheck
+			return saRestConfigMapper(ctx, o, c)
+		}
+		return synthRestConfigMapper(ctx, o, c)
+	}
+}
+
+func sythenticUserRestConfigMapper() 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 := o.(*ocv1.ClusterExtension)
+		cc := rest.CopyConfig(c)
+		cc.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+			return transport.NewImpersonatingRoundTripper(authentication.SyntheticImpersonationConfig(*cExt), rt)
+		})
+		return cc, nil
+	}
+}
+
+func serviceAccountRestConfigMapper(tokenGetter *authentication.TokenGetter) 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 := o.(*ocv1.ClusterExtension)
 		saKey := types.NamespacedName{
-			Name:      cExt.Spec.ServiceAccount.Name,
+			Name:      cExt.Spec.ServiceAccount.Name, //nolint:staticcheck
 			Namespace: cExt.Spec.Namespace,
 		}
 		saConfig := rest.AnonymousClientConfig(c)

internal/operator-controller/action/restconfig.go

@@ -0,0 +1,27 @@
+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("olmv1:clusterextensions:%s:admin", ext.Name)
+}
+
+func SyntheticGroups(_ ocv1.ClusterExtension) []string {
+	return []string{
+		"olmv1:clusterextensions:admin",
+		"system:authenticated",
+	}
+}
+
+func SyntheticImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig {
+	return transport.ImpersonationConfig{
+		UserName: SyntheticUserName(ext),
+		Groups:   SyntheticGroups(ext),
+	}
+}

internal/operator-controller/authentication/synthetic.go

@@ -44,9 +44,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
 						},
 					},
 					Namespace: "default",
-					ServiceAccount: ocv1.ServiceAccountReference{
-						Name: "default",
-					},
 				}))
 			}
 			if tc.unionField == "" {
@@ -55,9 +52,6 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
 						SourceType: tc.sourceType,
 					},
 					Namespace: "default",
-					ServiceAccount: ocv1.ServiceAccountReference{
-						Name: "default",
-					},
 				}))
 			}
 
@@ -114,9 +108,6 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -212,9 +203,6 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -267,9 +255,6 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for channel %q: %w", tc.channels, err)
@@ -320,9 +305,6 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 					},
 				},
 				Namespace: tc.namespace,
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for namespace %q: %w", tc.namespace, err)
@@ -374,7 +356,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
+				ServiceAccount: &ocv1.ServiceAccountReference{
 					Name: tc.serviceAccount,
 				},
 			}))
@@ -433,10 +415,7 @@ func TestClusterExtensionAdmissionInstall(t *testing.T) {
 					},
 				},
 				Namespace: "default",
-				ServiceAccount: ocv1.ServiceAccountReference{
-					Name: "default",
-				},
-				Install: tc.installConfig,
+				Install:   tc.installConfig,
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for install configuration %v: %w", tc.installConfig, err)