diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 2d78edda4f..f02818f1d5 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -598,7 +598,12 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl return err } - // TODO: add support for preflight checks + // determine if PreAuthorizer should be enabled based on feature gate + var preAuth authorization.PreAuthorizer + if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) { + preAuth = authorization.NewRBACPreAuthorizer(c.mgr.GetClient()) + } + // TODO: better scheme handling - which types do we want to support? _ = apiextensionsv1.AddToScheme(c.mgr.GetScheme()) rg := &applier.SimpleRevisionGenerator{ @@ -610,6 +615,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl Scheme: c.mgr.GetScheme(), RevisionGenerator: rg, Preflights: c.preflights, + PreAuthorizer: preAuth, FieldOwner: fmt.Sprintf("%s/clusterextension-controller", fieldOwnerPrefix), } revisionStatesGetter := &controllers.BoxcutterRevisionStatesGetter{Reader: c.mgr.GetClient()} diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 63578e9cb8..6885d56c6a 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "io" "io/fs" "maps" "slices" @@ -16,6 +17,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -25,6 +28,7 @@ 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/labels" "github.com/operator-framework/operator-controller/internal/shared/util/cache" ) @@ -279,28 +283,27 @@ type Boxcutter struct { Scheme *runtime.Scheme RevisionGenerator ClusterExtensionRevisionGenerator Preflights []Preflight + PreAuthorizer authorization.PreAuthorizer FieldOwner string } -func (bc *Boxcutter) getObjects(rev *ocv1.ClusterExtensionRevision) []client.Object { - var objs []client.Object - for _, phase := range rev.Spec.Phases { - for _, phaseObject := range phase.Objects { - objs = append(objs, &phaseObject.Object) - } - } - return objs -} - -func (bc *Boxcutter) createOrUpdate(ctx context.Context, obj client.Object) error { - if obj.GetObjectKind().GroupVersionKind().Empty() { - gvk, err := apiutil.GVKForObject(obj, bc.Scheme) +// createOrUpdate creates or updates the revision object. PreAuthorization checks are performed to ensure the +// manifestManager has sufficient permissions to manage the revision and its resources +func (bc *Boxcutter) createOrUpdate(ctx context.Context, manifestManager user.Info, rev *ocv1.ClusterExtensionRevision) error { + if rev.GetObjectKind().GroupVersionKind().Empty() { + gvk, err := apiutil.GVKForObject(rev, bc.Scheme) if err != nil { return err } - obj.GetObjectKind().SetGroupVersionKind(gvk) + rev.GetObjectKind().SetGroupVersionKind(gvk) } - return bc.Client.Patch(ctx, obj, client.Apply, client.FieldOwner(bc.FieldOwner), client.ForceOwnership) + + // Run auth preflight checks + if err := bc.runPreAuthorizationChecks(ctx, manifestManager, rev); err != nil { + return err + } + + return bc.Client.Patch(ctx, rev, client.Apply, client.FieldOwner(bc.FieldOwner), client.ForceOwnership) } func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) error { @@ -329,7 +332,7 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust desiredRevision.Spec.Revision = currentRevision.Spec.Revision desiredRevision.Name = currentRevision.Name - err := bc.createOrUpdate(ctx, desiredRevision) + err := bc.createOrUpdate(ctx, getUserInfo(ext), desiredRevision) switch { case apierrors.IsInvalid(err): // We could not update the current revision due to trying to update an immutable field. @@ -344,7 +347,7 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust } // Preflights - plainObjs := bc.getObjects(desiredRevision) + plainObjs := getObjects(desiredRevision) for _, preflight := range bc.Preflights { if shouldSkipPreflight(ctx, preflight, ext, state) { continue @@ -379,7 +382,7 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust return fmt.Errorf("garbage collecting old revisions: %w", err) } - if err := bc.createOrUpdate(ctx, desiredRevision); err != nil { + if err := bc.createOrUpdate(ctx, getUserInfo(ext), desiredRevision); err != nil { return fmt.Errorf("creating new Revision: %w", err) } } @@ -387,6 +390,23 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust return nil } +// runPreAuthorizationChecks runs PreAuthorization checks if the PreAuthorizer is set. An error will be returned if +// the ClusterExtension service account does not have the necessary permissions to manage the revision's resources +func (bc *Boxcutter) runPreAuthorizationChecks(ctx context.Context, manifestManager user.Info, rev *ocv1.ClusterExtensionRevision) error { + if bc.PreAuthorizer == nil { + return nil + } + + // collect the revision manifests + manifestReader, err := revisionManifestReader(rev) + if err != nil { + return err + } + + // run preauthorization check + return formatPreAuthorizerOutput(bc.PreAuthorizer.PreAuthorize(ctx, manifestManager, manifestReader, revisionManagementPerms(rev))) +} + // garbageCollectOldRevisions deletes archived revisions beyond ClusterExtensionRevisionRetentionLimit. // Active revisions are never deleted. revisionList must be sorted oldest to newest. func (bc *Boxcutter) garbageCollectOldRevisions(ctx context.Context, revisionList []ocv1.ClusterExtensionRevision) error { @@ -445,3 +465,45 @@ func splitManifestDocuments(file string) []string { } return docs } + +// getObjects returns a slice of all objects in the revision +func getObjects(rev *ocv1.ClusterExtensionRevision) []client.Object { + var objs []client.Object + for _, phase := range rev.Spec.Phases { + for _, phaseObject := range phase.Objects { + objs = append(objs, &phaseObject.Object) + } + } + return objs +} + +// revisionManifestReader returns an io.Reader containing all manifests in the revision +func revisionManifestReader(rev *ocv1.ClusterExtensionRevision) (io.Reader, error) { + var manifestBuilder strings.Builder + for _, obj := range getObjects(rev) { + objBytes, err := yaml.Marshal(obj) + if err != nil { + return nil, fmt.Errorf("error generating revision manifest: %w", err) + } + manifestBuilder.WriteString("---\n") + manifestBuilder.WriteString(string(objBytes)) + manifestBuilder.WriteString("\n") + } + return strings.NewReader(manifestBuilder.String()), nil +} + +func revisionManagementPerms(rev *ocv1.ClusterExtensionRevision) func(user.Info) []authorizer.AttributesRecord { + return func(user user.Info) []authorizer.AttributesRecord { + return []authorizer.AttributesRecord{ + { + User: user, + Name: rev.Name, + APIGroup: ocv1.GroupVersion.Group, + APIVersion: ocv1.GroupVersion.Version, + Resource: "clusterextensionrevisions/finalizers", + ResourceRequest: true, + Verb: "update", + }, + } + } +} diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 081747285c..6b66aec5ea 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "io/fs" "strings" "testing" @@ -16,11 +17,14 @@ import ( "helm.sh/helm/v3/pkg/storage/driver" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" 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/util/validation/field" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" k8scheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -28,6 +32,7 @@ import ( 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/labels" ) @@ -948,6 +953,191 @@ func TestBoxcutter_Apply(t *testing.T) { } } +func Test_PreAuthorizer_Integration(t *testing.T) { + testScheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(testScheme)) + + // This is the revision that the mock builder will produce by default. + // We calculate its hash to use in the tests. + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ext", + UID: "test-uid", + }, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "test-namespace", + ServiceAccount: ocv1.ServiceAccountReference{ + Name: "test-sa", + }, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(testScheme).Build() + dummyGenerator := &mockBundleRevisionBuilder{ + makeRevisionFunc: func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotation map[string]string) (*ocv1.ClusterExtensionRevision, error) { + return &ocv1.ClusterExtensionRevision{ + Spec: ocv1.ClusterExtensionRevisionSpec{ + Phases: []ocv1.ClusterExtensionRevisionPhase{ + { + Name: "some-phase", + Objects: []ocv1.ClusterExtensionRevisionObject{ + { + Object: unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "data": map[string]string{ + "test-data": "test-data", + }, + }, + }, + }, + }, + }, + }, + }, + }, nil + }, + } + dummyBundleFs := fstest.MapFS{} + revisionAnnotations := map[string]string{} + + for _, tc := range []struct { + name string + preAuthorizer func(t *testing.T) authorization.PreAuthorizer + validate func(t *testing.T, err error) + }{ + { + name: "preauthorizer called with correct parameters", + preAuthorizer: func(t *testing.T) authorization.PreAuthorizer { + return &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + require.Equal(t, "system:serviceaccount:test-namespace:test-sa", user.GetName()) + require.Empty(t, user.GetUID()) + require.Nil(t, user.GetExtra()) + require.Empty(t, user.GetGroups()) + + t.Log("has correct additional permissions") + require.Len(t, additionalRequiredPerms, 1) + perms := additionalRequiredPerms[0](user) + + require.Len(t, perms, 1) + require.Equal(t, authorizer.AttributesRecord{ + User: user, + Name: "test-ext-1", + APIGroup: "olm.operatorframework.io", + APIVersion: "v1", + Resource: "clusterextensionrevisions/finalizers", + ResourceRequest: true, + Verb: "update", + }, perms[0]) + + t.Log("has correct manifest reader") + manifests, err := io.ReadAll(reader) + require.NoError(t, err) + require.Equal(t, "---\napiVersion: v1\ndata:\n test-data: test-data\nkind: ConfigMap\n\n", string(manifests)) + return nil, nil + }, + } + }, + }, { + name: "preauthorizer errors are returned", + preAuthorizer: func(t *testing.T) authorization.PreAuthorizer { + return &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return nil, errors.New("test error") + }, + } + }, + validate: func(t *testing.T, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "pre-authorization failed") + require.Contains(t, err.Error(), "authorization evaluation error: test error") + }, + }, { + name: "preauthorizer missing permissions are returned as an error", + preAuthorizer: func(t *testing.T) authorization.PreAuthorizer { + return &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return []authorization.ScopedPolicyRules{ + { + Namespace: "", + MissingRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + }, nil + }, + } + }, + validate: func(t *testing.T, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "pre-authorization failed") + require.Contains(t, err.Error(), "service account requires the following permissions") + require.Contains(t, err.Error(), "Resources:[pods]") + require.Contains(t, err.Error(), "Verbs:[get,list,watch]") + }, + }, { + name: "preauthorizer missing permissions and errors are combined and returned as an error", + preAuthorizer: func(t *testing.T) authorization.PreAuthorizer { + return &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return []authorization.ScopedPolicyRules{ + { + Namespace: "", + MissingRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + }, + }, errors.New("test error") + }, + } + }, + validate: func(t *testing.T, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "pre-authorization failed") + require.Contains(t, err.Error(), "service account requires the following permissions") + require.Contains(t, err.Error(), "Resources:[pods]") + require.Contains(t, err.Error(), "Verbs:[get,list,watch]") + require.Contains(t, err.Error(), "authorization evaluation error: test error") + }, + }, { + name: "successful call to preauthorizer does not block applier", + preAuthorizer: func(t *testing.T) authorization.PreAuthorizer { + return &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return nil, nil + }, + } + }, + validate: func(t *testing.T, err error) { + require.NoError(t, err) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + boxcutter := &applier.Boxcutter{ + Client: fakeClient, + Scheme: testScheme, + FieldOwner: "test-owner", + RevisionGenerator: dummyGenerator, + PreAuthorizer: tc.preAuthorizer(t), + } + err := boxcutter.Apply(t.Context(), dummyBundleFs, ext, nil, revisionAnnotations) + if tc.validate != nil { + tc.validate(t, err) + } + }) + } +} + func TestBoxcutterStorageMigrator(t *testing.T) { t.Run("creates revision", func(t *testing.T) { testScheme := runtime.NewScheme() diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 22ed096fe1..86b5440bc4 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -19,6 +19,8 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apimachyaml "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -77,29 +79,8 @@ func (h *Helm) runPreAuthorizationChecks(ctx context.Context, ext *ocv1.ClusterE 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 + manifestManager := getUserInfo(ext) + return formatPreAuthorizerOutput(h.PreAuthorizer.PreAuthorize(ctx, manifestManager, strings.NewReader(tmplRel.Manifest), extManagementPerms(ext))) } func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) (bool, string, error) { @@ -328,3 +309,48 @@ func ruleDescription(ns string, rule rbacv1.PolicyRule) string { } return sb.String() } + +// formatPreAuthorizerOutput formats the output of PreAuthorizer.PreAuthorize calls into a consistent and deterministic error. +// If the call returns no missing rules, and no error, nil is returned. +func formatPreAuthorizerOutput(missingRules []authorization.ScopedPolicyRules, authErr error) error { + 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 getUserInfo(ext *ocv1.ClusterExtension) user.Info { + return &user.DefaultInfo{Name: fmt.Sprintf("system:serviceaccount:%s:%s", ext.Spec.Namespace, ext.Spec.ServiceAccount.Name)} +} + +func extManagementPerms(ext *ocv1.ClusterExtension) func(user.Info) []authorizer.AttributesRecord { + return func(user user.Info) []authorizer.AttributesRecord { + return []authorizer.AttributesRecord{ + { + User: user, + Name: ext.Name, + APIGroup: ocv1.GroupVersion.Group, + APIVersion: ocv1.GroupVersion.Version, + Resource: "clusterextensions/finalizers", + ResourceRequest: true, + Verb: "update", + }, + } + } +} diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index 8d07de9123..a1332812d4 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -15,6 +15,9 @@ import ( "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" "sigs.k8s.io/controller-runtime/pkg/client" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" @@ -70,16 +73,11 @@ type mockPreflight struct { } type mockPreAuthorizer struct { - missingRules []authorization.ScopedPolicyRules - returnError error + fn func(context.Context, user.Info, io.Reader, ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) } -func (p *mockPreAuthorizer) PreAuthorize( - ctx context.Context, - ext *ocv1.ClusterExtension, - manifestReader io.Reader, -) ([]authorization.ScopedPolicyRules, error) { - return p.missingRules, p.returnError +func (p *mockPreAuthorizer) PreAuthorize(ctx context.Context, manifestManager user.Info, manifestReader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return p.fn(ctx, manifestManager, manifestReader, additionalRequiredPerms...) } func (mp *mockPreflight) Install(context.Context, []client.Object) error { @@ -193,7 +191,17 @@ metadata: spec: clusterIP: 0.0.0.0` - testCE = &ocv1.ClusterExtension{} + testCE = &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ext", + }, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "test-namespace", + ServiceAccount: ocv1.ServiceAccountReference{ + Name: "test-sa", + }, + }, + } testObjectLabels = map[string]string{"object": "label"} testStorageLabels = map[string]string{"storage": "label"} errPreAuth = errors.New("problem running preauthorization") @@ -345,6 +353,52 @@ func TestApply_Installation(t *testing.T) { } func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { + t.Run("preauthorizer called with correct parameters", 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{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + t.Log("has correct user") + require.Equal(t, "system:serviceaccount:test-namespace:test-sa", user.GetName()) + require.Empty(t, user.GetUID()) + require.Nil(t, user.GetExtra()) + require.Empty(t, user.GetGroups()) + + t.Log("has correct additional permissions") + require.Len(t, additionalRequiredPerms, 1) + perms := additionalRequiredPerms[0](user) + + require.Len(t, perms, 1) + require.Equal(t, authorizer.AttributesRecord{ + User: user, + Name: "test-ext", + APIGroup: "olm.operatorframework.io", + APIVersion: "v1", + Resource: "clusterextensions/finalizers", + ResourceRequest: true, + Verb: "update", + }, perms[0]) + return nil, nil + }, + }, + HelmChartProvider: DummyHelmChartProvider, + HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, + } + + _, _, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) + require.Error(t, err) + }) + t.Run("fails during dry-run installation", func(t *testing.T) { mockAcg := &mockActionGetter{ getClientErr: driver.ErrReleaseNotFound, @@ -373,9 +427,13 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { } mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + PreAuthorizer: &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return nil, nil + }, + }, HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } @@ -397,8 +455,12 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth}, - HelmChartProvider: DummyHelmChartProvider, + PreAuthorizer: &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return nil, errPreAuth + }, + }, + HelmChartProvider: DummyHelmChartProvider, } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -426,8 +488,12 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil}, - HelmChartProvider: DummyHelmChartProvider, + PreAuthorizer: &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return missingRBAC, nil + }, + }, + HelmChartProvider: DummyHelmChartProvider, } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -454,8 +520,12 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{ + fn: func(ctx context.Context, user user.Info, reader io.Reader, additionalRequiredPerms ...authorization.UserAuthorizerAttributesFactory) ([]authorization.ScopedPolicyRules, error) { + return nil, nil + }, + }, HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, Manager: &mockManagedContentCacheManager{ diff --git a/internal/operator-controller/authorization/rbac.go b/internal/operator-controller/authorization/rbac.go index 357268615c..5c98880fda 100644 --- a/internal/operator-controller/authorization/rbac.go +++ b/internal/operator-controller/authorization/rbac.go @@ -31,12 +31,13 @@ import ( "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" ) +// UserAuthorizerAttributesFactory is a function that produces a slice of AttributesRecord for user +type UserAuthorizerAttributesFactory func(user user.Info) []authorizer.AttributesRecord + type PreAuthorizer interface { - PreAuthorize(ctx context.Context, ext *ocv1.ClusterExtension, manifestReader io.Reader) ([]ScopedPolicyRules, error) + PreAuthorize(ctx context.Context, user user.Info, manifestReader io.Reader, additionalRequiredPerms ...UserAuthorizerAttributesFactory) ([]ScopedPolicyRules, error) } type ScopedPolicyRules struct { @@ -68,9 +69,10 @@ func NewRBACPreAuthorizer(cl client.Client) PreAuthorizer { } } -// PreAuthorize validates whether the current user/request satisfies the necessary permissions +// PreAuthorize validates whether the user 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. +// the intended action to determine if the operation is allowed. Optional additional required permissions are also evaluated +// against user. // // Return Value: // - nil: indicates that the authorization check passed and the operation is permitted. @@ -79,13 +81,19 @@ func NewRBACPreAuthorizer(cl client.Client) PreAuthorizer { // 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) { +func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, user user.Info, manifestReader io.Reader, additionalRequiredPerms ...UserAuthorizerAttributesFactory) ([]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) + + // derive manifest related attributes records + attributesRecords := dm.asAuthorizationAttributesRecordsForUser(user) + + // append additional required perms + for _, fn := range additionalRequiredPerms { + attributesRecords = append(attributesRecords, fn(user)...) + } var preAuthEvaluationErrors []error missingRules, err := a.authorizeAttributesRecords(ctx, attributesRecords) @@ -97,7 +105,7 @@ func (a *rbacPreAuthorizer) PreAuthorize(ctx context.Context, ext *ocv1.ClusterE var parseErrors []error for _, obj := range dm.rbacObjects() { - if err := ec.checkEscalation(ctx, manifestManager, obj); err != nil { + if err := ec.checkEscalation(ctx, user, obj); err != nil { result, err := parseEscalationErrorForMissingRules(err) missingRules[obj.GetNamespace()] = append(missingRules[obj.GetNamespace()], result.MissingRules...) preAuthEvaluationErrors = append(preAuthEvaluationErrors, result.ResolutionErrors) @@ -316,7 +324,7 @@ func (dm *decodedManifest) rbacObjects() []client.Object { return objects } -func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info, ext *ocv1.ClusterExtension) []authorizer.AttributesRecord { +func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManager user.Info) []authorizer.AttributesRecord { var attributeRecords []authorizer.AttributesRecord for gvr, keys := range dm.gvrs { @@ -363,18 +371,6 @@ func (dm *decodedManifest) asAuthorizationAttributesRecordsForUser(manifestManag 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 } diff --git a/internal/operator-controller/authorization/rbac_test.go b/internal/operator-controller/authorization/rbac_test.go index 8e3d47687d..9d80cc52bb 100644 --- a/internal/operator-controller/authorization/rbac_test.go +++ b/internal/operator-controller/authorization/rbac_test.go @@ -14,12 +14,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/authorization/authorizer" "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 ( @@ -129,17 +128,9 @@ subjects: 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, - }, - }, - } + saName = "test-serviceaccount" + ns = "test-namespace" + testUser = &user.DefaultInfo{Name: fmt.Sprintf("system:serviceaccount:%s:%s", ns, saName)} objects = []client.Object{ &corev1.Namespace{ @@ -204,7 +195,7 @@ subjects: Rules: []rbacv1.PolicyRule{ { APIGroups: []string{"*"}, - Resources: []string{"serviceaccounts", "services", "clusterextensions/finalizers"}, + Resources: []string{"serviceaccounts", "services"}, Verbs: []string{"*"}, }, { @@ -236,12 +227,6 @@ subjects: 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), }, }, @@ -310,12 +295,6 @@ subjects: 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), }, }, @@ -418,7 +397,7 @@ 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)) + missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest)) require.NoError(t, err) require.Equal(t, []ScopedPolicyRules{}, missingRules) }) @@ -428,7 +407,7 @@ 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)) + missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest)) require.NoError(t, err) require.Equal(t, expectedSingleNamespaceMissingRules, missingRules) }) @@ -438,7 +417,7 @@ 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)) + missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifestMultiNamespace)) require.NoError(t, err) require.Equal(t, expectedMultiNamespaceMissingRules, missingRules) }) @@ -448,12 +427,44 @@ 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)) + missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest)) require.NoError(t, err) require.Equal(t, []ScopedPolicyRules{}, missingRules) }) } +func TestPreAuthorize_AdditionalRequiredPerms_MissingRBAC(t *testing.T) { + t.Run("preauthorize fails and finds missing rbac rules coming from the additional required permissions", func(t *testing.T) { + fakeClient := setupFakeClient(escalatingClusterRole) + preAuth := NewRBACPreAuthorizer(fakeClient) + missingRules, err := preAuth.PreAuthorize(context.TODO(), testUser, strings.NewReader(testManifest), func(user user.Info) []authorizer.AttributesRecord { + return []authorizer.AttributesRecord{ + { + User: user, + Verb: "create", + APIGroup: corev1.SchemeGroupVersion.Group, + APIVersion: corev1.SchemeGroupVersion.Version, + Resource: "pods", + ResourceRequest: true, + }, + } + }) + require.NoError(t, err) + require.Equal(t, []ScopedPolicyRules{ + { + Namespace: "", + MissingRules: []rbacv1.PolicyRule{ + { + Verbs: []string{"create"}, + APIGroups: []string{""}, + Resources: []string{"pods"}, + }, + }, + }, + }, 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) { diff --git a/test/e2e/features/recover.feature b/test/e2e/features/recover.feature index 0438f2d1a1..6f48848ad3 100644 --- a/test/e2e/features/recover.feature +++ b/test/e2e/features/recover.feature @@ -4,7 +4,6 @@ Feature: Recover cluster extension from errors that might occur during its lifet Given OLM is available And ClusterCatalog "test" serves bundles - Scenario: Restore removed resource Given ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE} And ClusterExtension is applied @@ -115,3 +114,37 @@ Feature: Recover cluster extension from errors that might occur during its lifet Then ClusterExtension is available And ClusterExtension reports Progressing as True with Reason Succeeded And ClusterExtension reports Installed as True + + @PreflightPermissions + Scenario: Add missing permissions + Given ServiceAccount "olm-sa" is available in ${TEST_NAMESPACE} + And ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + namespace: ${TEST_NAMESPACE} + serviceAccount: + name: olm-sa + source: + sourceType: Catalog + catalog: + packageName: test + selector: + matchLabels: + "olm.operatorframework.io/metadata.name": test-catalog + """ + And ClusterExtension reports Progressing as True with Reason Retrying and Message includes: + """ + error for resolved bundle "test-operator.1.2.0" with version "1.2.0": creating new Revision: pre-authorization failed: service account requires the following permissions to manage cluster extension: + """ + And ClusterExtension reports Progressing as True with Reason Retrying and Message includes: + """ + Namespace:"" APIGroups:[apiextensions.k8s.io] Resources:[customresourcedefinitions] ResourceNames:[olme2etests.olm.operatorframework.io] Verbs:[delete,get,patch,update] + """ + When ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE} + Then ClusterExtension is available + And ClusterExtension reports Progressing as True with Reason Succeeded + And ClusterExtension reports Installed as True diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index e1a55934dd..e52c37a4d2 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -57,6 +57,7 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)ClusterExtension is rolled out$`, ClusterExtensionIsRolledOut) sc.Step(`^(?i)ClusterExtension reports "([^"]+)" as active revision(s?)$`, ClusterExtensionReportsActiveRevisions) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message:$`, ClusterExtensionReportsCondition) + sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message includes:$`, ClusterExtensionReportsConditionWithMessageFragment) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+)$`, ClusterExtensionReportsConditionWithoutMsg) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+)$`, ClusterExtensionReportsConditionWithoutReason) sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+)$`, ClusterExtensionRevisionReportsConditionWithoutMsg) @@ -75,6 +76,7 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in test namespace$`, ServiceAccountWithNeededPermissionsIsAvailableInNamespace) sc.Step(`^(?i)ServiceAccount "([^"]*)" with needed permissions is available in \${TEST_NAMESPACE}$`, ServiceAccountWithNeededPermissionsIsAvailableInNamespace) + sc.Step(`^(?i)ServiceAccount "([^"]*)" is available in \${TEST_NAMESPACE}$`, ServiceAccountIsAvailableInNamespace) sc.Step(`^(?i)ServiceAccount "([^"]*)" in test namespace is cluster admin$`, ServiceAccountWithClusterAdminPermissionsIsAvailableInNamespace) sc.Step(`^(?i)ServiceAccount "([^"]+)" in test namespace has permissions to fetch "([^"]+)" metrics$`, ServiceAccountWithFetchMetricsPermissions) sc.Step(`^(?i)ServiceAccount "([^"]+)" sends request to "([^"]+)" endpoint of "([^"]+)" service$`, SendMetricsRequest) @@ -169,13 +171,7 @@ func substituteScenarioVars(content string, sc *scenarioContext) string { if v, found := os.LookupEnv("CATALOG_IMG"); found { vars["CATALOG_IMG"] = v } - m := func(k string) string { - if v, found := vars[k]; found { - return v - } - return "" - } - return os.Expand(content, m) + return templateContent(content, vars) } func ResourceApplyFails(ctx context.Context, errMsg string, yamlTemplate *godog.DocString) error { @@ -267,7 +263,21 @@ func waitFor(ctx context.Context, conditionFn func() bool) { require.Eventually(godog.T(ctx), conditionFn, timeout, tick) } -func waitForCondition(ctx context.Context, resourceType, resourceName, conditionType, conditionStatus string, conditionReason *string, msg *string) error { +type conditionMessageCmp func(string) bool + +func condMsgEquals(expected string) conditionMessageCmp { + return func(actual string) bool { + return actual == expected + } +} + +func condMsgContains(expected string) conditionMessageCmp { + return func(actual string) bool { + return strings.Contains(actual, expected) + } +} + +func waitForCondition(ctx context.Context, resourceType, resourceName, conditionType, conditionStatus string, conditionReason *string, msgCmp *conditionMessageCmp) error { require.Eventually(godog.T(ctx), func() bool { v, err := k8sClient("get", resourceType, resourceName, "-o", fmt.Sprintf("jsonpath={.status.conditions[?(@.type==\"%s\")]}", conditionType)) if err != nil { @@ -284,7 +294,7 @@ func waitForCondition(ctx context.Context, resourceType, resourceName, condition if conditionReason != nil && condition["reason"] != *conditionReason { return false } - if msg != nil && condition["message"] != *msg { + if msgCmp != nil && !(*msgCmp)(condition["message"].(string)) { return false } @@ -293,17 +303,25 @@ func waitForCondition(ctx context.Context, resourceType, resourceName, condition return nil } -func waitForExtensionCondition(ctx context.Context, conditionType, conditionStatus string, conditionReason *string, msg *string) error { +func waitForExtensionCondition(ctx context.Context, conditionType, conditionStatus string, conditionReason *string, msgCmp *conditionMessageCmp) error { sc := scenarioCtx(ctx) - return waitForCondition(ctx, "clusterextension", sc.clusterExtensionName, conditionType, conditionStatus, conditionReason, msg) + return waitForCondition(ctx, "clusterextension", sc.clusterExtensionName, conditionType, conditionStatus, conditionReason, msgCmp) } func ClusterExtensionReportsCondition(ctx context.Context, conditionType, conditionStatus, conditionReason string, msg *godog.DocString) error { - var conditionMsg *string + var conditionMsgCmp *conditionMessageCmp if msg != nil { - conditionMsg = ptr.To(substituteScenarioVars(strings.Join(strings.Fields(msg.Content), " "), scenarioCtx(ctx))) + conditionMsgCmp = ptr.To(condMsgEquals(substituteScenarioVars(strings.Join(strings.Fields(msg.Content), " "), scenarioCtx(ctx)))) } - return waitForExtensionCondition(ctx, conditionType, conditionStatus, &conditionReason, conditionMsg) + return waitForExtensionCondition(ctx, conditionType, conditionStatus, &conditionReason, conditionMsgCmp) +} + +func ClusterExtensionReportsConditionWithMessageFragment(ctx context.Context, conditionType, conditionStatus, conditionReason string, msgFragment *godog.DocString) error { + var conditionMsgCmp *conditionMessageCmp + if msgFragment != nil { + conditionMsgCmp = ptr.To(condMsgContains(substituteScenarioVars(strings.Join(strings.Fields(msgFragment.Content), " "), scenarioCtx(ctx)))) + } + return waitForExtensionCondition(ctx, conditionType, conditionStatus, &conditionReason, conditionMsgCmp) } func ClusterExtensionReportsConditionWithoutMsg(ctx context.Context, conditionType, conditionStatus, conditionReason string) error { @@ -457,34 +475,45 @@ func ResourceRestored(ctx context.Context, resource string) error { return nil } -func applyPermissionsToServiceAccount(ctx context.Context, serviceAccount, rbacTemplate string, keyValue ...string) error { +func applyServiceAccount(ctx context.Context, serviceAccount string) error { sc := scenarioCtx(ctx) - yamlContent, err := os.ReadFile(filepath.Join("steps", "testdata", rbacTemplate)) + vars := extendMap(map[string]string{ + "TEST_NAMESPACE": sc.namespace, + "SERVICE_ACCOUNT_NAME": serviceAccount, + "SERVICEACCOUNT_NAME": serviceAccount, + }) + + yaml, err := templateYaml(filepath.Join("steps", "testdata", "serviceaccount-template.yaml"), vars) if err != nil { - return fmt.Errorf("failed to read RBAC template yaml: %v", err) + return fmt.Errorf("failed to template ServiceAccount yaml: %v", err) } - vars := map[string]string{ + // Apply the ServiceAccount configuration + _, err = k8scliWithInput(yaml, "apply", "-f", "-") + if err != nil { + return fmt.Errorf("failed to apply ServiceAccount configuration: %v: %s", err, stderrOutput(err)) + } + + return nil +} + +func applyPermissionsToServiceAccount(ctx context.Context, serviceAccount, rbacTemplate string, keyValue ...string) error { + sc := scenarioCtx(ctx) + if err := applyServiceAccount(ctx, serviceAccount); err != nil { + return err + } + vars := extendMap(map[string]string{ "TEST_NAMESPACE": sc.namespace, "SERVICE_ACCOUNT_NAME": serviceAccount, "SERVICEACCOUNT_NAME": serviceAccount, "CLUSTER_EXTENSION_NAME": sc.clusterExtensionName, "CLUSTEREXTENSION_NAME": sc.clusterExtensionName, - } - if len(keyValue) > 0 { - for i := 0; i < len(keyValue); i += 2 { - vars[keyValue[i]] = keyValue[i+1] - } - } - m := func(k string) string { - if v, found := vars[k]; found { - return v - } - return "" - } + }, keyValue...) - // Replace template variables - yaml := os.Expand(string(yamlContent), m) + yaml, err := templateYaml(filepath.Join("steps", "testdata", rbacTemplate), vars) + if err != nil { + return fmt.Errorf("failed to template RBAC yaml: %v", err) + } // Apply the RBAC configuration _, err = k8scliWithInput(yaml, "apply", "-f", "-") @@ -495,6 +524,10 @@ func applyPermissionsToServiceAccount(ctx context.Context, serviceAccount, rbacT return nil } +func ServiceAccountIsAvailableInNamespace(ctx context.Context, serviceAccount string) error { + return applyServiceAccount(ctx, serviceAccount) +} + func ServiceAccountWithNeededPermissionsIsAvailableInNamespace(ctx context.Context, serviceAccount string) error { return applyPermissionsToServiceAccount(ctx, serviceAccount, "rbac-template.yaml") } @@ -730,3 +763,32 @@ func MarkTestOperatorNotReady(ctx context.Context, state string) error { _, err = k8sClient("exec", podName, "-n", sc.namespace, "--", op, "/var/www/ready") return err } + +func templateYaml(template string, values map[string]string) (string, error) { + yamlContent, err := os.ReadFile(template) + if err != nil { + return "", fmt.Errorf("failed to read template file '%s': %v", template, err) + } + return templateContent(string(yamlContent), values), nil +} + +func templateContent(content string, values map[string]string) string { + m := func(k string) string { + if v, found := values[k]; found { + return v + } + return "" + } + + // Replace template variables + return os.Expand(content, m) +} + +func extendMap(m map[string]string, keyValue ...string) map[string]string { + if len(keyValue) > 0 { + for i := 0; i < len(keyValue); i += 2 { + m[keyValue[i]] = keyValue[i+1] + } + } + return m +} diff --git a/test/e2e/steps/testdata/rbac-template.yaml b/test/e2e/steps/testdata/rbac-template.yaml index d975d76988..4b819e5d40 100644 --- a/test/e2e/steps/testdata/rbac-template.yaml +++ b/test/e2e/steps/testdata/rbac-template.yaml @@ -1,15 +1,4 @@ --- -apiVersion: v1 -kind: Namespace -metadata: - name: ${TEST_NAMESPACE} ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: ${SERVICEACCOUNT_NAME} - namespace: ${TEST_NAMESPACE} ---- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: @@ -23,7 +12,6 @@ rules: - apiGroups: [olm.operatorframework.io] resources: [clusterextensionrevisions, clusterextensionrevisions/finalizers] verbs: [update, create, list, watch, get, delete, patch] - - apiGroups: [apiextensions.k8s.io] resources: [customresourcedefinitions] verbs: [update, create, list, watch, get, delete, patch] @@ -60,8 +48,6 @@ rules: - apiGroups: ["authentication.k8s.io"] resources: ["tokenreviews"] verbs: [create] - - --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding diff --git a/test/e2e/steps/testdata/serviceaccount-template.yaml b/test/e2e/steps/testdata/serviceaccount-template.yaml new file mode 100644 index 0000000000..5d1c464493 --- /dev/null +++ b/test/e2e/steps/testdata/serviceaccount-template.yaml @@ -0,0 +1,11 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: ${TEST_NAMESPACE} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: ${SERVICEACCOUNT_NAME} + namespace: ${TEST_NAMESPACE} \ No newline at end of file