Use SA from spec

Author: skattoju

Makefile

@@ -152,7 +152,7 @@ build-push-e2e-catalog: ## Build the testdata catalog used for e2e tests and pus
 test-e2e: KIND_CLUSTER_NAME := operator-controller-e2e
 test-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/e2e
 test-e2e: GO_BUILD_FLAGS := -cover
-test-e2e: run image-registry build-push-e2e-catalog registry-load-bundles e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster
+test-e2e: run image-registry build-push-e2e-catalog registry-load-bundles apply-rbac e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster
 
 .PHONY: extension-developer-e2e
 extension-developer-e2e: KIND_CLUSTER_NAME := operator-controller-ext-dev-e2e  #EXHELP Run extension-developer e2e on local kind cluster
@@ -193,6 +193,9 @@ registry-load-bundles: ## Load selected e2e testdata container images created in
 	testdata/bundles/registry-v1/build-push-e2e-bundle.sh ${E2E_REGISTRY_NAMESPACE} $(REGISTRY_ROOT)/bundles/registry-v1/prometheus-operator:v1.2.0 prometheus-operator.v1.2.0 prometheus-operator.v1.0.0
 	testdata/bundles/registry-v1/build-push-e2e-bundle.sh ${E2E_REGISTRY_NAMESPACE} $(REGISTRY_ROOT)/bundles/registry-v1/prometheus-operator:v2.0.0 prometheus-operator.v2.0.0 prometheus-operator.v1.0.0
 
+apply-rbac: ## Apply RBAC expected when using service account from spec
+	kubectl apply -f testdata/rbac/prometheus-operator-bundle-rbac.yaml -n default
+
 #SECTION Build
 
 ifeq ($(origin VERSION), undefined)
@@ -238,7 +241,7 @@ run: docker-build kind-cluster kind-load kind-deploy #HELP Build the operator-co
 
 .PHONY: docker-build
 docker-build: build-linux #EXHELP Build docker image for operator-controller with GOOS=linux and local GOARCH.
-	$(CONTAINER_RUNTIME) build -t $(IMG) -f Dockerfile ./bin/linux
+	$(CONTAINER_RUNTIME) build -t $(IMG) -f Dockerfile ./bin/linux --load
 
 #SECTION Release
 ifeq ($(origin ENABLE_RELEASE_PIPELINE), undefined)

api/v1alpha1/clusterextension_types.go

@@ -78,6 +78,12 @@ type ClusterExtensionSpec struct {
 	// the bundle may contain resources that are cluster-scoped or that are
 	// installed in a different namespace. This namespace is expected to exist.
 	InstallNamespace string `json:"installNamespace"`
+
+	//+kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$
+	//+kubebuilder:validation:MaxLength:=253
+	// ServiceAccountName is the name of the ServiceAccount to use to manage the resources in the bundle.
+	// The service account is expected to exist in the InstallNamespace.
+	ServiceAccountName string `json:"serviceAccountName"`
 }
 
 const (

cmd/manager/main.go

@@ -28,7 +28,10 @@ import (
 	"go.uber.org/zap/zapcore"
 	k8slabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
+	"k8s.io/apimachinery/pkg/types"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
+	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	crcache "sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -44,6 +47,7 @@ import (
 	"github.com/operator-framework/rukpak/pkg/storage"
 
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
+	"github.com/operator-framework/operator-controller/internal/authentication"
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
 	catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
 	"github.com/operator-framework/operator-controller/internal/controllers"
@@ -159,19 +163,44 @@ func main() {
 	cl := mgr.GetClient()
 	catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(cachePath, httpClient))
 
-	installNamespaceMapper := helmclient.ObjectToStringMapper(func(obj client.Object) (string, error) {
-		ext := obj.(*ocv1alpha1.ClusterExtension)
+	saGetter, err := corev1client.NewForConfig(ctrl.GetConfigOrDie())
+	if err != nil {
+		setupLog.Error(err, "unable to create service account client")
+		os.Exit(1)
+	}
+
+	tg := authentication.NewTokenGetter(saGetter, 3600)
+	nsMapper := func(obj client.Object) (string, error) {
+		ext, ok := obj.(*ocv1alpha1.ClusterExtension)
+		if !ok {
+			return "", fmt.Errorf("cannot derive namespace from object of type %T", obj)
+		}
 		return ext.Spec.InstallNamespace, nil
-	})
+	}
+
+	rcm := func(ctx context.Context, obj client.Object, baseRestConfig *rest.Config) (*rest.Config, error) {
+		cfg := rest.AnonymousClientConfig(rest.CopyConfig(baseRestConfig))
+		ext, ok := obj.(*ocv1alpha1.ClusterExtension)
+		if !ok {
+			return cfg, nil
+		}
+		token, err := tg.Get(ctx, types.NamespacedName{Namespace: ext.Spec.InstallNamespace, Name: ext.Spec.ServiceAccountName})
+		if err != nil {
+			return nil, err
+		}
+		cfg.BearerToken = token
+		return cfg, nil
+	}
+
 	cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
-		helmclient.StorageNamespaceMapper(installNamespaceMapper),
-		helmclient.ClientNamespaceMapper(installNamespaceMapper),
+		helmclient.ClientNamespaceMapper(nsMapper),
+		helmclient.StorageNamespaceMapper(nsMapper),
+		helmclient.RestConfigMapper(rcm),
 	)
 	if err != nil {
 		setupLog.Error(err, "unable to config for creating helm client")
 		os.Exit(1)
 	}
-
 	acg, err := helmclient.NewActionClientGetter(cfgGetter)
 	if err != nil {
 		setupLog.Error(err, "unable to create helm client")

config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml

@@ -56,6 +56,13 @@ spec:
                 maxLength: 48
                 pattern: ^[a-z0-9]+(-[a-z0-9]+)*$
                 type: string
+              serviceAccountName:
+                description: |-
+                  ServiceAccountName is the name of the ServiceAccount to use to manage the resources in the bundle.
+                  The service account is expected to exist in the InstallNamespace.
+                maxLength: 253
+                pattern: ^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$
+                type: string
               upgradeConstraintPolicy:
                 default: Enforce
                 description: Defines the policy for how to handle upgrade constraints
@@ -77,6 +84,7 @@ spec:
             required:
             - installNamespace
             - packageName
+            - serviceAccountName
             type: object
           status:
             description: ClusterExtensionStatus defines the observed state of ClusterExtension

config/base/rbac/role.yaml

@@ -9,7 +9,8 @@ rules:
   resources:
   - '*'
   verbs:
-  - '*'
+  - list
+  - watch
 - apiGroups:
   - catalogd.operatorframework.io
   resources:
@@ -27,15 +28,16 @@ rules:
 - apiGroups:
   - ""
   resources:
-  - secrets
+  - configmaps
   verbs:
-  - create
-  - delete
-  - get
   - list
-  - patch
-  - update
   - watch
+- apiGroups:
+  - ""
+  resources:
+  - serviceaccounts/token
+  verbs:
+  - create
 - apiGroups:
   - olm.operatorframework.io
   resources:

config/samples/olm_v1alpha1_clusterextension.yaml

@@ -4,5 +4,6 @@ metadata:
   name: clusterextension-sample
 spec:
   installNamespace: default
+  serviceAccountName: default
   packageName: argocd-operator
   version: 0.6.0

internal/authentication/tokengetter.go

@@ -0,0 +1,97 @@
+package authentication
+
+import (
+	"context"
+	"sync"
+	"time"
+
+	authenticationv1 "k8s.io/api/authentication/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
+	"k8s.io/utils/ptr"
+)
+
+type TokenGetter struct {
+	client            corev1client.ServiceAccountsGetter
+	expirationSeconds int64
+	tokens            map[types.NamespacedName]*authenticationv1.TokenRequestStatus
+	tokenLocks        keyLock[types.NamespacedName]
+	mu                sync.RWMutex
+}
+
+func NewTokenGetter(client corev1client.ServiceAccountsGetter, expirationSeconds int64) *TokenGetter {
+	return &TokenGetter{
+		client:            client,
+		expirationSeconds: expirationSeconds,
+		tokenLocks:        newKeyLock[types.NamespacedName](),
+		tokens:            map[types.NamespacedName]*authenticationv1.TokenRequestStatus{},
+	}
+}
+
+type keyLock[K comparable] struct {
+	locks map[K]*sync.Mutex
+	mu    sync.Mutex
+}
+
+func newKeyLock[K comparable]() keyLock[K] {
+	return keyLock[K]{locks: map[K]*sync.Mutex{}}
+}
+
+func (k *keyLock[K]) Lock(key K) {
+	k.getLock(key).Lock()
+}
+
+func (k *keyLock[K]) Unlock(key K) {
+	k.getLock(key).Unlock()
+}
+
+func (k *keyLock[K]) getLock(key K) *sync.Mutex {
+	k.mu.Lock()
+	defer k.mu.Unlock()
+
+	lock, ok := k.locks[key]
+	if !ok {
+		lock = &sync.Mutex{}
+		k.locks[key] = lock
+	}
+	return lock
+}
+
+func (t *TokenGetter) Get(ctx context.Context, key types.NamespacedName) (string, error) {
+	t.tokenLocks.Lock(key)
+	defer t.tokenLocks.Unlock(key)
+
+	t.mu.RLock()
+	token, ok := t.tokens[key]
+	t.mu.RUnlock()
+
+	expireTime := time.Time{}
+	if ok {
+		expireTime = token.ExpirationTimestamp.Time
+	}
+
+	fiveMinutesAfterNow := metav1.Now().Add(5 * time.Minute)
+	if expireTime.Before(fiveMinutesAfterNow) {
+		var err error
+		token, err = t.getToken(ctx, key)
+		if err != nil {
+			return "", err
+		}
+		t.mu.Lock()
+		t.tokens[key] = token
+		t.mu.Unlock()
+	}
+
+	return token.Token, nil
+}
+
+func (t *TokenGetter) getToken(ctx context.Context, key types.NamespacedName) (*authenticationv1.TokenRequestStatus, error) {
+	req, err := t.client.ServiceAccounts(key.Namespace).CreateToken(ctx, key.Name, &authenticationv1.TokenRequest{Spec: authenticationv1.TokenRequestSpec{
+		ExpirationSeconds: ptr.To[int64](3600),
+	}}, metav1.CreateOptions{})
+	if err != nil {
+		return nil, err
+	}
+	return &req.Status, nil
+}

internal/controllers/clusterextension_admission_test.go

@@ -43,8 +43,9 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
 			t.Parallel()
 			cl := newClient(t)
 			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
-				PackageName:      tc.pkgName,
-				InstallNamespace: "default",
+				PackageName:        tc.pkgName,
+				InstallNamespace:   "default",
+				ServiceAccountName: "default",
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -131,9 +132,10 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
 			t.Parallel()
 			cl := newClient(t)
 			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
-				PackageName:      "package",
-				Version:          tc.version,
-				InstallNamespace: "default",
+				PackageName:        "package",
+				Version:            tc.version,
+				InstallNamespace:   "default",
+				ServiceAccountName: "default",
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -176,9 +178,10 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
 			t.Parallel()
 			cl := newClient(t)
 			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
-				PackageName:      "package",
-				Channel:          tc.channelName,
-				InstallNamespace: "default",
+				PackageName:        "package",
+				Channel:            tc.channelName,
+				InstallNamespace:   "default",
+				ServiceAccountName: "default",
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for channel %q: %w", tc.channelName, err)
@@ -222,8 +225,9 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 			t.Parallel()
 			cl := newClient(t)
 			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
-				PackageName:      "package",
-				InstallNamespace: tc.installNamespace,
+				PackageName:        "package",
+				InstallNamespace:   tc.installNamespace,
+				ServiceAccountName: "default",
 			}))
 			if tc.errMsg == "" {
 				require.NoError(t, err, "unexpected error for installNamespace %q: %w", tc.installNamespace, err)
@@ -235,6 +239,51 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
 	}
 }
 
+func TestClusterExtensionAdmissionServiceAccountName(t *testing.T) {
+	tooLongError := "spec.serviceAccountName: Too long: may not be longer than 253"
+	regexMismatchError := "spec.serviceAccountName in body should match"
+
+	testCases := []struct {
+		name               string
+		serviceAccountName string
+		errMsg             string
+	}{
+		{"just alphanumeric", "justalphanumberic1", ""},
+		{"hypen-separated", "hyphenated-name", ""},
+		{"no  service account name", "", regexMismatchError},
+		{"longest valid  service account name", strings.Repeat("x", 253), ""},
+		{"too long service account name", strings.Repeat("x", 254), tooLongError},
+		{"spaces", "spaces spaces", regexMismatchError},
+		{"capitalized", "Capitalized", regexMismatchError},
+		{"camel case", "camelCase", regexMismatchError},
+		{"invalid characters", "many/invalid$characters+in_name", regexMismatchError},
+		{"starts with hyphen", "-start-with-hyphen", regexMismatchError},
+		{"ends with hyphen", "end-with-hyphen-", regexMismatchError},
+		{"starts with period", ".start-with-period", regexMismatchError},
+		{"ends with period", "end-with-period.", regexMismatchError},
+	}
+
+	t.Parallel()
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			cl := newClient(t)
+			err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
+				PackageName:        "package",
+				InstallNamespace:   "default",
+				ServiceAccountName: tc.serviceAccountName,
+			}))
+			if tc.errMsg == "" {
+				require.NoError(t, err, "unexpected error for serviceAccountName %q: %w", tc.serviceAccountName, err)
+			} else {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.errMsg)
+			}
+		})
+	}
+}
+
 func buildClusterExtension(spec ocv1alpha1.ClusterExtensionSpec) *ocv1alpha1.ClusterExtension {
 	return &ocv1alpha1.ClusterExtension{
 		ObjectMeta: metav1.ObjectMeta{

internal/controllers/clusterextension_controller.go

@@ -103,12 +103,15 @@ const (
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/status,verbs=update;patch
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions/finalizers,verbs=update
-//+kubebuilder:rbac:groups=core,resources=secrets,verbs=create;update;patch;delete;get;list;watch
-//+kubebuilder:rbac:groups=*,resources=*,verbs=*
+
+//+kubebuilder:rbac:groups=core,resources=configmaps,verbs=list;watch
+//+kubebuilder:rbac:groups=core,resources=serviceaccounts/token,verbs=create
 
 //+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=clustercatalogs,verbs=list;watch
 //+kubebuilder:rbac:groups=catalogd.operatorframework.io,resources=catalogmetadata,verbs=list;watch
 
+//+kubebuilder:rbac:groups=*,resources=*,verbs=list;watch
+
 // The operator controller needs to watch all the bundle objects and reconcile accordingly. Though not ideal, but these permissions are required.
 // This has been taken from rukpak, and an issue was created before to discuss it: https://github.com/operator-framework/rukpak/issues/800.
 func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {