diff --git a/pkg/registry/apis/dashboard/register.go b/pkg/registry/apis/dashboard/register.go index a8e5de42e39..309c46b4363 100644 --- a/pkg/registry/apis/dashboard/register.go +++ b/pkg/registry/apis/dashboard/register.go @@ -10,6 +10,7 @@ import ( "github.com/prometheus/client_golang/prometheus" apierrors "k8s.io/apimachinery/pkg/api/errors" 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" @@ -329,9 +330,14 @@ func (b *DashboardsAPIBuilder) validateCreate(ctx context.Context, a admission.A // Validate folder existence if specified if !a.IsDryRun() && accessor.GetFolder() != "" { - if err := b.validateFolderExists(ctx, accessor.GetFolder(), id.GetOrgID()); err != nil { + folder, err := b.validateFolderExists(ctx, accessor.GetFolder(), id.GetOrgID()) + if err != nil { return err } + + if err := b.validateFolderManagedBySameManager(folder, accessor); err != nil { + return apierrors.NewBadRequest(err.Error()) + } } // Validate quota @@ -398,9 +404,14 @@ func (b *DashboardsAPIBuilder) validateUpdate(ctx context.Context, a admission.A return err } - if err := b.validateFolderExists(ctx, newAccessor.GetFolder(), nsInfo.OrgID); err != nil { + folder, err := b.validateFolderExists(ctx, newAccessor.GetFolder(), nsInfo.OrgID) + if err != nil { return apierrors.NewNotFound(folders.FolderResourceInfo.GroupResource(), newAccessor.GetFolder()) } + + if err := b.validateFolderManagedBySameManager(folder, newAccessor); err != nil { + return err + } } // Validate refresh interval @@ -412,21 +423,43 @@ func (b *DashboardsAPIBuilder) validateUpdate(ctx context.Context, a admission.A } // validateFolderExists checks if a folder exists -func (b *DashboardsAPIBuilder) validateFolderExists(ctx context.Context, folderUID string, orgID int64) error { +func (b *DashboardsAPIBuilder) validateFolderExists(ctx context.Context, folderUID string, orgID int64) (*unstructured.Unstructured, error) { ns, err := request.NamespaceInfoFrom(ctx, false) if err != nil { - return err + return nil, err } folderClient := b.folderClientProvider.GetOrCreateHandler(ns.Value) - _, err = folderClient.Get(ctx, folderUID, orgID, metav1.GetOptions{}) + folder, err := folderClient.Get(ctx, folderUID, orgID, metav1.GetOptions{}) // Check if the error is a context deadline exceeded error if err != nil { // historically, we returned a more verbose error with folder name when its not found, below just keeps that behavior if apierrors.IsNotFound(err) { - return apierrors.NewNotFound(folders.FolderResourceInfo.GroupResource(), folderUID) + return nil, apierrors.NewNotFound(folders.FolderResourceInfo.GroupResource(), folderUID) } - return err + return nil, err + } + + return folder, nil +} + +// validation should fail if: +// 1. The parent folder is managed but this dashboard is not +// 2. The parent folder is managed by a different repository than this dashboard +func (b *DashboardsAPIBuilder) validateFolderManagedBySameManager(folder *unstructured.Unstructured, dashboardAccessor utils.GrafanaMetaAccessor) error { + folderAccessor, err := utils.MetaAccessor(folder) + if err != nil { + return fmt.Errorf("error getting meta accessor: %w", err) + } + + if folderManager, ok := folderAccessor.GetManagerProperties(); ok && folderManager.Kind == utils.ManagerKindRepo { + manager, ok := dashboardAccessor.GetManagerProperties() + if !ok { + return fmt.Errorf("folder is managed by a repository, but the dashboard is not managed") + } + if manager.Kind != utils.ManagerKindRepo || manager.Identity != folderManager.Identity { + return fmt.Errorf("folder is managed by a repository, but the dashboard is not managed by the same manager") + } } return nil diff --git a/pkg/registry/apis/dashboard/register_test.go b/pkg/registry/apis/dashboard/register_test.go index f788d5dbe67..b349975ccc4 100644 --- a/pkg/registry/apis/dashboard/register_test.go +++ b/pkg/registry/apis/dashboard/register_test.go @@ -8,6 +8,8 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" 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" @@ -15,7 +17,9 @@ import ( dashv1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" dashv2alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1" dashv2beta1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2beta1" + folderv1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/user" @@ -257,3 +261,132 @@ func (m *mockFeatureToggles) GetEnabled(ctx context.Context) map[string]bool { return res } + +func TestDashboardAPIBuilder_validateFolderManagedBySameManager(t *testing.T) { + tests := []struct { + name string + folderManager *utils.ManagerProperties + dashboardManager *utils.ManagerProperties + expectedError bool + }{ + { + name: "folder not managed by repository", + folderManager: nil, + dashboardManager: nil, + expectedError: false, + }, + { + name: "folder not managed by repository, dashboard managed", + folderManager: nil, + dashboardManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + expectedError: false, + }, + { + name: "folder managed by plugin", + folderManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindPlugin, + Identity: "plugin-1", + }, + dashboardManager: nil, + expectedError: false, + }, + { + name: "folder and dashboard managed by same repository", + folderManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + dashboardManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + expectedError: false, + }, + { + name: "folder managed by repository, dashboard is not managed", + folderManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + dashboardManager: nil, + expectedError: true, + }, + { + name: "folder and dashboard managed by different repositories", + folderManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + dashboardManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-2", + }, + expectedError: true, + }, + { + name: "folder managed by repository, dashboard managed by plugin", + folderManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + dashboardManager: &utils.ManagerProperties{ + Kind: utils.ManagerKindPlugin, + Identity: "plugin-1", + }, + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + folder := &folderv1.Folder{ + TypeMeta: metav1.TypeMeta{ + Kind: "Folder", + APIVersion: "folder.grafana.app/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "folder-1", + Namespace: "default", + }, + } + if tt.folderManager != nil { + folderAccessor, err := utils.MetaAccessor(folder) + require.NoError(t, err) + folderAccessor.SetManagerProperties(*tt.folderManager) + } + folderUnstructured := &unstructured.Unstructured{} + folderMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(folder) + require.NoError(t, err) + folderUnstructured.Object = folderMap + + dashboard := &dashv1.Dashboard{ + TypeMeta: metav1.TypeMeta{ + Kind: "Dashboard", + APIVersion: "dashboard.grafana.app/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "dashboard-1", + Namespace: "default", + }, + } + if tt.dashboardManager != nil { + dashboardAccessor, err := utils.MetaAccessor(dashboard) + require.NoError(t, err) + dashboardAccessor.SetManagerProperties(*tt.dashboardManager) + } + dashboardAccessor, err := utils.MetaAccessor(dashboard) + require.NoError(t, err) + + builder := &DashboardsAPIBuilder{} + err = builder.validateFolderManagedBySameManager(folderUnstructured, dashboardAccessor) + if tt.expectedError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +}