From e083c055324d85fb0c52cad3947f79521d1494eb Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Fri, 26 Sep 2025 16:16:07 -0600 Subject: [PATCH] Folders: Create default permissions on root level folder in API Service and cleanup after delete (#111690) --- pkg/registry/apis/folders/folder_storage.go | 18 +-- pkg/registry/apis/folders/hooks.go | 38 +++++- pkg/registry/apis/folders/register.go | 116 ++++++++++++++++-- .../apis/provisioning/resources/folders.go | 2 + pkg/storage/unified/apistore/permissions.go | 7 +- .../unified/apistore/permissions_test.go | 40 +++--- 6 files changed, 171 insertions(+), 50 deletions(-) diff --git a/pkg/registry/apis/folders/folder_storage.go b/pkg/registry/apis/folders/folder_storage.go index 58f1af5eee5..8d96a494859 100644 --- a/pkg/registry/apis/folders/folder_storage.go +++ b/pkg/registry/apis/folders/folder_storage.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" - "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" @@ -112,6 +111,10 @@ func (s *folderStorage) Create(ctx context.Context, parentUid := accessor.GetFolder() + // TODO: once the feature flag kubernetesAuthzResourcePermissionApis is removed AND the frontend is calling + // /apis directly (to set AnnoKeyGrantPermissions on root level folders), the below should be removed + // and we should instead initialize resourcePermissionsSvc in the RegisterAPIService function + // and rely on StorageOptions.Permissions. err = s.setDefaultFolderPermissions(ctx, info.OrgID, user, p.Name, parentUid) if err != nil { return nil, err @@ -133,22 +136,11 @@ func (s *folderStorage) Update(ctx context.Context, // GracefulDeleter func (s *folderStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { - info, err := request.NamespaceInfoFrom(ctx, true) - if err != nil { - return nil, false, err - } - obj, async, err := s.store.Delete(ctx, name, deleteValidation, options) if err != nil { return obj, async, err } - if accessErr := s.folderPermissionsSvc.DeleteResourcePermissions(ctx, info.OrgID, name); accessErr != nil { - // TODO: add a proper logger to this struct. - logger := log.New().FromContext(ctx) - logger.Warn("failed to delete folder permission after successfully deleting folder resource", "folder", name, "error", accessErr) - } - return obj, async, err } @@ -157,7 +149,7 @@ func (s *folderStorage) DeleteCollection(ctx context.Context, deleteValidation r return nil, fmt.Errorf("DeleteCollection for folders not implemented") } -func (s *folderStorage) setDefaultFolderPermissions(ctx context.Context, orgID int64, user identity.Requester, uid string, parentUID string) error { +func (s *folderStorage) setDefaultFolderPermissions(ctx context.Context, orgID int64, user identity.Requester, uid, parentUID string) error { var permissions []accesscontrol.SetResourcePermissionCommand if user.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) { diff --git a/pkg/registry/apis/folders/hooks.go b/pkg/registry/apis/folders/hooks.go index 38942c19c5b..9adf40c1293 100644 --- a/pkg/registry/apis/folders/hooks.go +++ b/pkg/registry/apis/folders/hooks.go @@ -2,13 +2,18 @@ package folders import ( "context" + "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic/registry" + claims "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" + folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/util" ) // K8S docs say "Almost nobody should use this hook" about the "begin" hooks, but we do because we only need to @@ -77,10 +82,35 @@ func (b *FolderAPIBuilder) afterDelete(obj runtime.Object, _ *metav1.DeleteOptio return } - log.Info("Propagating deleted folder to Zanzana", "folder", meta.GetName(), "parent", meta.GetFolder()) - err = b.permissionStore.DeleteFolderParents(ctx, meta.GetNamespace(), meta.GetName()) - if err != nil { - log.Warn("failed to propagate folder to zanzana", "err", err) + if b.features.IsEnabledGlobally(featuremgmt.FlagZanzana) { + log.Info("Propagating deleted folder to Zanzana", "folder", meta.GetName(), "parent", meta.GetFolder()) + err = b.permissionStore.DeleteFolderParents(ctx, meta.GetNamespace(), meta.GetName()) + if err != nil { + log.Warn("failed to propagate folder to zanzana", "err", err) + } + } + + if b.resourcePermissionsSvc != nil { + log.Debug("deleting folder permissions", "uid", meta.GetName(), "namespace", meta.GetNamespace()) + client := (*b.resourcePermissionsSvc).Namespace(meta.GetNamespace()) + err := client.Delete(ctx, fmt.Sprintf("%s-%s-%s", folders.FolderResourceInfo.GroupVersionResource().Group, folders.FolderResourceInfo.GroupVersionResource().Resource, meta.GetName()), metav1.DeleteOptions{}) + if err != nil { + log.Error("failed to delete folder permissions", "error", err) + } + return + } + + // TODO: once the feature flag kubernetesAuthzResourcePermissionApis is removed, we should initialize resourcePermissionsSvc + // in the RegisterAPIService function and the below should be removed + if !util.IsInterfaceNil(b.folderPermissionsSvc) { + ns, err := claims.ParseNamespace(meta.GetNamespace()) + if err != nil { + log.Error("failed to parse namespace", "error", err) + return + } + if accessErr := b.folderPermissionsSvc.DeleteResourcePermissions(ctx, ns.OrgID, meta.GetName()); accessErr != nil { + log.Warn("failed to delete folder permission after successfully deleting folder resource", "folder", meta.GetName(), "error", accessErr) + } } } diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go index 12ac60b46de..4f818f40ea8 100644 --- a/pkg/registry/apis/folders/register.go +++ b/pkg/registry/apis/folders/register.go @@ -7,6 +7,7 @@ import ( "github.com/prometheus/client_golang/prometheus" 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/runtime/schema" "k8s.io/apiserver/pkg/admission" @@ -14,6 +15,7 @@ import ( genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" + "k8s.io/client-go/dynamic" "k8s.io/kube-openapi/pkg/common" "k8s.io/kube-openapi/pkg/spec3" @@ -22,8 +24,10 @@ import ( folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" "github.com/grafana/grafana/apps/iam/pkg/reconcilers" + "github.com/grafana/grafana/pkg/apimachinery/utils" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" + "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" "github.com/grafana/grafana/pkg/services/accesscontrol" grafanaauthorizer "github.com/grafana/grafana/pkg/services/apiserver/auth/authorizer" "github.com/grafana/grafana/pkg/services/apiserver/builder" @@ -54,10 +58,11 @@ type FolderAPIBuilder struct { permissionsOnCreate bool // Legacy services -- these will not exist in the MT environment - folderSvc folder.LegacyService - folderPermissionsSvc accesscontrol.FolderPermissionsService - acService accesscontrol.Service - ac accesscontrol.AccessControl + folderSvc folder.LegacyService + resourcePermissionsSvc *dynamic.NamespaceableResourceInterface + folderPermissionsSvc accesscontrol.FolderPermissionsService // TODO: Remove this once kubernetesAuthzResourcePermissionApis is removed and the frontend is calling /apis directly to create root level folders + acService accesscontrol.Service + ac accesscontrol.AccessControl } func RegisterAPIService(cfg *setting.Cfg, @@ -88,12 +93,13 @@ func RegisterAPIService(cfg *setting.Cfg, return builder } -func NewAPIService(ac authlib.AccessClient, searcher resource.ResourceClient, features featuremgmt.FeatureToggles, zanzanaClient zanzana.Client) *FolderAPIBuilder { +func NewAPIService(ac authlib.AccessClient, searcher resource.ResourceClient, features featuremgmt.FeatureToggles, zanzanaClient zanzana.Client, resourcePermissionsSvc *dynamic.NamespaceableResourceInterface) *FolderAPIBuilder { return &FolderAPIBuilder{ - features: features, - accessClient: ac, - searcher: searcher, - permissionStore: reconcilers.NewZanzanaPermissionStore(zanzanaClient), + features: features, + accessClient: ac, + searcher: searcher, + permissionStore: reconcilers.NewZanzanaPermissionStore(zanzanaClient), + resourcePermissionsSvc: resourcePermissionsSvc, } } @@ -138,7 +144,9 @@ func (b *FolderAPIBuilder) AllowedV0Alpha1Resources() []string { func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupInfo, opts builder.APIGroupOptions) error { opts.StorageOptsRegister(resourceInfo.GroupResource(), apistore.StorageOptions{ EnableFolderSupport: true, - RequireDeprecatedInternalID: true}) + RequireDeprecatedInternalID: true, + Permissions: b.setDefaultFolderPermissions, + }) unified, err := grafanaregistry.NewRegistryStore(opts.Scheme, resourceInfo, opts.OptsGetter) if err != nil { @@ -193,17 +201,101 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API return nil } +var defaultPermissions = []map[string]any{ + { + "kind": "BasicRole", + "name": "Admin", + "verb": "admin", + }, + { + "kind": "BasicRole", + "name": "Editor", + "verb": "edit", + }, + { + "kind": "BasicRole", + "name": "Viewer", + "verb": "view", + }, +} + +func (b *FolderAPIBuilder) setDefaultFolderPermissions(ctx context.Context, key *resourcepb.ResourceKey, id authlib.AuthInfo, obj utils.GrafanaMetaAccessor) error { + if b.resourcePermissionsSvc == nil { + return nil + } + + // only set default permissions for root folders + if obj.GetFolder() != "" { + return nil + } + + log := logging.FromContext(ctx) + log.Debug("setting default folder permissions", "uid", obj.GetName(), "namespace", obj.GetNamespace()) + + client := (*b.resourcePermissionsSvc).Namespace(obj.GetNamespace()) + name := fmt.Sprintf("%s-%s-%s", folders.FolderResourceInfo.GroupVersionResource().Group, folders.FolderResourceInfo.GroupVersionResource().Resource, obj.GetName()) + + // the resource permission will likely already exist with admin can admin, so we will need to update it + if _, err := client.Get(ctx, name, metav1.GetOptions{}); err == nil { + _, err := client.Update(ctx, &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]any{ + "name": name, + "namespace": obj.GetNamespace(), + }, + "spec": map[string]any{ + "resource": map[string]any{ + "apiGroup": folders.FolderResourceInfo.GroupVersionResource().Group, + "resource": folders.FolderResourceInfo.GroupVersionResource().Resource, + "name": obj.GetName(), + }, + "permissions": defaultPermissions, + }, + }, + }, metav1.UpdateOptions{}) + if err != nil { + logger.Error("failed to update root permissions", "error", err) + return fmt.Errorf("update root permissions: %w", err) + } + + return nil + } + + _, err := client.Create(ctx, &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]any{ + "name": name, + "namespace": obj.GetNamespace(), + }, + "spec": map[string]any{ + "resource": map[string]any{ + "apiGroup": folders.FolderResourceInfo.GroupVersionResource().Group, + "resource": folders.FolderResourceInfo.GroupVersionResource().Resource, + "name": obj.GetName(), + }, + "permissions": defaultPermissions, + }, + }, + }, metav1.CreateOptions{}) + if err != nil { + logger.Error("failed to create root permissions", "error", err) + return fmt.Errorf("create root permissions: %w", err) + } + + return nil +} + func (b *FolderAPIBuilder) registerPermissionHooks(store *genericregistry.Store) { log := logging.FromContext(context.Background()) - if b.features.IsEnabledGlobally(featuremgmt.FlagZanzana) { log.Info("Enabling Zanzana folder propagation hooks") store.BeginCreate = b.beginCreate store.BeginUpdate = b.beginUpdate - store.AfterDelete = b.afterDelete } else { log.Info("Zanzana is not enabled; skipping folder propagation hooks") } + + store.AfterDelete = b.afterDelete } func (b *FolderAPIBuilder) GetOpenAPIDefinitions() common.GetOpenAPIDefinitions { diff --git a/pkg/registry/apis/provisioning/resources/folders.go b/pkg/registry/apis/provisioning/resources/folders.go index d86c0ba9f5e..ba6388b31cf 100644 --- a/pkg/registry/apis/provisioning/resources/folders.go +++ b/pkg/registry/apis/provisioning/resources/folders.go @@ -132,6 +132,8 @@ func (fm *FolderManager) EnsureFolderExists(ctx context.Context, folder Folder, if parent != "" { meta.SetFolder(parent) + } else { + meta.SetAnnotation(utils.AnnoKeyGrantPermissions, utils.AnnoGrantPermissionsDefault) } meta.SetManagerProperties(utils.ManagerProperties{ Kind: utils.ManagerKindRepo, diff --git a/pkg/storage/unified/apistore/permissions.go b/pkg/storage/unified/apistore/permissions.go index d9850db2511..95d2099cb86 100644 --- a/pkg/storage/unified/apistore/permissions.go +++ b/pkg/storage/unified/apistore/permissions.go @@ -34,17 +34,14 @@ func afterCreatePermissionCreator(ctx context.Context, if err != nil { return nil, err } - if val.GetAnnotation(utils.AnnoKeyManagerKind) != "" { - return nil, fmt.Errorf("managed resource may not grant permissions") - } auth, ok := authtypes.AuthInfoFrom(ctx) if !ok { return nil, errors.New("missing auth info") } idtype := auth.GetIdentityType() - if idtype != authtypes.TypeUser && idtype != authtypes.TypeServiceAccount { - return nil, fmt.Errorf("only users or service accounts may grant themselves permissions using an annotation") + if idtype != authtypes.TypeUser && idtype != authtypes.TypeServiceAccount && idtype != authtypes.TypeAccessPolicy { + return nil, fmt.Errorf("only users, service accounts, and access policies may grant permissions using an annotation") } return func(ctx context.Context) error { diff --git a/pkg/storage/unified/apistore/permissions_test.go b/pkg/storage/unified/apistore/permissions_test.go index 57e1409cc24..ff42a4cd189 100644 --- a/pkg/storage/unified/apistore/permissions_test.go +++ b/pkg/storage/unified/apistore/permissions_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" authtypes "github.com/grafana/authlib/types" @@ -40,20 +39,6 @@ func TestAfterCreatePermissionCreator(t *testing.T) { require.Contains(t, err.Error(), "missing default permission creator") }) - t.Run("should error for managed resources", func(t *testing.T) { - obj := &v0alpha1.Dashboard{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - utils.AnnoKeyManagerKind: "test", - }, - }, - } - creator, err := afterCreatePermissionCreator(context.Background(), nil, utils.AnnoGrantPermissionsDefault, obj, mockSetter) - require.Error(t, err) - require.Nil(t, creator) - require.Contains(t, err.Error(), "managed resource may not grant permissions") - }) - t.Run("should error when auth info is missing", func(t *testing.T) { obj := &v0alpha1.Dashboard{} creator, err := afterCreatePermissionCreator(context.Background(), nil, utils.AnnoGrantPermissionsDefault, obj, mockSetter) @@ -108,6 +93,29 @@ func TestAfterCreatePermissionCreator(t *testing.T) { require.NoError(t, err) }) + t.Run("should succeed for access policy identity", func(t *testing.T) { + ctx := identity.WithRequester(context.Background(), &identity.StaticRequester{ + Type: authtypes.TypeAccessPolicy, + OrgID: 1, + OrgRole: "Admin", + UserID: 1, + }) + obj := &v0alpha1.Dashboard{} + key := &resourcepb.ResourceKey{ + Group: "test", + Resource: "test", + Namespace: "test", + Name: "test", + } + + creator, err := afterCreatePermissionCreator(ctx, key, utils.AnnoGrantPermissionsDefault, obj, mockSetter) + require.NoError(t, err) + require.NotNil(t, creator) + + err = creator(ctx) + require.NoError(t, err) + }) + t.Run("should error for non-user/non-service-account identity", func(t *testing.T) { ctx := identity.WithRequester(context.Background(), &identity.StaticRequester{ Type: authtypes.TypeAnonymous, @@ -117,6 +125,6 @@ func TestAfterCreatePermissionCreator(t *testing.T) { creator, err := afterCreatePermissionCreator(ctx, nil, utils.AnnoGrantPermissionsDefault, obj, mockSetter) require.Error(t, err) require.Nil(t, creator) - require.Contains(t, err.Error(), "only users or service accounts may grant themselves permissions") + require.Contains(t, err.Error(), "only users, service accounts, and access policies may grant permissions") }) }