From bd81243bbbc36896ce0f6f4438bc95f39b91fda3 Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Mon, 30 Jun 2025 17:38:23 -0600 Subject: [PATCH] Git sync: Implement folder deletion --- pkg/registry/apis/provisioning/files.go | 6 - .../apis/provisioning/repository/local.go | 9 +- .../provisioning/repository/local_test.go | 35 +++++ .../apis/provisioning/resources/dualwriter.go | 147 +++++++++++++++++- .../apis/provisioning/provisioning_test.go | 124 +++++++++++++++ .../provisioning/testdata/timeline-demo.json | 1 + 6 files changed, 313 insertions(+), 9 deletions(-) create mode 120000 pkg/tests/apis/provisioning/testdata/timeline-demo.json diff --git a/pkg/registry/apis/provisioning/files.go b/pkg/registry/apis/provisioning/files.go index 619bad795fe..a0548e1055b 100644 --- a/pkg/registry/apis/provisioning/files.go +++ b/pkg/registry/apis/provisioning/files.go @@ -128,12 +128,6 @@ func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime. return } - // TODO: Implement folder delete - if r.Method == http.MethodDelete && isDir { - responder.Error(apierrors.NewBadRequest("folder navigation not yet supported")) - return - } - var obj *provisioning.ResourceWrapper code := http.StatusOK switch r.Method { diff --git a/pkg/registry/apis/provisioning/repository/local.go b/pkg/registry/apis/provisioning/repository/local.go index fc7bd18c99f..8c1ca1d56c0 100644 --- a/pkg/registry/apis/provisioning/repository/local.go +++ b/pkg/registry/apis/provisioning/repository/local.go @@ -360,5 +360,12 @@ func (r *localRepository) Delete(ctx context.Context, path string, ref string, c return err } - return os.Remove(safepath.Join(r.path, path)) + fullPath := safepath.Join(r.path, path) + + if safepath.IsDir(path) { + // if it is a folder, delete all of its contents + return os.RemoveAll(fullPath) + } + + return os.Remove(fullPath) } diff --git a/pkg/registry/apis/provisioning/repository/local_test.go b/pkg/registry/apis/provisioning/repository/local_test.go index 7aabc542402..8881ab3cb6f 100644 --- a/pkg/registry/apis/provisioning/repository/local_test.go +++ b/pkg/registry/apis/provisioning/repository/local_test.go @@ -542,6 +542,41 @@ func TestLocalRepository_Delete(t *testing.T) { comment: "test delete with ref", expectedErr: apierrors.NewBadRequest("local repository does not support ref"), }, + { + name: "delete folder with nested files", + setup: func(t *testing.T) (string, *localRepository) { + tempDir := t.TempDir() + nestedFolderPath := filepath.Join(tempDir, "folder") + err := os.MkdirAll(nestedFolderPath, 0700) + require.NoError(t, err) + subFolderPath := filepath.Join(nestedFolderPath, "nested-folder") + err = os.MkdirAll(subFolderPath, 0700) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(nestedFolderPath, "nested-dash.txt"), []byte("content1"), 0600) + require.NoError(t, err) + + // Create repository with the temp directory as permitted prefix + repo := &localRepository{ + config: &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Local: &provisioning.LocalRepositoryConfig{ + Path: tempDir, + }, + }, + }, + resolver: &LocalFolderResolver{ + PermittedPrefixes: []string{tempDir}, + }, + path: tempDir, + } + + return tempDir, repo + }, + path: "folder/", + ref: "", + comment: "test delete folder with nested content", + expectedErr: nil, + }, } for _, tc := range testCases { diff --git a/pkg/registry/apis/provisioning/resources/dualwriter.go b/pkg/registry/apis/provisioning/resources/dualwriter.go index 7d7b46e0406..d35c66b4f16 100644 --- a/pkg/registry/apis/provisioning/resources/dualwriter.go +++ b/pkg/registry/apis/provisioning/resources/dualwriter.go @@ -3,9 +3,12 @@ package resources import ( "context" "fmt" + "sort" + "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana-app-sdk/logging" @@ -76,9 +79,8 @@ func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*Pa return nil, err } - // TODO: implement this if safepath.IsDir(opts.Path) { - return nil, fmt.Errorf("folder delete not supported") + return r.deleteFolder(ctx, opts) } // Read the file from the default branch as it won't exist in the possibly new branch @@ -131,6 +133,24 @@ func (r *DualReadWriter) Delete(ctx context.Context, opts DualWriteOptions) (*Pa return parsed, err } +func (r *DualReadWriter) getConfiguredBranch() string { + cfg := r.repo.Config() + switch cfg.Spec.Type { + case provisioning.GitHubRepositoryType: + if cfg.Spec.GitHub != nil { + return cfg.Spec.GitHub.Branch + } + case provisioning.GitRepositoryType: + if cfg.Spec.Git != nil { + return cfg.Spec.Git.Branch + } + case provisioning.LocalRepositoryType: + // branches are not supported for local repositories + return "" + } + return "" +} + // CreateFolder creates a new folder in the repository // FIXME: fix signature to return ParsedResource func (r *DualReadWriter) CreateFolder(ctx context.Context, opts DualWriteOptions) (*provisioning.ResourceWrapper, error) { @@ -317,3 +337,126 @@ func (r *DualReadWriter) authorizeCreateFolder(ctx context.Context, _ string) er return apierrors.NewForbidden(FolderResource.GroupResource(), "", fmt.Errorf("must be admin or editor to access folders with provisioning")) } + +func (r *DualReadWriter) deleteFolder(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) { + // if the ref is not the active branch, just delete the files from the branch + // do not delete the items from grafana itself + if opts.Ref != "" && opts.Ref != r.getConfiguredBranch() { + err := r.repo.Delete(ctx, opts.Path, opts.Ref, opts.Message) + if err != nil { + return nil, fmt.Errorf("error deleting folder from repository: %w", err) + } + + return folderDeleteResponse(opts.Path, opts.Ref, r.repo.Config()), nil + } + + // before deleting from the repo, first get all children resources to delete from grafana afterwards + treeEntries, err := r.repo.ReadTree(ctx, "") + if err != nil { + return nil, fmt.Errorf("read repository tree: %w", err) + } + // note: parsedFolders will include the folder itself + parsedResources, parsedFolders, err := r.getChildren(ctx, opts.Path, treeEntries) + if err != nil { + return nil, fmt.Errorf("parse resources in folder: %w", err) + } + + // delete from the repo + err = r.repo.Delete(ctx, opts.Path, opts.Ref, opts.Message) + if err != nil { + return nil, fmt.Errorf("delete folder from repository: %w", err) + } + + // delete from grafana + ctx, _, err = identity.WithProvisioningIdentity(ctx, r.repo.Config().Namespace) + if err != nil { + return nil, err + } + if err := r.deleteChildren(ctx, parsedResources, parsedFolders); err != nil { + return nil, fmt.Errorf("delete folder from grafana: %w", err) + } + + return folderDeleteResponse(opts.Path, opts.Ref, r.repo.Config()), nil +} + +func folderDeleteResponse(path, ref string, cfg *provisioning.Repository) *ParsedResource { + return &ParsedResource{ + Action: provisioning.ResourceActionDelete, + Info: &repository.FileInfo{ + Path: path, + Ref: ref, + }, + GVK: schema.GroupVersionKind{ + Group: FolderResource.Group, + Version: FolderResource.Version, + Kind: "Folder", + }, + GVR: FolderResource, + Repo: provisioning.ResourceRepositoryInfo{ + Type: cfg.Spec.Type, + Namespace: cfg.Namespace, + Name: cfg.Name, + Title: cfg.Spec.Title, + }, + } +} + +func (r *DualReadWriter) getChildren(ctx context.Context, folderPath string, treeEntries []repository.FileTreeEntry) ([]*ParsedResource, []Folder, error) { + var resourcesInFolder []repository.FileTreeEntry + var foldersInFolder []Folder + for _, entry := range treeEntries { + // the folder itself should be included in this, to do that, trim the suffix of the folder path and see if it matches exactly + if !strings.HasPrefix(entry.Path, folderPath) && entry.Path != strings.TrimSuffix(folderPath, "/") { + continue + } + // folders cannot be parsed as resources, so handle them separately + if entry.Blob { + resourcesInFolder = append(resourcesInFolder, entry) + } else { + folder := ParseFolder(entry.Path, r.repo.Config().Name) + foldersInFolder = append(foldersInFolder, folder) + } + } + + var parsedResources []*ParsedResource + for _, entry := range resourcesInFolder { + fileInfo, err := r.repo.Read(ctx, entry.Path, "") + if err != nil && !apierrors.IsNotFound(err) { + return nil, nil, fmt.Errorf("could not find resource in repository: %w", err) + } + + parsed, err := r.parser.Parse(ctx, fileInfo) + if err != nil { + return nil, nil, fmt.Errorf("could not parse resource: %w", err) + } + + parsedResources = append(parsedResources, parsed) + } + + return parsedResources, foldersInFolder, nil +} + +func (r *DualReadWriter) deleteChildren(ctx context.Context, childrenResources []*ParsedResource, folders []Folder) error { + for _, parsed := range childrenResources { + err := parsed.Client.Delete(ctx, parsed.Obj.GetName(), metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete nested resource from grafana: %w", err) + } + } + + // we need to delete the folders furthest down in the tree first, as folder deletion will fail if there is anything inside of it + sort.Slice(folders, func(i, j int) bool { + depthI := strings.Count(folders[i].Path, "/") + depthJ := strings.Count(folders[j].Path, "/") + + return depthI > depthJ + }) + for _, f := range folders { + err := r.folders.Client().Delete(ctx, f.ID, metav1.DeleteOptions{}) + if err != nil { + return fmt.Errorf("failed to delete folder from grafana: %w", err) + } + } + + return nil +} diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index f9a4be946fc..b00501bdd2b 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -3,6 +3,7 @@ package provisioning import ( "context" "encoding/json" + "fmt" "net/http" "os" "path/filepath" @@ -651,3 +652,126 @@ func TestProvisioning_ExportUnifiedToRepository(t *testing.T) { require.Nil(t, obj["status"], "should not have a status element") } } + +func TestIntegrationProvisioning_DeleteResources(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + helper := runGrafana(t) + ctx := context.Background() + + const repo = "delete-test-repo" + localTmp := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{ + "Name": repo, + "SyncEnabled": true, + "SyncTarget": "instance", + }) + _, err := helper.Repositories.Resource.Create(ctx, localTmp, metav1.CreateOptions{}) + require.NoError(t, err) + + // create the structure: + // dashboard1.json + // folder/ + // dashboard2.json + // nested/ + // dashboard3.json + dashboard1 := helper.LoadFile("testdata/all-panels.json") + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "dashboard1.json"). + Body(dashboard1). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error()) + dashboard2 := helper.LoadFile("testdata/text-options.json") + result = helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "folder", "dashboard2.json"). + Body(dashboard2). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error()) + dashboard3 := helper.LoadFile("testdata/timeline-demo.json") + result = helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "folder", "nested", "dashboard3.json"). + Body(dashboard3). + SetHeader("Content-Type", "application/json"). + Do(ctx) + require.NoError(t, result.Error()) + + helper.SyncAndWait(t, repo, nil) + + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 3, len(dashboards.Items)) + + folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 2, len(folders.Items)) + + t.Run("delete individual dashboard file, should delete from repo and grafana", func(t *testing.T) { + result := helper.AdminREST.Delete(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "dashboard1.json"). + Do(ctx) + require.NoError(t, result.Error()) + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard1.json") + require.Error(t, err) + dashboards, err = helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, 2, len(dashboards.Items)) + }) + + t.Run("delete folder, should delete from repo and grafana all nested resources too", func(t *testing.T) { + // need to delete directly through the url, because the k8s client doesn't support `/` in a subresource + // but that is needed by gitsync to know that it is a folder + addr := helper.GetEnv().Server.HTTPServer.Listener.Addr().String() + url := fmt.Sprintf("http://admin:admin@%s/apis/provisioning.grafana.app/v0alpha1/namespaces/default/repositories/%s/files/folder/", addr, repo) + req, err := http.NewRequest(http.MethodDelete, url, nil) + require.NoError(t, err) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + + // should be deleted from the repo + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder") + require.Error(t, err) + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder", "dashboard2.json") + require.Error(t, err) + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder", "nested") + require.Error(t, err) + _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder", "nested", "dashboard3.json") + require.Error(t, err) + + // all should be deleted from grafana + for _, d := range dashboards.Items { + _, err = helper.DashboardsV1.Resource.Get(ctx, d.GetName(), metav1.GetOptions{}) + require.Error(t, err) + } + for _, f := range folders.Items { + _, err = helper.Folders.Resource.Get(ctx, f.GetName(), metav1.GetOptions{}) + require.Error(t, err) + } + }) + + t.Run("deleting a non-existent file should fail", func(t *testing.T) { + result := helper.AdminREST.Delete(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("files", "non-existent.json"). + Do(ctx) + require.Error(t, result.Error()) + }) +} diff --git a/pkg/tests/apis/provisioning/testdata/timeline-demo.json b/pkg/tests/apis/provisioning/testdata/timeline-demo.json new file mode 120000 index 00000000000..af7d0ce7aac --- /dev/null +++ b/pkg/tests/apis/provisioning/testdata/timeline-demo.json @@ -0,0 +1 @@ +../../../../../devenv/dev-dashboards/panel-timeline/timeline-demo.json \ No newline at end of file