Switch from token-based auth to ServiceAccount impersonation

This commit replaces token-based authentication with ServiceAccount
impersonation for better security and simpler token management.

Changes:

Authentication layer:
- Added ServiceAccountImpersonationConfig() function that returns an
  ImpersonationConfig for a ServiceAccount user
- Updated ServiceAccountRestConfigMapper() to use impersonation via
  NewImpersonatingRoundTripper instead of TokenInjectingRoundTripper
- Removed TokenGetter parameter from ServiceAccountRestConfigMapper
- Deleted tokengetter.go, tokengetter_test.go, and tripper.go as
  they are no longer needed

RBAC:
- Changed from serviceaccounts/token create to serviceaccounts
  impersonate permission

Controller:
- Removed ServiceAccountNotFoundError handling since impersonation
  doesn't require the ServiceAccount to exist beforehand
- Removed authentication package import from controller

Main setup:
- Removed TokenGetter initialization
- Updated userInfoMapper to use ServiceAccountImpersonationConfig

Tests:
- Updated tests to verify impersonation headers instead of token
  injection
- Added tests for ServiceAccountImpersonationConfig
- Updated controller tests that referenced TokenGetter

Benefits of impersonation over tokens:
- No need to manage token lifecycle or expiration
- No need to check if ServiceAccount exists before use
- Simpler code with fewer moving parts
- More secure as no tokens are created or cached
- More secure as it is no longer required to create highly privileged
  ServiceAccounts that could be used by workloads in the install
  namespace.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

Author: joelanford

cmd/operator-controller/main.go

@@ -646,8 +646,7 @@ 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)
+	clientRestConfigMapper := action.ServiceAccountRestConfigMapper()
 	if features.OperatorControllerFeatureGate.Enabled(features.SyntheticPermissions) {
 		clientRestConfigMapper = action.SyntheticUserRestConfigMapper(clientRestConfigMapper)
 	}
@@ -681,7 +680,8 @@ func setupHelm(
 			} else if ext.Spec.ServiceAccount.Name == "" || ext.Spec.Namespace == "" {
 				return nil, errors.New("service account name and namespace must be specified")
 			}
-			return &user.DefaultInfo{Name: fmt.Sprintf("system:serviceaccount:%s:%s", ext.Spec.Namespace, ext.Spec.ServiceAccount.Name)}, nil
+			saConfig := authentication.ServiceAccountImpersonationConfig(*ext)
+			return &user.DefaultInfo{Name: saConfig.UserName, Groups: saConfig.Groups}, nil
 		})
 	}
 

config/samples/olm_v1_clusterextension.yaml

@@ -4,11 +4,13 @@ kind: Namespace
 metadata:
   name: argocd
 ---
-apiVersion: v1
-kind: ServiceAccount
-metadata:
-  name: argocd-installer
-  namespace: argocd
+# NOTE: The ServiceAccount resource is intentionally NOT created here.
+# OLM v1 uses Kubernetes impersonation and does not require the ServiceAccount to exist.
+# For security reasons, you should NOT create a ServiceAccount resource for the installer,
+# as it would be highly privileged and could be mounted by other pods in the namespace.
+# Instead, only the RBAC resources (ClusterRole, ClusterRoleBinding, Role, RoleBinding)
+# are created, which reference the ServiceAccount name "argocd-installer".
+# OLM will impersonate that ServiceAccount and be subject to these RBAC permissions.
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding

docs/howto/derive-service-account.md

@@ -300,12 +300,20 @@ Therefore, the following permissions must be given to the installer service acco
 Once the installer service account required cluster-scoped and namespace-scoped permissions have been collected:
 
 1. Create the installation namespace
-2. Create the installer `ServiceAccount`
-3. Create the installer `ClusterRole`
-4. Create the `ClusterRoleBinding` between the installer service account and its cluster role
-5. Create the installer `Role`
-6. Create the `RoleBinding` between the installer service account and its role
-7. Create the `ClusterExtension`
+2. Create the installer `ClusterRole`
+3. Create the `ClusterRoleBinding` between the installer service account and its cluster role
+4. Create the installer `Role`
+5. Create the `RoleBinding` between the installer service account and its role
+6. Create the `ClusterExtension`
+
+!!! important "Do Not Create the ServiceAccount Resource"
+    OLM v1 uses Kubernetes impersonation and does not require the ServiceAccount to exist as an actual resource.
+    For security reasons, you should **NOT** create a ServiceAccount resource for the installer,
+    as it would be highly privileged and could be mounted by other pods in the namespace.
+
+    Instead, only create the RBAC resources (ClusterRole, ClusterRoleBinding, Role, RoleBinding)
+    that reference the ServiceAccount name. OLM will impersonate that ServiceAccount and be subject
+    to these RBAC permissions without the ServiceAccount resource actually existing in the cluster.
 
 A manifest with the full set of resources can be found [here](https://github.com/operator-framework/operator-controller/blob/main/config/samples/olm_v1_clusterextension.yaml).
 

docs/tutorials/upgrade-extension.md

@@ -33,12 +33,6 @@ saving it to a local file and then running `kubectl apply -f FILENAME`:
     metadata:
       name: argocd
     ---
-    apiVersion: v1
-    kind: ServiceAccount
-    metadata:
-      name: argocd-installer
-      namespace: argocd
-    ---
     apiVersion: rbac.authorization.k8s.io/v1
     kind: ClusterRoleBinding
     metadata:
@@ -376,4 +370,4 @@ kubectl get clusterextension argocd -o jsonpath-as-json="{.status.install}"
     After your upgrade, the contents of the `kubectl.kubernetes.io/last-applied-configuration` annotation field will
     differ depending on your method of upgrade. If you apply a new ClusterExtension manifest as in the first method shown,
     the last applied configuration will show the new version since we replaced the existing manifest. If you use the patch
-    method or `kubectl edit clusterextension`, then the last applied configuration will show the old version.
+    method or `kubectl edit clusterextension`, then the last applied configuration will show the old version.

helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml

@@ -12,9 +12,9 @@ rules:
   - apiGroups:
       - ""
     resources:
-      - serviceaccounts/token
+      - serviceaccounts
     verbs:
-      - create
+      - impersonate
   {{- if (has "SyntheticPermissions" .Values.options.operatorController.features.enabled) }}
   - apiGroups:
       - ""

internal/operator-controller/action/restconfig.go

@@ -5,7 +5,6 @@ import (
 	"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"
@@ -34,25 +33,19 @@ func SyntheticUserRestConfigMapper(defaultAuthMapper func(ctx context.Context, o
 }
 
 // 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) {
+// be a ClusterExtension. It uses impersonation, which allows authentication as a ServiceAccount without requiring the
+// ServiceAccount to actually exist or having to manage tokens.
+func ServiceAccountRestConfigMapper() 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,
-				},
-			}
+		cc := rest.CopyConfig(c)
+		cc.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+			return transport.NewImpersonatingRoundTripper(authentication.ServiceAccountImpersonationConfig(*cExt), rt)
 		})
-		return saConfig, nil
+		return cc, nil
 	}
 }
 

internal/operator-controller/action/restconfig_test.go

@@ -9,13 +9,11 @@ import (
 	"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) {
@@ -55,21 +53,21 @@ func Test_ServiceAccountRestConfigMapper(t *testing.T) {
 		},
 	} {
 		t.Run(tc.description, func(t *testing.T) {
-			tokenGetter := &authentication.TokenGetter{}
-			saMapper := action.ServiceAccountRestConfigMapper(tokenGetter)
+			saMapper := action.ServiceAccountRestConfigMapper()
 			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)
+				require.NotNil(t, actualCfg)
+
+				// test that the impersonation headers are appropriately injected into the request
+				// nolint:bodyclose
+				_, _ = actualCfg.WrapTransport(fakeRoundTripper(func(req *http.Request) (*http.Response, error) {
+					require.Equal(t, "system:serviceaccount:my-namespace:my-service-account", req.Header.Get("Impersonate-User"))
+					return &http.Response{}, nil
+				})).RoundTrip(&http.Request{})
 			}
 		})
 	}

internal/operator-controller/authentication/serviceaccount.go

@@ -0,0 +1,17 @@
+package authentication
+
+import (
+	"fmt"
+
+	"k8s.io/client-go/transport"
+
+	ocv1 "github.com/operator-framework/operator-controller/api/v1"
+)
+
+// ServiceAccountImpersonationConfig returns an ImpersonationConfig for impersonating a ServiceAccount.
+// This allows authentication as a ServiceAccount without requiring an actual token.
+func ServiceAccountImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig {
+	return transport.ImpersonationConfig{
+		UserName: fmt.Sprintf("system:serviceaccount:%s:%s", ext.Spec.Namespace, ext.Spec.ServiceAccount.Name),
+	}
+}

internal/operator-controller/authentication/serviceaccount_test.go

@@ -0,0 +1,24 @@
+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 TestServiceAccountImpersonationConfig(t *testing.T) {
+	config := authentication.ServiceAccountImpersonationConfig(ocv1.ClusterExtension{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "my-ext",
+		},
+		Spec: ocv1.ClusterExtensionSpec{Namespace: "my-namespace", ServiceAccount: ocv1.ServiceAccountReference{Name: "my-service-account"}},
+	})
+	require.Equal(t, "system:serviceaccount:my-namespace:my-service-account", config.UserName)
+	require.Nil(t, config.Groups)
+	require.Empty(t, config.UID)
+	require.Empty(t, config.Extra)
+}