From 9785e573aa9ea71b03c7461e4cf3e4aac05c1557 Mon Sep 17 00:00:00 2001 From: Costa Alexoglou Date: Wed, 27 Aug 2025 12:20:57 +0200 Subject: [PATCH] Provisioning: Fix Dashboard Creation For First-Level Repository Folders (#109962) --- .../apis/provisioning/resources/parser.go | 5 ++ .../provisioning/resources/parser_test.go | 50 +++++++++++++++++++ pkg/tests/apis/provisioning/files_test.go | 20 ++++++++ pkg/tests/apis/provisioning/helper_test.go | 28 +++++++++++ 4 files changed, 103 insertions(+) diff --git a/pkg/registry/apis/provisioning/resources/parser.go b/pkg/registry/apis/provisioning/resources/parser.go index 35b94007390..447fb3eda11 100644 --- a/pkg/registry/apis/provisioning/resources/parser.go +++ b/pkg/registry/apis/provisioning/resources/parser.go @@ -65,6 +65,7 @@ func (f *parserFactory) GetParser(ctx context.Context, repo repository.Reader) ( }, urls: urls, clients: clients, + config: config, }, nil } @@ -75,6 +76,8 @@ type parser struct { // for repositories that have URL support urls repository.RepositoryWithURLs + config *provisioning.Repository + // ResourceClients give access to k8s apis clients ResourceClients } @@ -192,6 +195,8 @@ func (r *parser) Parse(ctx context.Context, info *repository.FileInfo) (parsed * dirPath := safepath.Dir(info.Path) if dirPath != "" { parsed.Meta.SetFolder(ParseFolder(dirPath, r.repo.Name).ID) + } else { + parsed.Meta.SetFolder(RootFolder(r.config)) } } obj.SetUID("") // clear identifiers diff --git a/pkg/registry/apis/provisioning/resources/parser_test.go b/pkg/registry/apis/provisioning/resources/parser_test.go index 61f374a8e05..c3aa679be84 100644 --- a/pkg/registry/apis/provisioning/resources/parser_test.go +++ b/pkg/registry/apis/provisioning/resources/parser_test.go @@ -11,6 +11,7 @@ import ( dashboardV1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestParser(t *testing.T) { @@ -27,6 +28,16 @@ func TestParser(t *testing.T) { Name: "repo", }, clients: clients, + config: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "xxx", + Name: "repo", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.LocalRepositoryType, + Sync: provisioning.SyncOptions{Target: provisioning.SyncTargetTypeFolder}, + }, + }, } t.Run("invalid input", func(t *testing.T) { @@ -93,4 +104,43 @@ spec: require.Equal(t, "dashboard.grafana.app", dash.GVR.Group) require.Equal(t, "v0alpha1", dash.GVR.Version) }) + + t.Run("validate proper folder metadata is set", func(t *testing.T) { + testCases := []struct { + name string + filePath string + expectedFolder string + }{ + { + name: "file in subdirectory should use parsed folder ID", + filePath: "team-a/testing-valid-dashboard.json", + expectedFolder: ParseFolder("team-a/", "repo").ID, + }, + { + name: "file in first-level directory should use parent folder id", + filePath: "testing-valid-dashboard.json", + expectedFolder: parser.repo.Name, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dash, err := parser.Parse(context.Background(), &repository.FileInfo{ + Path: tc.filePath, + Data: []byte(`apiVersion: dashboard.grafana.app/v0alpha1 +kind: Dashboard +metadata: + name: test-dashboard +spec: + title: Test dashboard +`), + }) + require.NoError(t, err) + require.Equal(t, tc.expectedFolder, dash.Meta.GetFolder(), "folder should match expected") + annotations := dash.Obj.GetAnnotations() + require.NotNil(t, annotations, "annotations should not be nil") + require.Equal(t, tc.expectedFolder, annotations["grafana.app/folder"], "folder annotation should match expected") + }) + } + }) } diff --git a/pkg/tests/apis/provisioning/files_test.go b/pkg/tests/apis/provisioning/files_test.go index 7fa121e00f5..24a25849ecf 100644 --- a/pkg/tests/apis/provisioning/files_test.go +++ b/pkg/tests/apis/provisioning/files_test.go @@ -43,6 +43,8 @@ func TestIntegrationProvisioning_DeleteResources(t *testing.T) { require.NoError(t, err) require.Equal(t, 3, len(dashboards.Items)) + helper.validateManagedDashboardsFolderMetadata(t, ctx, repo, dashboards.Items) + folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{}) require.NoError(t, err) require.Equal(t, 2, len(folders.Items)) @@ -126,6 +128,13 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) { ExpectedFolders: 0, }) + // Validate the dashboard metadata + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 1, len(dashboards.Items)) + + helper.validateManagedDashboardsFolderMetadata(t, ctx, repo, dashboards.Items) + // Verify the original dashboard exists in Grafana (using the UID from all-panels.json) const allPanelsUID = "n1jR8vnnz" // This is the UID from the all-panels.json file obj, err := helper.DashboardsV1.Resource.Get(ctx, allPanelsUID, metav1.GetOptions{}) @@ -420,6 +429,17 @@ func TestIntegrationProvisioning_FilesOwnershipProtection(t *testing.T) { ExpectedFolders: 2, // Total across both repos }) + allDashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + for _, dashboard := range allDashboards.Items { + annotations := dashboard.GetAnnotations() + // Expect to be managed by repo1 or repo2 + managerID := annotations["grafana.app/managerId"] + if managerID != repo1 && managerID != repo2 { + t.Fatalf("dashboard %s is not managed by repo1 or repo2", dashboard.GetName()) + } + } + t.Run("CREATE file with UID already owned by different repository - should fail", func(t *testing.T) { // Try to create a dashboard in repo2 that has the same UID as the one in repo1 // The all-panels.json has UID "n1jR8vnnz" which is already owned by repo1 diff --git a/pkg/tests/apis/provisioning/helper_test.go b/pkg/tests/apis/provisioning/helper_test.go index 1d03cbe1d02..3ba1c7e7604 100644 --- a/pkg/tests/apis/provisioning/helper_test.go +++ b/pkg/tests/apis/provisioning/helper_test.go @@ -460,6 +460,34 @@ func (h *provisioningTestHelper) logRepositoryObject(t *testing.T, obj map[strin } } +// validateManagedDashboardsFolderMetadata validates the folder metadata +// of the managed dashboards. +// If folder is nested, folder annotations should not be empty. +// Also checks that the managerId property exists. +func (h *provisioningTestHelper) validateManagedDashboardsFolderMetadata(t *testing.T, + ctx context.Context, repoName string, dashboards []unstructured.Unstructured) { + t.Helper() + + // Check if folder is nested or not. + // If not, folder annotations should be empty as we have an "instance" sync target + for _, d := range dashboards { + sourcePath, _, _ := unstructured.NestedString(d.Object, "metadata", "annotations", "grafana.app/sourcePath") + isNested := strings.Contains(sourcePath, "/") + + folder, found, _ := unstructured.NestedString(d.Object, "metadata", "annotations", "grafana.app/folder") + if isNested { + require.True(t, found, "dashboard should have a folder annotation") + require.NotEmpty(t, folder, "dashboard should be in a non-empty folder") + } else { + require.False(t, found, "dashboard should not have a folder annotation") + } + + managerID, _, _ := unstructured.NestedString(d.Object, "metadata", "annotations", "grafana.app/managerId") + // require.Equal(t, repoName, managerID, "dashboard should be managed by gitsync repo") + require.Equal(t, repoName, managerID, "dashboard should be managed by gitsync repo") + } +} + type TestRepo struct { Name string Target string