K8s/ManagedBy: Enforce who can CRUD provisioning resources (#103322)

2025-04-08 14:17:33 +03:00
parent 577ea8f6a9
commit d3e6e308a0
8 changed files with 290 additions and 8 deletions
@@ -9,6 +9,7 @@ import (

 	"github.com/grafana/grafana-app-sdk/logging"
 	"github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
+	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
 	"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository"
 	"github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath"
@@ -80,6 +81,11 @@ func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, me

 	// Delete the file in the grafana database
 	if ref == "" {
+		ctx, _, err := identity.WithProvisioningIdentity(ctx, parsed.Obj.GetNamespace())
+		if err != nil {
+			return parsed, err
+		}
+
 		// FIXME: empty folders with no repository files will remain in the system
 		// until the next reconciliation.
 		err = parsed.Client.Delete(ctx, parsed.Obj.GetName(), metav1.DeleteOptions{})
@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana-app-sdk/logging"
 	dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
 	"github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
+	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
 	"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository"
@@ -240,7 +241,12 @@ func (f *ParsedResource) Run(ctx context.Context) error {
 		return fmt.Errorf("unable to find client")
 	}

-	var err error
+	// Always use the provisioning identity when writing
+	ctx, _, err := identity.WithProvisioningIdentity(ctx, f.Obj.GetNamespace())
+	if err != nil {
+		return err
+	}
+
 	// FIXME: shouldn't we check for the specific error?
 	// Run update or create
 	f.Existing, _ = f.Client.Get(ctx, f.Obj.GetName(), metav1.GetOptions{})
@@ -0,0 +1,88 @@
+package apistore
+
+import (
+	"net/http"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	authtypes "github.com/grafana/authlib/types"
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
+)
+
+func checkManagerPropertiesOnDelete(auth authtypes.AuthInfo, obj utils.GrafanaMetaAccessor) error {
+	return enforceManagerProperties(auth, obj)
+}
+
+func checkManagerPropertiesOnCreate(auth authtypes.AuthInfo, obj utils.GrafanaMetaAccessor) error {
+	return enforceManagerProperties(auth, obj)
+}
+
+func checkManagerPropertiesOnUpdateSpec(auth authtypes.AuthInfo, obj utils.GrafanaMetaAccessor, old utils.GrafanaMetaAccessor) error {
+	objKind := obj.GetAnnotation(utils.AnnoKeyManagerKind)
+	oldKind := old.GetAnnotation(utils.AnnoKeyManagerKind)
+	if objKind == "" && objKind == oldKind {
+		return nil // not managed
+	}
+
+	// Check the current settings
+	err := checkManagerPropertiesOnCreate(auth, obj)
+	if err != nil { // new settings failed
+		return err
+	}
+
+	managerNew, okNew := obj.GetManagerProperties()
+	managerOld, okOld := old.GetManagerProperties()
+	if managerNew == managerOld || (okNew && !okOld) { // added manager is OK
+		return nil
+	}
+
+	if !okNew && okOld {
+		// This allows removing the managedBy annotations if you were allowed to write them originally
+		if err := checkManagerPropertiesOnCreate(auth, old); err != nil {
+			return &apierrors.StatusError{ErrStatus: metav1.Status{
+				Status:  metav1.StatusFailure,
+				Code:    http.StatusForbidden,
+				Reason:  metav1.StatusReasonForbidden,
+				Message: "Can not remove resource manager from resource",
+			}}
+		}
+	}
+	return nil
+}
+
+func enforceManagerProperties(auth authtypes.AuthInfo, obj utils.GrafanaMetaAccessor) error {
+	kind := obj.GetAnnotation(utils.AnnoKeyManagerKind)
+	if kind == "" {
+		return nil
+	}
+
+	switch utils.ParseManagerKindString(kind) {
+	case utils.ManagerKindUnknown:
+		return nil // not managed
+
+	case utils.ManagerKindRepo:
+		if auth.GetUID() == "access-policy:provisioning" {
+			return nil // OK!
+		}
+		return &apierrors.StatusError{ErrStatus: metav1.Status{
+			Status:  metav1.StatusFailure,
+			Code:    http.StatusForbidden,
+			Reason:  metav1.StatusReasonForbidden,
+			Message: "Provisioned resources must be manaaged by the provisioning service account",
+		}}
+
+	case utils.ManagerKindPlugin, utils.ManagerKindClassicFP: // nolint:staticcheck
+		// ?? what identity do we use for legacy internal requests?
+		return nil // no error
+
+	case utils.ManagerKindTerraform, utils.ManagerKindKubectl:
+		manager, _ := obj.GetManagerProperties()
+		if manager.AllowsEdits {
+			return nil // no error anyone can do it
+		}
+		// TODO: check the kubectl+terraform resource
+		return nil
+	}
+	return nil
+}
@@ -0,0 +1,136 @@
+package apistore
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	authtypes "github.com/grafana/authlib/types"
+	dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1alpha1"
+	"github.com/grafana/grafana/pkg/apimachinery/identity"
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
+)
+
+func TestManagedAuthorizer(t *testing.T) {
+	user := &identity.StaticRequester{Type: authtypes.TypeUser, UserUID: "uuu"}
+	_, provisioner, err := identity.WithProvisioningIdentity(context.Background(), "default")
+	require.NoError(t, err)
+
+	tests := []struct {
+		name string
+		auth authtypes.AuthInfo
+		obj  runtime.Object
+		old  runtime.Object
+		err  string
+	}{
+		{
+			name: "user can create",
+			auth: user,
+			obj:  &unstructured.Unstructured{},
+		},
+		{
+			name: "provisioning can create",
+			auth: provisioner,
+			obj: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.AnnoKeyManagerKind:     string(utils.ManagerKindRepo),
+						utils.AnnoKeyManagerIdentity: "abc",
+					},
+				},
+			},
+		},
+		{
+			name: "user can not create provisioned resource",
+			auth: user,
+			err:  "Provisioned resources must be manaaged by the provisioning service account",
+			obj: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.AnnoKeyManagerKind:     string(utils.ManagerKindRepo),
+						utils.AnnoKeyManagerIdentity: "abc",
+					},
+				},
+			},
+		},
+		{
+			name: "user can not update provisioned resource",
+			auth: user,
+			err:  "Provisioned resources must be manaaged by the provisioning service account",
+			obj: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Generation: 1,
+				},
+			},
+			old: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Generation: 2,
+					Annotations: map[string]string{
+						utils.AnnoKeyManagerKind:     string(utils.ManagerKindRepo),
+						utils.AnnoKeyManagerIdentity: "abc",
+					},
+				},
+			},
+		},
+		{
+			name: "provisioner can remove manager flags",
+			auth: provisioner,
+			obj: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Generation: 1,
+				},
+			},
+			old: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Generation: 2,
+					Annotations: map[string]string{
+						utils.AnnoKeyManagerKind:     string(utils.ManagerKindRepo),
+						utils.AnnoKeyManagerIdentity: "abc",
+					},
+				},
+			},
+		},
+		{
+			name: "provisioner can add manager flags",
+			auth: provisioner,
+			old: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Generation: 1,
+				},
+			},
+			obj: &dashboard.Dashboard{
+				ObjectMeta: v1.ObjectMeta{
+					Generation: 2,
+					Annotations: map[string]string{
+						utils.AnnoKeyManagerKind:     string(utils.ManagerKindRepo),
+						utils.AnnoKeyManagerIdentity: "abc",
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			obj, err := utils.MetaAccessor(tt.obj)
+			require.NoError(t, err)
+
+			if tt.old == nil {
+				err = checkManagerPropertiesOnCreate(tt.auth, obj)
+			} else {
+				old, _ := utils.MetaAccessor(tt.old)
+				err = checkManagerPropertiesOnUpdateSpec(tt.auth, obj, old)
+			}
+
+			if tt.err != "" {
+				require.Error(t, err, tt.err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
@@ -61,6 +61,9 @@ func (s *Storage) prepareObjectForStorage(ctx context.Context, newObject runtime
 	if obj.GetFolder() != "" && !s.opts.EnableFolderSupport {
 		return nil, apierrors.NewBadRequest(fmt.Sprintf("folders are not supported for: %s", s.gr.String()))
 	}
+	if err := checkManagerPropertiesOnCreate(info, obj); err != nil {
+		return nil, err
+	}

 	if s.opts.RequireDeprecatedInternalID {
 		// nolint:staticcheck
@@ -160,6 +163,11 @@ func (s *Storage) prepareObjectForUpdate(ctx context.Context, updateObject runti
 		obj.SetGeneration(previous.GetGeneration() + 1)
 		obj.SetUpdatedBy(info.GetUID())
 		obj.SetUpdatedTimestampMillis(time.Now().UnixMilli())
+
+		// Only validate when the generation has changed
+		if err := checkManagerPropertiesOnUpdateSpec(info, obj, previous); err != nil {
+			return nil, err
+		}
 	} else {
 		obj.SetGeneration(previous.GetGeneration())
 		obj.SetAnnotation(utils.AnnoKeyUpdatedBy, previous.GetAnnotation(utils.AnnoKeyUpdatedBy))
@@ -87,6 +87,9 @@ func TestPrepareObjectForStorage(t *testing.T) {
 	})

 	t.Run("Should keep manager info", func(t *testing.T) {
+		ctx, _, err := identity.WithProvisioningIdentity(ctx, "default")
+		require.NoError(t, err)
+
 		dashboard := v1alpha1.Dashboard{}
 		dashboard.Name = "test-name"
 		obj := dashboard.DeepCopyObject()
@@ -31,6 +31,7 @@ import (

 	"github.com/bwmarrin/snowflake"

+	authtypes "github.com/grafana/authlib/types"
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
 	"github.com/grafana/grafana/pkg/apiserver/rest"
@@ -219,6 +220,11 @@ func (s *Storage) Delete(
 	_ runtime.Object,
 	opts storage.DeleteOptions,
 ) error {
+	info, ok := authtypes.AuthInfoFrom(ctx)
+	if !ok {
+		return errors.New("missing auth info")
+	}
+
 	if err := s.Get(ctx, key, storage.GetOptions{}, out); err != nil {
 		return err
 	}
@@ -250,6 +256,15 @@ func (s *Storage) Delete(
 			return err
 		}
 	}
+
+	meta, err := utils.MetaAccessor(out)
+	if err != nil {
+		return fmt.Errorf("unable to read object %w", err)
+	}
+	if err = checkManagerPropertiesOnDelete(info, meta); err != nil {
+		return err
+	}
+
 	rsp, err := s.store.Delete(ctx, cmd)
 	if err != nil {
 		return resource.GetError(resource.AsErrorResult(err))
@@ -14,6 +14,7 @@ import (
 	ghmock "github.com/migueleliasweb/go-github-mock/src/mock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

@@ -325,7 +326,7 @@ func TestIntegrationProvisioning_ImportAllPanelsFromLocalRepository(t *testing.T
 	// Set up the repository and the file to import.
 	helper.CopyToProvisioningPath(t, "testdata/all-panels.json", "all-panels.json")

-	localTmp := helper.RenderObject(t, "testdata/local-readonly.json.tmpl", map[string]any{
+	localTmp := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{
 		"Name":        repo,
 		"SyncEnabled": true,
 	})
@@ -349,18 +350,37 @@ func TestIntegrationProvisioning_ImportAllPanelsFromLocalRepository(t *testing.T
 	const allPanels = "n1jR8vnnz"
 	_, err = helper.Dashboards.Resource.Get(ctx, allPanels, metav1.GetOptions{})
 	require.Error(t, err, "no all-panels dashboard should exist")
+	require.True(t, apierrors.IsNotFound(err))

 	// Now, we import it, such that it may exist
 	helper.SyncAndWait(t, repo, nil)

-	found, err := helper.Dashboards.Resource.List(ctx, metav1.ListOptions{})
+	_, err = helper.Dashboards.Resource.List(ctx, metav1.ListOptions{})
 	require.NoError(t, err, "can list values")

-	names := []string{}
-	for _, v := range found.Items {
-		names = append(names, v.GetName())
-	}
-	require.Contains(t, names, allPanels, "all-panels dashboard should now exist")
+	obj, err = helper.Dashboards.Resource.Get(ctx, allPanels, metav1.GetOptions{})
+	require.NoError(t, err, "all-panels dashboard should exist")
+	require.Equal(t, repo, obj.GetAnnotations()[utils.AnnoKeyManagerIdentity])
+
+	// Try writing the value directly
+	err = unstructured.SetNestedField(obj.Object, []any{"aaa", "bbb"}, "spec", "tags")
+	require.NoError(t, err, "set tags")
+	_, err = helper.Dashboards.Resource.Update(ctx, obj, metav1.UpdateOptions{})
+	require.Error(t, err, "only the provisionding service should be able to update")
+	require.True(t, apierrors.IsForbidden(err))
+
+	// Should not be able to directly delete the managed resource
+	err = helper.Dashboards.Resource.Delete(ctx, allPanels, metav1.DeleteOptions{})
+	require.Error(t, err, "only the provisioning service should be able to delete")
+	require.True(t, apierrors.IsForbidden(err))
+
+	// But we can delete the repository file, and this should also remove the resource
+	err = helper.Repositories.Resource.Delete(ctx, repo, metav1.DeleteOptions{}, "files", "all-panels.json")
+	require.NoError(t, err, "should delete the resource file")
+
+	_, err = helper.Dashboards.Resource.Get(ctx, allPanels, metav1.GetOptions{})
+	require.Error(t, err, "should delete the internal resource")
+	require.True(t, apierrors.IsNotFound(err))
 }

 func TestProvisioning_ExportUnifiedToRepository(t *testing.T) {