merge main

pkg/registry/apis/folders/validate.go

@@ -10,6 +10,7 @@ import (
 
 	folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
+	utils2 "github.com/grafana/grafana/pkg/registry/apis/preferences/utils" // TODO, will be moved into utils
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/folder"
@@ -23,6 +24,7 @@ func validateOnCreate(ctx context.Context, f *folders.Folder, getter parentsGett
 	if slices.Contains([]string{
 		folder.GeneralFolderUID,
 		folder.SharedWithMeFolderUID,
+		// "namespace" is not valid based on owner parsing
 	}, id) {
 		return dashboards.ErrFolderInvalidUID
 	}
@@ -32,6 +34,29 @@ func validateOnCreate(ctx context.Context, f *folders.Folder, getter parentsGett
 		return fmt.Errorf("unable to read metadata from object: %w", err)
 	}
 
+	owner, ok := utils2.ParseOwnerFromName(id)
+	if ok {
+		if owner.Owner == utils2.NamespaceResourceOwner {
+			return fmt.Errorf("folder may not be a namespace")
+		}
+		if meta.GetFolder() != "" {
+			return fmt.Errorf("%s folder must be a root", owner.Owner)
+		}
+		if meta.GetAnnotation(utils.AnnoKeyGrantPermissions) != "" {
+			return fmt.Errorf("%s folders do not support: %s", owner.Owner, utils.AnnoKeyGrantPermissions)
+		}
+
+		if err = owner.Validate(meta); err != nil {
+			return err
+		}
+
+		// The title will be based on the team/user so we should not save a value in the folder
+		if f.Spec.Title != "" {
+			return fmt.Errorf("folder title must be empty when creating a %s folder", owner.Owner)
+		}
+		return nil
+	}
+
 	if !util.IsValidShortUID(id) {
 		return dashboards.ErrDashboardInvalidUid
 	}
@@ -83,6 +108,19 @@ func validateOnUpdate(ctx context.Context,
 		return err
 	}
 
+	name, ok := utils2.ParseOwnerFromName(obj.Name)
+	if ok {
+		if err = name.Validate(folderObj); err != nil {
+			return err
+		}
+
+		// The title will be based on the team/user so we should not save a value in the folder
+		if obj.Spec.Title != "" {
+			return fmt.Errorf("folder title must be empty")
+		}
+		return nil
+	}
+
 	if obj.Spec.Title == "" {
 		return dashboards.ErrFolderTitleEmpty
 	}

pkg/registry/apis/folders/validate_test.go

@@ -127,7 +127,121 @@ func TestValidateCreate(t *testing.T) {
 			},
 			maxDepth: folder.MaxNestedFolderDepth,
 		},
-	}
+		{
+			name: "team folder",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "team-abc",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "abc", Kind: "Team", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+			},
+		}, {
+			name: "user folder",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "user-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "xyz", Kind: "User", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+			},
+		}, {
+			name: "team without owner reference",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "user-xyz",
+				},
+			},
+			expectedErr: "missing owner reference (user-xyz)",
+		}, {
+			name: "team with owner mismatch",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "user-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "ABC", Kind: "User", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+			},
+			expectedErr: "owner reference must match the same name",
+		}, {
+			name: "team with wrong owner kind",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "user-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "xyz", Kind: "NotUser", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+			},
+			expectedErr: "owner reference kind must match the name",
+		}, {
+			name: "team with wrong owner apiVersion",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "user-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "xyz", Kind: "User", APIVersion: "not-iam.grafana.app"},
+					},
+				},
+			},
+			expectedErr: "owner reference should be iam.grafana.app",
+		}, {
+			name: "team with multiple owners",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "user-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "ABC", Kind: "User", APIVersion: "iam.grafana.app/vAnything"},
+						{Name: "EFG", Kind: "User", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+			},
+			expectedErr: "multiple owner references",
+		}, {
+			name: "team with title set",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "user-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "xyz", Kind: "User", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+				Spec: folders.FolderSpec{
+					Title: "should not set a title",
+				},
+			},
+			expectedErr: "folder title must be empty",
+		}, {
+			name: "team folder must be root",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "user-xyz",
+					Annotations: map[string]string{"grafana.app/folder": "p1"},
+				},
+			},
+			expectedErr: "folder must be a root",
+		}, {
+			name: "team folder must be root",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "user-xyz",
+					Annotations: map[string]string{"grafana.app/folder": "p1"},
+				},
+			},
+			expectedErr: "folder must be a root",
+		}, {
+			name: "team folder with grant permissions",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "team-abc",
+					Annotations: map[string]string{"grafana.app/grant-permissions": "default"},
+				},
+			},
+			expectedErr: "team folders do not support:",
+		}}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -328,8 +442,38 @@ func TestValidateUpdate(t *testing.T) {
 				},
 			},
 			expectedErr: "cannot move folder under its own descendant",
-		},
-	}
+		}, {
+			name: "change team folder title",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "team-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "xyz", Kind: "Team", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+				Spec: folders.FolderSpec{
+					Title: "changed",
+				},
+			},
+			old: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "team-xyz",
+					OwnerReferences: []metav1.OwnerReference{
+						{Name: "xyz", Kind: "Team", APIVersion: "iam.grafana.app/vAnything"},
+					},
+				},
+			},
+			expectedErr: "folder title must be empty",
+		}, {
+			name: "remove owner from team folder",
+			folder: &folders.Folder{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "team-xyz",
+				},
+			},
+			old:         &folders.Folder{},
+			expectedErr: "missing owner reference",
+		}}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/registry/apis/preferences/utils/names.go

@@ -1,6 +1,11 @@
 package utils
 
-import "strings"
+import (
+	"fmt"
+	"strings"
+
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
+)
 
 // +enum
 type ResourceOwner string
@@ -42,3 +47,26 @@ func ParseOwnerFromName(name string) (OwnerReference, bool) {
 	}
 	return OwnerReference{}, false
 }
+
+func (o OwnerReference) Validate(obj utils.GrafanaMetaAccessor) error {
+	// Make sure a team/root
+	refs := obj.GetOwnerReferences()
+	switch len(refs) {
+	case 0:
+		return fmt.Errorf("missing owner reference (%s)", o.AsName())
+	case 1: // OK
+	default:
+		return fmt.Errorf("multiple owner references (%s)", o.AsName())
+	}
+	ref := refs[0]
+	if ref.Name != o.Identifier {
+		return fmt.Errorf("owner reference must match the same name")
+	}
+	if strings.ToLower(ref.Kind) != string(o.Owner) {
+		return fmt.Errorf("owner reference kind must match the name")
+	}
+	if !strings.HasPrefix(ref.APIVersion, "iam.grafana.app/") {
+		return fmt.Errorf("owner reference should be iam.grafana.app")
+	}
+	return nil
+}

pkg/registry/apis/preferences/utils/names_test.go

@@ -4,7 +4,11 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 
+	preferences "github.com/grafana/grafana/apps/preferences/pkg/apis/preferences/v1alpha1"
+	utils1 "github.com/grafana/grafana/pkg/apimachinery/utils"
 	"github.com/grafana/grafana/pkg/registry/apis/preferences/utils"
 )
 
@@ -14,44 +18,121 @@ func TestLegacyAuthorizer(t *testing.T) {
 		input  string
 		output utils.OwnerReference
 		found  bool
-	}{
-		{
-			name:   "invalid",
-			input:  "xxx-yyy",
-			output: utils.OwnerReference{},
-			found:  false,
+		obj    runtime.Object
+		err    string
+	}{{
+		name:   "invalid",
+		input:  "xxx-yyy",
+		output: utils.OwnerReference{},
+		found:  false,
+	}, {
+		name:   "with user",
+		input:  "user-a",
+		output: utils.OwnerReference{Owner: utils.UserResourceOwner, Identifier: "a"},
+		found:  true,
+		obj: &preferences.Stars{
+			ObjectMeta: v1.ObjectMeta{
+				OwnerReferences: []v1.OwnerReference{{
+					APIVersion: "iam.grafana.app/v0alpha1",
+					Kind:       "User",
+					Name:       "a",
+				}},
+			},
 		},
-		{
-			name:   "with user",
-			input:  "user-a",
-			output: utils.OwnerReference{Owner: utils.UserResourceOwner, Identifier: "a"},
-			found:  true,
+	}, {
+		name:   "missing user",
+		input:  "user-",
+		output: utils.OwnerReference{},
+		found:  false,
+	}, {
+		name:   "with team",
+		input:  "team-b",
+		output: utils.OwnerReference{Owner: utils.TeamResourceOwner, Identifier: "b"},
+		found:  true,
+	}, {
+		name:   "missing team",
+		input:  "team-",
+		output: utils.OwnerReference{},
+		found:  false,
+	}, {
+		name:   "for namespace",
+		input:  "namespace",
+		output: utils.OwnerReference{Owner: utils.NamespaceResourceOwner},
+		found:  true,
+	}, {
+		name:   "missing reference",
+		input:  "user-a",
+		output: utils.OwnerReference{Owner: utils.UserResourceOwner, Identifier: "a"},
+		found:  true,
+		err:    "missing owner reference",
+		obj: &preferences.Stars{
+			ObjectMeta: v1.ObjectMeta{},
 		},
-		{
-			name:   "missing user",
-			input:  "user-",
-			output: utils.OwnerReference{},
-			found:  false,
+	}, {
+		name:   "names mismatch",
+		input:  "user-a",
+		output: utils.OwnerReference{Owner: utils.UserResourceOwner, Identifier: "a"},
+		found:  true,
+		err:    "owner reference must match the same name",
+		obj: &preferences.Stars{
+			ObjectMeta: v1.ObjectMeta{
+				OwnerReferences: []v1.OwnerReference{{
+					APIVersion: "iam.grafana.app/v0alpha1",
+					Kind:       "User",
+					Name:       "not-same-name",
+				}},
+			},
 		},
-		{
-			name:   "with team",
-			input:  "team-b",
-			output: utils.OwnerReference{Owner: utils.TeamResourceOwner, Identifier: "b"},
-			found:  true,
+	}, {
+		name:   "kinds mismatch",
+		input:  "user-a",
+		output: utils.OwnerReference{Owner: utils.UserResourceOwner, Identifier: "a"},
+		found:  true,
+		err:    "owner reference kind must match the name",
+		obj: &preferences.Stars{
+			ObjectMeta: v1.ObjectMeta{
+				OwnerReferences: []v1.OwnerReference{{
+					APIVersion: "iam.grafana.app/v0alpha1",
+					Kind:       "NotUserKind",
+					Name:       "a",
+				}},
+			},
 		},
-		{
-			name:   "missing team",
-			input:  "team-",
-			output: utils.OwnerReference{},
-			found:  false,
+	}, {
+		name:   "not iam",
+		input:  "user-a",
+		output: utils.OwnerReference{Owner: utils.UserResourceOwner, Identifier: "a"},
+		found:  true,
+		err:    "owner reference should be iam.grafana.app",
+		obj: &preferences.Stars{
+			ObjectMeta: v1.ObjectMeta{
+				OwnerReferences: []v1.OwnerReference{{
+					APIVersion: "something",
+					Kind:       "User",
+					Name:       "a",
+				}},
+			},
 		},
-		{
-			name:   "for namespace",
-			input:  "namespace",
-			output: utils.OwnerReference{Owner: utils.NamespaceResourceOwner},
-			found:  true,
+	}, {
+		name:   "multiple reference",
+		input:  "user-a",
+		output: utils.OwnerReference{Owner: utils.UserResourceOwner, Identifier: "a"},
+		found:  true,
+		err:    "multiple owner references",
+		obj: &preferences.Stars{
+			ObjectMeta: v1.ObjectMeta{
+				OwnerReferences: []v1.OwnerReference{{
+					APIVersion: "iam.grafana.app/v0alpha1",
+					Kind:       "User",
+					Name:       "a",
+				}, {
+					APIVersion: "iam.grafana.app/v0alpha1",
+					Kind:       "User",
+					Name:       "b",
+				}},
+			},
 		},
-	}
+	}}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -61,6 +142,17 @@ func TestLegacyAuthorizer(t *testing.T) {
 			if tt.found {
 				require.Equal(t, tt.input, output.AsName())
 			}
+
+			if tt.obj != nil {
+				obj, err := utils1.MetaAccessor(tt.obj)
+				require.NoError(t, err)
+				err = output.Validate(obj)
+				if tt.err == "" {
+					require.NoError(t, err)
+				} else {
+					require.ErrorContains(t, err, tt.err)
+				}
+			}
 		})
 	}
 }