Skip to content

Commit e083c05

Browse files
Folders: Create default permissions on root level folder in API Service and cleanup after delete (grafana#111690)
1 parent ffe85d7 commit e083c05

File tree

6 files changed

+171
-50
lines changed

6 files changed

+171
-50
lines changed

pkg/registry/apis/folders/folder_storage.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/grafana/grafana/pkg/apimachinery/identity"
1717
"github.com/grafana/grafana/pkg/apimachinery/utils"
1818
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
19-
"github.com/grafana/grafana/pkg/infra/log"
2019
"github.com/grafana/grafana/pkg/services/accesscontrol"
2120
"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
2221
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
@@ -112,6 +111,10 @@ func (s *folderStorage) Create(ctx context.Context,
112111

113112
parentUid := accessor.GetFolder()
114113

114+
// TODO: once the feature flag kubernetesAuthzResourcePermissionApis is removed AND the frontend is calling
115+
// /apis directly (to set AnnoKeyGrantPermissions on root level folders), the below should be removed
116+
// and we should instead initialize resourcePermissionsSvc in the RegisterAPIService function
117+
// and rely on StorageOptions.Permissions.
115118
err = s.setDefaultFolderPermissions(ctx, info.OrgID, user, p.Name, parentUid)
116119
if err != nil {
117120
return nil, err
@@ -133,22 +136,11 @@ func (s *folderStorage) Update(ctx context.Context,
133136

134137
// GracefulDeleter
135138
func (s *folderStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
136-
info, err := request.NamespaceInfoFrom(ctx, true)
137-
if err != nil {
138-
return nil, false, err
139-
}
140-
141139
obj, async, err := s.store.Delete(ctx, name, deleteValidation, options)
142140
if err != nil {
143141
return obj, async, err
144142
}
145143

146-
if accessErr := s.folderPermissionsSvc.DeleteResourcePermissions(ctx, info.OrgID, name); accessErr != nil {
147-
// TODO: add a proper logger to this struct.
148-
logger := log.New().FromContext(ctx)
149-
logger.Warn("failed to delete folder permission after successfully deleting folder resource", "folder", name, "error", accessErr)
150-
}
151-
152144
return obj, async, err
153145
}
154146

@@ -157,7 +149,7 @@ func (s *folderStorage) DeleteCollection(ctx context.Context, deleteValidation r
157149
return nil, fmt.Errorf("DeleteCollection for folders not implemented")
158150
}
159151

160-
func (s *folderStorage) setDefaultFolderPermissions(ctx context.Context, orgID int64, user identity.Requester, uid string, parentUID string) error {
152+
func (s *folderStorage) setDefaultFolderPermissions(ctx context.Context, orgID int64, user identity.Requester, uid, parentUID string) error {
161153
var permissions []accesscontrol.SetResourcePermissionCommand
162154

163155
if user.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) {

pkg/registry/apis/folders/hooks.go

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,18 @@ package folders
22

33
import (
44
"context"
5+
"fmt"
56

67
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
78
"k8s.io/apimachinery/pkg/runtime"
89
"k8s.io/apiserver/pkg/registry/generic/registry"
910

11+
claims "github.com/grafana/authlib/types"
1012
"github.com/grafana/grafana-app-sdk/logging"
13+
folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
1114
"github.com/grafana/grafana/pkg/apimachinery/utils"
15+
"github.com/grafana/grafana/pkg/services/featuremgmt"
16+
"github.com/grafana/grafana/pkg/util"
1217
)
1318

1419
// 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
7782
return
7883
}
7984

80-
log.Info("Propagating deleted folder to Zanzana", "folder", meta.GetName(), "parent", meta.GetFolder())
81-
err = b.permissionStore.DeleteFolderParents(ctx, meta.GetNamespace(), meta.GetName())
82-
if err != nil {
83-
log.Warn("failed to propagate folder to zanzana", "err", err)
85+
if b.features.IsEnabledGlobally(featuremgmt.FlagZanzana) {
86+
log.Info("Propagating deleted folder to Zanzana", "folder", meta.GetName(), "parent", meta.GetFolder())
87+
err = b.permissionStore.DeleteFolderParents(ctx, meta.GetNamespace(), meta.GetName())
88+
if err != nil {
89+
log.Warn("failed to propagate folder to zanzana", "err", err)
90+
}
91+
}
92+
93+
if b.resourcePermissionsSvc != nil {
94+
log.Debug("deleting folder permissions", "uid", meta.GetName(), "namespace", meta.GetNamespace())
95+
client := (*b.resourcePermissionsSvc).Namespace(meta.GetNamespace())
96+
err := client.Delete(ctx, fmt.Sprintf("%s-%s-%s", folders.FolderResourceInfo.GroupVersionResource().Group, folders.FolderResourceInfo.GroupVersionResource().Resource, meta.GetName()), metav1.DeleteOptions{})
97+
if err != nil {
98+
log.Error("failed to delete folder permissions", "error", err)
99+
}
100+
return
101+
}
102+
103+
// TODO: once the feature flag kubernetesAuthzResourcePermissionApis is removed, we should initialize resourcePermissionsSvc
104+
// in the RegisterAPIService function and the below should be removed
105+
if !util.IsInterfaceNil(b.folderPermissionsSvc) {
106+
ns, err := claims.ParseNamespace(meta.GetNamespace())
107+
if err != nil {
108+
log.Error("failed to parse namespace", "error", err)
109+
return
110+
}
111+
if accessErr := b.folderPermissionsSvc.DeleteResourcePermissions(ctx, ns.OrgID, meta.GetName()); accessErr != nil {
112+
log.Warn("failed to delete folder permission after successfully deleting folder resource", "folder", meta.GetName(), "error", accessErr)
113+
}
84114
}
85115
}
86116

pkg/registry/apis/folders/register.go

Lines changed: 104 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@ import (
77

88
"github.com/prometheus/client_golang/prometheus"
99
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1011
"k8s.io/apimachinery/pkg/runtime"
1112
"k8s.io/apimachinery/pkg/runtime/schema"
1213
"k8s.io/apiserver/pkg/admission"
1314
"k8s.io/apiserver/pkg/authorization/authorizer"
1415
genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
1516
"k8s.io/apiserver/pkg/registry/rest"
1617
genericapiserver "k8s.io/apiserver/pkg/server"
18+
"k8s.io/client-go/dynamic"
1719
"k8s.io/kube-openapi/pkg/common"
1820
"k8s.io/kube-openapi/pkg/spec3"
1921

@@ -22,8 +24,10 @@ import (
2224

2325
folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
2426
"github.com/grafana/grafana/apps/iam/pkg/reconcilers"
27+
"github.com/grafana/grafana/pkg/apimachinery/utils"
2528
grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
2629
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
30+
"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
2731
"github.com/grafana/grafana/pkg/services/accesscontrol"
2832
grafanaauthorizer "github.com/grafana/grafana/pkg/services/apiserver/auth/authorizer"
2933
"github.com/grafana/grafana/pkg/services/apiserver/builder"
@@ -54,10 +58,11 @@ type FolderAPIBuilder struct {
5458
permissionsOnCreate bool
5559

5660
// Legacy services -- these will not exist in the MT environment
57-
folderSvc folder.LegacyService
58-
folderPermissionsSvc accesscontrol.FolderPermissionsService
59-
acService accesscontrol.Service
60-
ac accesscontrol.AccessControl
61+
folderSvc folder.LegacyService
62+
resourcePermissionsSvc *dynamic.NamespaceableResourceInterface
63+
folderPermissionsSvc accesscontrol.FolderPermissionsService // TODO: Remove this once kubernetesAuthzResourcePermissionApis is removed and the frontend is calling /apis directly to create root level folders
64+
acService accesscontrol.Service
65+
ac accesscontrol.AccessControl
6166
}
6267

6368
func RegisterAPIService(cfg *setting.Cfg,
@@ -88,12 +93,13 @@ func RegisterAPIService(cfg *setting.Cfg,
8893
return builder
8994
}
9095

91-
func NewAPIService(ac authlib.AccessClient, searcher resource.ResourceClient, features featuremgmt.FeatureToggles, zanzanaClient zanzana.Client) *FolderAPIBuilder {
96+
func NewAPIService(ac authlib.AccessClient, searcher resource.ResourceClient, features featuremgmt.FeatureToggles, zanzanaClient zanzana.Client, resourcePermissionsSvc *dynamic.NamespaceableResourceInterface) *FolderAPIBuilder {
9297
return &FolderAPIBuilder{
93-
features: features,
94-
accessClient: ac,
95-
searcher: searcher,
96-
permissionStore: reconcilers.NewZanzanaPermissionStore(zanzanaClient),
98+
features: features,
99+
accessClient: ac,
100+
searcher: searcher,
101+
permissionStore: reconcilers.NewZanzanaPermissionStore(zanzanaClient),
102+
resourcePermissionsSvc: resourcePermissionsSvc,
97103
}
98104
}
99105

@@ -138,7 +144,9 @@ func (b *FolderAPIBuilder) AllowedV0Alpha1Resources() []string {
138144
func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupInfo, opts builder.APIGroupOptions) error {
139145
opts.StorageOptsRegister(resourceInfo.GroupResource(), apistore.StorageOptions{
140146
EnableFolderSupport: true,
141-
RequireDeprecatedInternalID: true})
147+
RequireDeprecatedInternalID: true,
148+
Permissions: b.setDefaultFolderPermissions,
149+
})
142150

143151
unified, err := grafanaregistry.NewRegistryStore(opts.Scheme, resourceInfo, opts.OptsGetter)
144152
if err != nil {
@@ -193,17 +201,101 @@ func (b *FolderAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.API
193201
return nil
194202
}
195203

204+
var defaultPermissions = []map[string]any{
205+
{
206+
"kind": "BasicRole",
207+
"name": "Admin",
208+
"verb": "admin",
209+
},
210+
{
211+
"kind": "BasicRole",
212+
"name": "Editor",
213+
"verb": "edit",
214+
},
215+
{
216+
"kind": "BasicRole",
217+
"name": "Viewer",
218+
"verb": "view",
219+
},
220+
}
221+
222+
func (b *FolderAPIBuilder) setDefaultFolderPermissions(ctx context.Context, key *resourcepb.ResourceKey, id authlib.AuthInfo, obj utils.GrafanaMetaAccessor) error {
223+
if b.resourcePermissionsSvc == nil {
224+
return nil
225+
}
226+
227+
// only set default permissions for root folders
228+
if obj.GetFolder() != "" {
229+
return nil
230+
}
231+
232+
log := logging.FromContext(ctx)
233+
log.Debug("setting default folder permissions", "uid", obj.GetName(), "namespace", obj.GetNamespace())
234+
235+
client := (*b.resourcePermissionsSvc).Namespace(obj.GetNamespace())
236+
name := fmt.Sprintf("%s-%s-%s", folders.FolderResourceInfo.GroupVersionResource().Group, folders.FolderResourceInfo.GroupVersionResource().Resource, obj.GetName())
237+
238+
// the resource permission will likely already exist with admin can admin, so we will need to update it
239+
if _, err := client.Get(ctx, name, metav1.GetOptions{}); err == nil {
240+
_, err := client.Update(ctx, &unstructured.Unstructured{
241+
Object: map[string]interface{}{
242+
"metadata": map[string]any{
243+
"name": name,
244+
"namespace": obj.GetNamespace(),
245+
},
246+
"spec": map[string]any{
247+
"resource": map[string]any{
248+
"apiGroup": folders.FolderResourceInfo.GroupVersionResource().Group,
249+
"resource": folders.FolderResourceInfo.GroupVersionResource().Resource,
250+
"name": obj.GetName(),
251+
},
252+
"permissions": defaultPermissions,
253+
},
254+
},
255+
}, metav1.UpdateOptions{})
256+
if err != nil {
257+
logger.Error("failed to update root permissions", "error", err)
258+
return fmt.Errorf("update root permissions: %w", err)
259+
}
260+
261+
return nil
262+
}
263+
264+
_, err := client.Create(ctx, &unstructured.Unstructured{
265+
Object: map[string]interface{}{
266+
"metadata": map[string]any{
267+
"name": name,
268+
"namespace": obj.GetNamespace(),
269+
},
270+
"spec": map[string]any{
271+
"resource": map[string]any{
272+
"apiGroup": folders.FolderResourceInfo.GroupVersionResource().Group,
273+
"resource": folders.FolderResourceInfo.GroupVersionResource().Resource,
274+
"name": obj.GetName(),
275+
},
276+
"permissions": defaultPermissions,
277+
},
278+
},
279+
}, metav1.CreateOptions{})
280+
if err != nil {
281+
logger.Error("failed to create root permissions", "error", err)
282+
return fmt.Errorf("create root permissions: %w", err)
283+
}
284+
285+
return nil
286+
}
287+
196288
func (b *FolderAPIBuilder) registerPermissionHooks(store *genericregistry.Store) {
197289
log := logging.FromContext(context.Background())
198-
199290
if b.features.IsEnabledGlobally(featuremgmt.FlagZanzana) {
200291
log.Info("Enabling Zanzana folder propagation hooks")
201292
store.BeginCreate = b.beginCreate
202293
store.BeginUpdate = b.beginUpdate
203-
store.AfterDelete = b.afterDelete
204294
} else {
205295
log.Info("Zanzana is not enabled; skipping folder propagation hooks")
206296
}
297+
298+
store.AfterDelete = b.afterDelete
207299
}
208300

209301
func (b *FolderAPIBuilder) GetOpenAPIDefinitions() common.GetOpenAPIDefinitions {

pkg/registry/apis/provisioning/resources/folders.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@ func (fm *FolderManager) EnsureFolderExists(ctx context.Context, folder Folder,
132132

133133
if parent != "" {
134134
meta.SetFolder(parent)
135+
} else {
136+
meta.SetAnnotation(utils.AnnoKeyGrantPermissions, utils.AnnoGrantPermissionsDefault)
135137
}
136138
meta.SetManagerProperties(utils.ManagerProperties{
137139
Kind: utils.ManagerKindRepo,

pkg/storage/unified/apistore/permissions.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,14 @@ func afterCreatePermissionCreator(ctx context.Context,
3434
if err != nil {
3535
return nil, err
3636
}
37-
if val.GetAnnotation(utils.AnnoKeyManagerKind) != "" {
38-
return nil, fmt.Errorf("managed resource may not grant permissions")
39-
}
4037
auth, ok := authtypes.AuthInfoFrom(ctx)
4138
if !ok {
4239
return nil, errors.New("missing auth info")
4340
}
4441

4542
idtype := auth.GetIdentityType()
46-
if idtype != authtypes.TypeUser && idtype != authtypes.TypeServiceAccount {
47-
return nil, fmt.Errorf("only users or service accounts may grant themselves permissions using an annotation")
43+
if idtype != authtypes.TypeUser && idtype != authtypes.TypeServiceAccount && idtype != authtypes.TypeAccessPolicy {
44+
return nil, fmt.Errorf("only users, service accounts, and access policies may grant permissions using an annotation")
4845
}
4946

5047
return func(ctx context.Context) error {

pkg/storage/unified/apistore/permissions_test.go

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"testing"
66

77
"github.com/stretchr/testify/require"
8-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
98

109
authtypes "github.com/grafana/authlib/types"
1110

@@ -40,20 +39,6 @@ func TestAfterCreatePermissionCreator(t *testing.T) {
4039
require.Contains(t, err.Error(), "missing default permission creator")
4140
})
4241

43-
t.Run("should error for managed resources", func(t *testing.T) {
44-
obj := &v0alpha1.Dashboard{
45-
ObjectMeta: metav1.ObjectMeta{
46-
Annotations: map[string]string{
47-
utils.AnnoKeyManagerKind: "test",
48-
},
49-
},
50-
}
51-
creator, err := afterCreatePermissionCreator(context.Background(), nil, utils.AnnoGrantPermissionsDefault, obj, mockSetter)
52-
require.Error(t, err)
53-
require.Nil(t, creator)
54-
require.Contains(t, err.Error(), "managed resource may not grant permissions")
55-
})
56-
5742
t.Run("should error when auth info is missing", func(t *testing.T) {
5843
obj := &v0alpha1.Dashboard{}
5944
creator, err := afterCreatePermissionCreator(context.Background(), nil, utils.AnnoGrantPermissionsDefault, obj, mockSetter)
@@ -108,6 +93,29 @@ func TestAfterCreatePermissionCreator(t *testing.T) {
10893
require.NoError(t, err)
10994
})
11095

96+
t.Run("should succeed for access policy identity", func(t *testing.T) {
97+
ctx := identity.WithRequester(context.Background(), &identity.StaticRequester{
98+
Type: authtypes.TypeAccessPolicy,
99+
OrgID: 1,
100+
OrgRole: "Admin",
101+
UserID: 1,
102+
})
103+
obj := &v0alpha1.Dashboard{}
104+
key := &resourcepb.ResourceKey{
105+
Group: "test",
106+
Resource: "test",
107+
Namespace: "test",
108+
Name: "test",
109+
}
110+
111+
creator, err := afterCreatePermissionCreator(ctx, key, utils.AnnoGrantPermissionsDefault, obj, mockSetter)
112+
require.NoError(t, err)
113+
require.NotNil(t, creator)
114+
115+
err = creator(ctx)
116+
require.NoError(t, err)
117+
})
118+
111119
t.Run("should error for non-user/non-service-account identity", func(t *testing.T) {
112120
ctx := identity.WithRequester(context.Background(), &identity.StaticRequester{
113121
Type: authtypes.TypeAnonymous,
@@ -117,6 +125,6 @@ func TestAfterCreatePermissionCreator(t *testing.T) {
117125
creator, err := afterCreatePermissionCreator(ctx, nil, utils.AnnoGrantPermissionsDefault, obj, mockSetter)
118126
require.Error(t, err)
119127
require.Nil(t, creator)
120-
require.Contains(t, err.Error(), "only users or service accounts may grant themselves permissions")
128+
require.Contains(t, err.Error(), "only users, service accounts, and access policies may grant permissions")
121129
})
122130
}

0 commit comments

Comments
 (0)