K8s/Permissions: Enable a grant-permissions annotation action to set default permissions (#102527)

* create permissions

* add key

* lint

* structure as a delayed callback

* legacy API hook

* merge main

* wired up

* and folders

* watch repos

* missing return statement

* Set the correct permissions

* add TestAfterCreatePermissionCreator

* do not add perms on folder create

* fix tests

* add annotation on create

* lint

* lint

* ensure we set permissions when the FT is disabled

* remove custom folder_storage

* fix lint

* change default

* lint

* lint

* fix: annotation

* ensure permissions are added on folder legacy

* remove folderstorage again

* fix tests

* add FT

* undo change to folder

* dashboard on create

* remove annotation for folder

* fix tests

* fix prepare after rebase

* fix tests

* fix tests

* fix tests

* lint

* address comments

* add test for prepareObjectForStorage

* add again skipIfMode as per comment

---------

Co-authored-by: Georges Chaudy <chaudyg@gmail.com>
This commit is contained in:
Ryan McKinley
2025-04-09 13:05:37 +02:00
committed by GitHub
co-authored by Georges Chaudy
parent ceed824378
commit af8a70bbab
18 changed files with 466 additions and 83 deletions
@@ -0,0 +1,52 @@
package apistore
import (
"context"
"errors"
"fmt"
"k8s.io/apimachinery/pkg/runtime"
authtypes "github.com/grafana/authlib/types"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/storage/unified/resource"
)
type permissionCreatorFunc = func(ctx context.Context) error
func afterCreatePermissionCreator(ctx context.Context,
key *resource.ResourceKey,
grantPermisions string,
obj runtime.Object,
setter DefaultPermissionSetter,
) (permissionCreatorFunc, error) {
if grantPermisions == "" {
return nil, nil
}
if grantPermisions != utils.AnnoGrantPermissionsDefault {
return nil, fmt.Errorf("invalid permissions value. only '%s' supported", utils.AnnoGrantPermissionsDefault)
}
if setter == nil {
return nil, fmt.Errorf("missing default permission creator")
}
val, err := utils.MetaAccessor(obj)
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")
}
return func(ctx context.Context) error {
return setter(ctx, key, auth, val)
}, nil
}
@@ -0,0 +1,120 @@
package apistore
import (
"context"
"testing"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
authtypes "github.com/grafana/authlib/types"
"github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/storage/unified/resource"
)
func TestAfterCreatePermissionCreator(t *testing.T) {
mockSetter := func(ctx context.Context, key *resource.ResourceKey, auth authtypes.AuthInfo, val utils.GrafanaMetaAccessor) error {
return nil
}
t.Run("should return nil when grantPermissions is empty", func(t *testing.T) {
creator, err := afterCreatePermissionCreator(context.Background(), nil, "", nil, mockSetter)
require.NoError(t, err)
require.Nil(t, creator)
})
t.Run("should error with invalid grantPermissions value", func(t *testing.T) {
creator, err := afterCreatePermissionCreator(context.Background(), nil, "invalid", nil, mockSetter)
require.Error(t, err)
require.Nil(t, creator)
require.Contains(t, err.Error(), "invalid permissions value")
})
t.Run("should error when setter is nil", func(t *testing.T) {
creator, err := afterCreatePermissionCreator(context.Background(), nil, utils.AnnoGrantPermissionsDefault, nil, nil)
require.Error(t, err)
require.Nil(t, creator)
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)
require.Error(t, err)
require.Nil(t, creator)
require.Contains(t, err.Error(), "missing auth info")
})
t.Run("should succeed for user identity", func(t *testing.T) {
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{
OrgID: 1,
OrgRole: "Admin",
UserID: 1,
})
obj := &v0alpha1.Dashboard{}
key := &resource.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 succeed for service account identity", func(t *testing.T) {
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{
OrgID: 1,
OrgRole: "Admin",
UserID: 1,
})
obj := &v0alpha1.Dashboard{}
key := &resource.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,
})
obj := &v0alpha1.Dashboard{}
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")
})
}
+18 -10
View File
@@ -39,30 +39,35 @@ func formatBytes(numBytes int) string {
}
// Called on create
func (s *Storage) prepareObjectForStorage(ctx context.Context, newObject runtime.Object) ([]byte, error) {
func (s *Storage) prepareObjectForStorage(ctx context.Context, newObject runtime.Object) ([]byte, string, error) {
info, ok := authtypes.AuthInfoFrom(ctx)
if !ok {
return nil, errors.New("missing auth info")
return nil, "", errors.New("missing auth info")
}
obj, err := utils.MetaAccessor(newObject)
if err != nil {
return nil, err
return nil, "", err
}
if obj.GetName() == "" {
return nil, storage.NewInvalidObjError("", "missing name")
return nil, "", storage.NewInvalidObjError("", "missing name")
}
if obj.GetResourceVersion() != "" {
return nil, storage.ErrResourceVersionSetOnCreate
return nil, "", storage.ErrResourceVersionSetOnCreate
}
if obj.GetUID() == "" {
obj.SetUID(types.UID(uuid.NewString()))
}
if obj.GetFolder() != "" && !s.opts.EnableFolderSupport {
return nil, apierrors.NewBadRequest(fmt.Sprintf("folders are not supported for: %s", s.gr.String()))
return nil, "", apierrors.NewBadRequest(fmt.Sprintf("folders are not supported for: %s", s.gr.String()))
}
grantPermisions := obj.GetAnnotation(utils.AnnoKeyGrantPermissions)
if grantPermisions != "" {
obj.SetAnnotation(utils.AnnoKeyGrantPermissions, "") // remove the annotation
}
if err := checkManagerPropertiesOnCreate(info, obj); err != nil {
return nil, err
return nil, "", err
}
if s.opts.RequireDeprecatedInternalID {
@@ -88,9 +93,11 @@ func (s *Storage) prepareObjectForStorage(ctx context.Context, newObject runtime
var buf bytes.Buffer
if err = s.codec.Encode(newObject, &buf); err != nil {
return nil, err
return nil, "", err
}
return s.handleLargeResources(ctx, obj, buf)
val, err := s.handleLargeResources(ctx, obj, buf)
return val, grantPermisions, err
}
// Called on update
@@ -130,7 +137,8 @@ func (s *Storage) prepareObjectForUpdate(ctx context.Context, updateObject runti
obj.SetCreatedBy(previous.GetCreatedBy())
obj.SetCreationTimestamp(previous.GetCreationTimestamp())
obj.SetResourceVersion("") // removed from saved JSON because the RV is not yet calculated
obj.SetResourceVersion("") // removed from saved JSON because the RV is not yet calculated
obj.SetAnnotation(utils.AnnoKeyGrantPermissions, "") // Grant is ignored for update requests
// for dashboards, a mutation hook will set it if it didn't exist on the previous obj
// avoid setting it back to 0
+29 -11
View File
@@ -42,14 +42,14 @@ func TestPrepareObjectForStorage(t *testing.T) {
)
t.Run("Error getting auth info from context", func(t *testing.T) {
_, err := s.prepareObjectForStorage(context.Background(), nil)
_, _, err := s.prepareObjectForStorage(context.Background(), nil)
require.Error(t, err)
require.Contains(t, err.Error(), "missing auth info")
})
t.Run("Error on missing name", func(t *testing.T) {
dashboard := v1alpha1.Dashboard{}
_, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
_, _, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.Error(t, err)
require.Contains(t, err.Error(), "missing name")
})
@@ -58,7 +58,7 @@ func TestPrepareObjectForStorage(t *testing.T) {
dashboard := v1alpha1.Dashboard{}
dashboard.Name = "test-name"
dashboard.ResourceVersion = "123"
_, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
_, _, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.Error(t, err)
require.Equal(t, storage.ErrResourceVersionSetOnCreate, err)
})
@@ -67,7 +67,7 @@ func TestPrepareObjectForStorage(t *testing.T) {
dashboard := v1alpha1.Dashboard{}
dashboard.Name = "test-name"
encodedData, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
encodedData, _, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v1alpha1.Dashboard{})
@@ -106,7 +106,7 @@ func TestPrepareObjectForStorage(t *testing.T) {
TimestampMillis: now.UnixMilli(),
})
encodedData, err := s.prepareObjectForStorage(ctx, obj)
encodedData, _, err := s.prepareObjectForStorage(ctx, obj)
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v1alpha1.Dashboard{})
@@ -133,7 +133,7 @@ func TestPrepareObjectForStorage(t *testing.T) {
meta.SetFolder("aaa")
require.NoError(t, err)
encodedData, err := s.prepareObjectForStorage(ctx, obj)
encodedData, _, err := s.prepareObjectForStorage(ctx, obj)
require.NoError(t, err)
insertedObject, _, err := s.codec.Decode(encodedData, nil, &v1alpha1.Dashboard{})
@@ -189,7 +189,7 @@ func TestPrepareObjectForStorage(t *testing.T) {
dashboard := v1alpha1.Dashboard{}
dashboard.Name = "test-name"
encodedData, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
encodedData, _, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v1alpha1.Dashboard{})
require.NoError(t, err)
@@ -208,7 +208,7 @@ func TestPrepareObjectForStorage(t *testing.T) {
require.NoError(t, err)
meta.SetDeprecatedInternalID(1) // nolint:staticcheck
encodedData, err := s.prepareObjectForStorage(ctx, obj)
encodedData, _, err := s.prepareObjectForStorage(ctx, obj)
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v1alpha1.Dashboard{})
require.NoError(t, err)
@@ -217,6 +217,24 @@ func TestPrepareObjectForStorage(t *testing.T) {
require.Equal(t, meta.GetDeprecatedInternalID(), int64(1)) // nolint:staticcheck
})
t.Run("Should remove grant permissions annotation", func(t *testing.T) {
dashboard := v1alpha1.Dashboard{}
dashboard.Name = "test-name"
obj := dashboard.DeepCopyObject()
meta, err := utils.MetaAccessor(obj)
require.NoError(t, err)
meta.SetAnnotation(utils.AnnoKeyGrantPermissions, "default")
encodedData, p, err := s.prepareObjectForStorage(ctx, obj)
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v1alpha1.Dashboard{})
require.NoError(t, err)
meta, err = utils.MetaAccessor(newObject)
require.NoError(t, err)
require.Empty(t, meta.GetAnnotation(utils.AnnoKeyGrantPermissions))
require.Equal(t, p, "default")
})
t.Run("calculate generation", func(t *testing.T) {
dash := &v1alpha1.Dashboard{
ObjectMeta: v1.ObjectMeta{
@@ -286,7 +304,7 @@ func getPreparedObject(t *testing.T, ctx context.Context, s *Storage, obj runtim
var err error
if old == nil {
raw, err = s.prepareObjectForStorage(ctx, obj)
raw, _, err = s.prepareObjectForStorage(ctx, obj)
} else {
raw, err = s.prepareObjectForUpdate(ctx, obj, old)
}
@@ -323,7 +341,7 @@ func TestPrepareLargeObjectForStorage(t *testing.T) {
},
}
_, err := f.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
_, _, err := f.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.Nil(t, err)
require.True(t, los.deconstructed)
})
@@ -341,7 +359,7 @@ func TestPrepareLargeObjectForStorage(t *testing.T) {
},
}
_, err := f.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
_, _, err := f.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.Nil(t, err)
require.False(t, los.deconstructed)
})
+19 -1
View File
@@ -46,6 +46,8 @@ const (
var _ storage.Interface = (*Storage)(nil)
type DefaultPermissionSetter = func(ctx context.Context, key *resource.ResourceKey, id authtypes.AuthInfo, obj utils.GrafanaMetaAccessor) error
// Optional settings that apply to a single resource
type StorageOptions struct {
// ????: should we constrain this to only dashboards for now?
@@ -57,6 +59,9 @@ type StorageOptions struct {
// Add internalID label when missing
RequireDeprecatedInternalID bool
// Temporary fix to support adding default permissions AfterCreate
Permissions DefaultPermissionSetter
}
// Storage implements storage.Interface and storage resources as JSON files on disk.
@@ -164,15 +169,23 @@ func (s *Storage) convertToObject(data []byte, obj runtime.Object) (runtime.Obje
// set to the read value from database.
func (s *Storage) Create(ctx context.Context, key string, obj runtime.Object, out runtime.Object, ttl uint64) error {
var err error
var permissions string
req := &resource.CreateRequest{}
req.Value, err = s.prepareObjectForStorage(ctx, obj)
req.Value, permissions, err = s.prepareObjectForStorage(ctx, obj)
if err != nil {
return err
}
req.Key, err = s.getKey(key)
if err != nil {
return err
}
grantPermissions, err := afterCreatePermissionCreator(ctx, req.Key, permissions, obj, s.opts.Permissions)
if err != nil {
return err
}
rsp, err := s.store.Create(ctx, req)
if err != nil {
return resource.GetError(resource.AsErrorResult(err))
@@ -203,6 +216,11 @@ func (s *Storage) Create(ctx context.Context, key string, obj runtime.Object, ou
})
}
// Synchronous AfterCreate permissions -- allows users to become "admin" of the thing they made
if grantPermissions != nil {
return grantPermissions(ctx)
}
return nil
}
+4
View File
@@ -376,6 +376,10 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *Resour
}
}
if obj.GetAnnotation(utils.AnnoKeyGrantPermissions) != "" {
return nil, NewBadRequestError("can not save annotation: " + utils.AnnoKeyGrantPermissions)
}
check := claims.CheckRequest{
Verb: utils.VerbCreate,
Group: key.Group,