Provisioning: Deprecate folder move and delete on configured branch (#115329)
* Provisioning: Deprecate single file/folder move and delete on configured branch Reject individual file and folder move/delete operations on the configured branch via the single files endpoints (HTTP 405 MethodNotAllowed). Users must use the bulk operations API (jobs API) instead. Motivation: - Reconciliation for these operations is not reliable as it must be recursive and cannot run synchronously since it could take a long time - Simplifies authorization logic - fewer operations to secure and validate - Reduces complexity and surface area for potential bugs - Bulk operations via jobs API provide better control and observability Operations on non-configured branches (e.g., creating PRs) continue to work as before since they don't update the Grafana database. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: remove trailing whitespace in test file * Fix behaviour to match current behavior * Revert changes for individual files --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Roberto Jiménez Sánchez
co-authored by
Claude Sonnet 4.5
Claude Sonnet 4.5
parent
9ceff992aa
commit
35affc57c2
@@ -3,6 +3,7 @@ package resources
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
apierrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
@@ -315,7 +316,19 @@ func (r *DualReadWriter) MoveResource(ctx context.Context, opts DualWriteOptions
|
||||
}
|
||||
|
||||
func (r *DualReadWriter) moveDirectory(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) {
|
||||
// For directory moves, we just perform the repository move without parsing
|
||||
// Reject directory move operations for configured branch - use bulk operations instead
|
||||
if r.isConfiguredBranch(opts) {
|
||||
return nil, &apierrors.StatusError{
|
||||
ErrStatus: metav1.Status{
|
||||
Status: metav1.StatusFailure,
|
||||
Code: http.StatusMethodNotAllowed,
|
||||
Reason: metav1.StatusReasonMethodNotAllowed,
|
||||
Message: "directory move operations are not available for configured branch. Use bulk move operations via the jobs API instead",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// For branch operations, we just perform the repository move without updating Grafana DB
|
||||
// Always use the provisioning identity when writing
|
||||
ctx, _, err := identity.WithProvisioningIdentity(ctx, r.repo.Config().Namespace)
|
||||
if err != nil {
|
||||
@@ -349,35 +362,6 @@ func (r *DualReadWriter) moveDirectory(ctx context.Context, opts DualWriteOption
|
||||
},
|
||||
}
|
||||
|
||||
// Handle folder management for main branch
|
||||
if r.shouldUpdateGrafanaDB(opts, nil) {
|
||||
// Ensure destination folder path exists
|
||||
if _, err := r.folders.EnsureFolderPathExist(ctx, opts.Path); err != nil {
|
||||
return nil, fmt.Errorf("ensure destination folder path exists: %w", err)
|
||||
}
|
||||
|
||||
// Try to delete the old folder structure from grafana (if it exists)
|
||||
// This handles cleanup when folders are moved to new locations
|
||||
oldFolderName, err := r.folders.EnsureFolderPathExist(ctx, opts.OriginalPath)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("ensure original folder path exists: %w", err)
|
||||
}
|
||||
|
||||
if oldFolderName != "" {
|
||||
oldFolder, err := r.folders.GetFolder(ctx, oldFolderName)
|
||||
if err != nil && !apierrors.IsNotFound(err) {
|
||||
return nil, fmt.Errorf("get old folder for cleanup: %w", err)
|
||||
}
|
||||
|
||||
if err == nil {
|
||||
err = r.folders.Client().Delete(ctx, oldFolder.GetName(), metav1.DeleteOptions{})
|
||||
if err != nil && !apierrors.IsNotFound(err) {
|
||||
return nil, fmt.Errorf("delete old folder from storage: %w", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return parsed, nil
|
||||
}
|
||||
|
||||
@@ -551,41 +535,22 @@ func (r *DualReadWriter) authorizeCreateFolder(ctx context.Context, _ string) er
|
||||
}
|
||||
|
||||
func (r *DualReadWriter) deleteFolder(ctx context.Context, opts DualWriteOptions) (*ParsedResource, error) {
|
||||
// if the ref is set, it is not the active branch, so just delete the files from the branch
|
||||
// and do not delete the items from grafana itself
|
||||
if !r.shouldUpdateGrafanaDB(opts, nil) {
|
||||
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)
|
||||
// Reject directory delete operations for configured branch - use bulk operations instead
|
||||
if r.isConfiguredBranch(opts) {
|
||||
return nil, &apierrors.StatusError{
|
||||
ErrStatus: metav1.Status{
|
||||
Status: metav1.StatusFailure,
|
||||
Code: http.StatusMethodNotAllowed,
|
||||
Reason: metav1.StatusReasonMethodNotAllowed,
|
||||
Message: "directory delete operations are not available for configured branch. Use bulk delete operations via the jobs API instead",
|
||||
},
|
||||
}
|
||||
|
||||
return folderDeleteResponse(ctx, opts.Path, opts.Ref, r.repo)
|
||||
}
|
||||
|
||||
// before deleting from the repo, first get all children resources to delete from grafana afterwards
|
||||
treeEntries, err := r.repo.ReadTree(ctx, "")
|
||||
// For branch operations, just delete from the repository without updating Grafana DB
|
||||
err := r.repo.Delete(ctx, opts.Path, opts.Ref, opts.Message)
|
||||
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 nil, fmt.Errorf("error deleting folder from repository: %w", err)
|
||||
}
|
||||
|
||||
return folderDeleteResponse(ctx, opts.Path, opts.Ref, r.repo)
|
||||
@@ -640,60 +605,11 @@ func folderDeleteResponse(ctx context.Context, path, ref string, repo repository
|
||||
return parsed, nil
|
||||
}
|
||||
|
||||
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 {
|
||||
// make sure the path is supported (i.e. not ignored by git sync) and that the path is the folder itself or a child of the folder
|
||||
if IsPathSupported(entry.Path) != nil || !safepath.InDir(entry.Path, 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)
|
||||
}
|
||||
}
|
||||
|
||||
parsedResources := make([]*ParsedResource, len(resourcesInFolder))
|
||||
for i, 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[i] = 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
|
||||
safepath.SortByDepth(folders, func(f Folder) string { return f.Path }, false)
|
||||
|
||||
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
|
||||
// isConfiguredBranch returns true if the ref targets the configured branch
|
||||
// (empty ref means configured branch, or ref explicitly matches configured branch)
|
||||
func (r *DualReadWriter) isConfiguredBranch(opts DualWriteOptions) bool {
|
||||
configuredBranch := r.repo.Config().Branch()
|
||||
return opts.Ref == "" || opts.Ref == configuredBranch
|
||||
}
|
||||
|
||||
// shouldUpdateGrafanaDB returns true if we have an empty ref (targeting the configured branch)
|
||||
@@ -703,9 +619,5 @@ func (r *DualReadWriter) shouldUpdateGrafanaDB(opts DualWriteOptions, parsed *Pa
|
||||
return false
|
||||
}
|
||||
|
||||
if opts.Ref != "" && opts.Ref != opts.Branch {
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
return r.isConfiguredBranch(opts)
|
||||
}
|
||||
|
||||
@@ -68,22 +68,45 @@ func TestIntegrationProvisioning_DeleteResources(t *testing.T) {
|
||||
|
||||
helper.validateManagedDashboardsFolderMetadata(t, ctx, repo, dashboards.Items)
|
||||
|
||||
t.Run("delete individual dashboard file, should delete from repo and grafana", func(t *testing.T) {
|
||||
t.Run("delete individual dashboard file on configured branch should succeed", 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))
|
||||
require.NoError(t, result.Error(), "delete file on configured branch should succeed")
|
||||
|
||||
// Verify the dashboard is removed from Grafana
|
||||
const allPanelsUID = "n1jR8vnnz" // UID from all-panels.json
|
||||
_, err := helper.DashboardsV1.Resource.Get(ctx, allPanelsUID, metav1.GetOptions{})
|
||||
require.Error(t, err, "dashboard should be deleted from Grafana")
|
||||
require.True(t, apierrors.IsNotFound(err), "should return NotFound for deleted dashboard")
|
||||
})
|
||||
|
||||
t.Run("delete folder, should delete from repo and grafana all nested resources too", func(t *testing.T) {
|
||||
t.Run("delete individual dashboard file on branch should succeed", func(t *testing.T) {
|
||||
// Create a branch first by creating a file on a branch
|
||||
branchRef := "test-branch-delete"
|
||||
helper.CopyToProvisioningPath(t, "testdata/text-options.json", "branch-test-delete.json")
|
||||
|
||||
// Delete on branch should work
|
||||
result := helper.AdminREST.Delete().
|
||||
Namespace("default").
|
||||
Resource("repositories").
|
||||
Name(repo).
|
||||
SubResource("files", "branch-test-delete.json").
|
||||
Param("ref", branchRef).
|
||||
Do(ctx)
|
||||
// Note: This might fail if branch doesn't exist, but the important thing is it doesn't return MethodNotAllowed
|
||||
if result.Error() != nil {
|
||||
var statusErr *apierrors.StatusError
|
||||
if errors.As(result.Error(), &statusErr) {
|
||||
require.NotEqual(t, int32(http.StatusMethodNotAllowed), statusErr.ErrStatus.Code, "should not return MethodNotAllowed for branch delete")
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("delete folder on configured branch should return MethodNotAllowed", 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()
|
||||
@@ -94,27 +117,11 @@ func TestIntegrationProvisioning_DeleteResources(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
// nolint:errcheck
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
require.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode, "should return MethodNotAllowed for configured branch folder delete")
|
||||
|
||||
// should be deleted from the repo
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "folder")
|
||||
require.Error(t, err)
|
||||
// Verify a file inside the folder still exists (operation was rejected)
|
||||
_, 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)
|
||||
}
|
||||
require.NoError(t, err, "file inside folder should still exist after rejected delete")
|
||||
})
|
||||
|
||||
t.Run("deleting a non-existent file should fail", func(t *testing.T) {
|
||||
@@ -158,10 +165,10 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) {
|
||||
require.NoError(t, err, "original dashboard should exist in Grafana")
|
||||
require.Equal(t, repo, obj.GetAnnotations()[utils.AnnoKeyManagerIdentity])
|
||||
|
||||
t.Run("move file without content change", func(t *testing.T) {
|
||||
t.Run("move file without content change on configured branch should succeed", func(t *testing.T) {
|
||||
const targetPath = "moved/simple-move.json"
|
||||
|
||||
// Perform the move operation using helper function
|
||||
// Perform the move operation using helper function (no ref = configured branch)
|
||||
resp := helper.postFilesRequest(t, repo, filesPostOptions{
|
||||
targetPath: targetPath,
|
||||
originalPath: "all-panels.json",
|
||||
@@ -169,32 +176,52 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) {
|
||||
})
|
||||
// nolint:errcheck
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "move operation should succeed")
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "move operation on configured branch should succeed")
|
||||
|
||||
// Verify the file moved in the repository
|
||||
movedObj, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved", "simple-move.json")
|
||||
require.NoError(t, err, "moved file should exist in repository")
|
||||
// Verify file was moved - read from new location
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved", "simple-move.json")
|
||||
require.NoError(t, err, "file should exist at new location")
|
||||
|
||||
// Check the content is preserved (verify it's still the all-panels dashboard)
|
||||
resource, _, err := unstructured.NestedMap(movedObj.Object, "resource")
|
||||
require.NoError(t, err)
|
||||
dryRun, _, err := unstructured.NestedMap(resource, "dryRun")
|
||||
require.NoError(t, err)
|
||||
title, _, err := unstructured.NestedString(dryRun, "spec", "title")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "Panel tests - All panels", title, "content should be preserved")
|
||||
|
||||
// Verify original file no longer exists
|
||||
// Verify file no longer exists at old location
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "all-panels.json")
|
||||
require.Error(t, err, "original file should no longer exist")
|
||||
|
||||
// Verify dashboard still exists in Grafana with same content but may have updated path references
|
||||
helper.SyncAndWait(t, repo, nil)
|
||||
_, err = helper.DashboardsV1.Resource.Get(ctx, allPanelsUID, metav1.GetOptions{})
|
||||
require.NoError(t, err, "dashboard should still exist in Grafana after move")
|
||||
require.Error(t, err, "file should not exist at old location")
|
||||
})
|
||||
|
||||
t.Run("move file to nested path without ref", func(t *testing.T) {
|
||||
t.Run("move file without content change on branch should succeed", func(t *testing.T) {
|
||||
const targetPath = "moved/simple-move-branch.json"
|
||||
branchRef := "test-branch-move"
|
||||
|
||||
// Perform the move operation using helper function with ref parameter
|
||||
resp := helper.postFilesRequest(t, repo, filesPostOptions{
|
||||
targetPath: targetPath,
|
||||
originalPath: "all-panels.json",
|
||||
message: "move file without content change",
|
||||
ref: branchRef,
|
||||
})
|
||||
// nolint:errcheck
|
||||
defer resp.Body.Close()
|
||||
// Note: This might fail if branch doesn't exist, but the important thing is it doesn't return MethodNotAllowed
|
||||
if resp.StatusCode == http.StatusMethodNotAllowed {
|
||||
t.Fatal("should not return MethodNotAllowed for branch move")
|
||||
}
|
||||
|
||||
// If move succeeded (not MethodNotAllowed), verify the file moved in the repository
|
||||
if resp.StatusCode == http.StatusOK {
|
||||
movedObj, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved", "simple-move-branch.json")
|
||||
require.NoError(t, err, "moved file should exist in repository")
|
||||
|
||||
// Check the content is preserved (verify it's still the all-panels dashboard)
|
||||
resource, _, err := unstructured.NestedMap(movedObj.Object, "resource")
|
||||
require.NoError(t, err)
|
||||
dryRun, _, err := unstructured.NestedMap(resource, "dryRun")
|
||||
require.NoError(t, err)
|
||||
title, _, err := unstructured.NestedString(dryRun, "spec", "title")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "Panel tests - All panels", title, "content should be preserved")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("move file to nested path on configured branch should succeed", func(t *testing.T) {
|
||||
// Test a different scenario: Move a file that was never synced to Grafana
|
||||
// This might reveal the issue if dashboard creation fails during move
|
||||
const sourceFile = "never-synced.json"
|
||||
@@ -203,7 +230,7 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) {
|
||||
// DO NOT sync - move the file immediately without it ever being in Grafana
|
||||
const targetPath = "deep/nested/timeline.json"
|
||||
|
||||
// Perform the move operation without the file ever being synced to Grafana
|
||||
// Perform the move operation without the file ever being synced to Grafana (no ref = configured branch)
|
||||
resp := helper.postFilesRequest(t, repo, filesPostOptions{
|
||||
targetPath: targetPath,
|
||||
originalPath: sourceFile,
|
||||
@@ -211,70 +238,25 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) {
|
||||
})
|
||||
// nolint:errcheck
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "move operation should succeed")
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "move operation on configured branch should succeed")
|
||||
|
||||
// Check folders were created and validate hierarchy
|
||||
folderList, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{})
|
||||
require.NoError(t, err, "should be able to list folders")
|
||||
// File should exist at new location
|
||||
_, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "deep", "nested", "timeline.json")
|
||||
require.NoError(t, err, "file should exist at new nested location")
|
||||
|
||||
// Build a map of folder names to their objects for easier lookup
|
||||
folders := make(map[string]*unstructured.Unstructured)
|
||||
for _, folder := range folderList.Items {
|
||||
title, _, _ := unstructured.NestedString(folder.Object, "spec", "title")
|
||||
folders[title] = &folder
|
||||
parent, _, _ := unstructured.NestedString(folder.Object, "metadata", "annotations", "grafana.app/folder")
|
||||
t.Logf(" - %s: %s (parent: %s)", folder.GetName(), title, parent)
|
||||
}
|
||||
|
||||
// Validate expected folders exist with proper hierarchy
|
||||
// Expected structure: deep -> deep/nested
|
||||
deepFolderTitle := "deep"
|
||||
nestedFolderTitle := "nested"
|
||||
|
||||
// Validate "deep" folder exists and has no parent (is top-level)
|
||||
require.Contains(t, folders, deepFolderTitle, "deep folder should exist")
|
||||
f := folders[deepFolderTitle]
|
||||
deepFolderName := f.GetName()
|
||||
title, _, _ := unstructured.NestedString(f.Object, "spec", "title")
|
||||
require.Equal(t, deepFolderTitle, title, "deep folder should have correct title")
|
||||
parent, found, _ := unstructured.NestedString(f.Object, "metadata", "annotations", "grafana.app/folder")
|
||||
require.True(t, !found || parent == "", "deep folder should be top-level (no parent)")
|
||||
|
||||
// Validate "deep/nested" folder exists and has "deep" as parent
|
||||
require.Contains(t, folders, nestedFolderTitle, "nested folder should exist")
|
||||
f = folders[nestedFolderTitle]
|
||||
nestedFolderName := f.GetName()
|
||||
title, _, _ = unstructured.NestedString(f.Object, "spec", "title")
|
||||
require.Equal(t, nestedFolderTitle, title, "nested folder should have correct title")
|
||||
parent, _, _ = unstructured.NestedString(f.Object, "metadata", "annotations", "grafana.app/folder")
|
||||
require.Equal(t, deepFolderName, parent, "nested folder should have deep folder as parent")
|
||||
|
||||
// The key test: Check if dashboard was created in Grafana during move
|
||||
const timelineUID = "mIJjFy8Kz"
|
||||
dashboard, err := helper.DashboardsV1.Resource.Get(ctx, timelineUID, metav1.GetOptions{})
|
||||
require.NoError(t, err, "dashboard should exist in Grafana after moving never-synced file")
|
||||
dashboardFolder, _, _ := unstructured.NestedString(dashboard.Object, "metadata", "annotations", "grafana.app/folder")
|
||||
|
||||
// Validate dashboard is in the correct nested folder
|
||||
require.Equal(t, nestedFolderName, dashboardFolder, "dashboard should be in the nested folder")
|
||||
|
||||
// Verify the file moved in the repository
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "deep", "nested", "timeline.json")
|
||||
require.NoError(t, err, "moved file should exist in nested repository path")
|
||||
|
||||
// Verify the original file no longer exists in the repository
|
||||
// File should not exist at original location
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", sourceFile)
|
||||
require.Error(t, err, "original file should no longer exist in repository")
|
||||
require.Error(t, err, "file should not exist at original location after move")
|
||||
})
|
||||
|
||||
t.Run("move file with content update", func(t *testing.T) {
|
||||
const sourcePath = "moved/simple-move.json" // Use the file from previous test
|
||||
t.Run("move file with content update on configured branch should succeed", func(t *testing.T) {
|
||||
const sourcePath = "moved/simple-move.json" // Use the file we moved earlier
|
||||
const targetPath = "updated/content-updated.json"
|
||||
|
||||
// Use text-options.json content for the update
|
||||
updatedContent := helper.LoadFile("testdata/text-options.json")
|
||||
|
||||
// Perform move with content update using helper function
|
||||
// Perform move with content update using helper function (no ref = configured branch)
|
||||
resp := helper.postFilesRequest(t, repo, filesPostOptions{
|
||||
targetPath: targetPath,
|
||||
originalPath: sourcePath,
|
||||
@@ -283,51 +265,27 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) {
|
||||
})
|
||||
// nolint:errcheck
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "move with content update should succeed")
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "move with content update on configured branch should succeed")
|
||||
|
||||
// Verify the moved file has updated content (should now be text-options dashboard)
|
||||
// File should exist at new location with updated content
|
||||
movedObj, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "updated", "content-updated.json")
|
||||
require.NoError(t, err, "moved file should exist in repository")
|
||||
require.NoError(t, err, "file should exist at new location")
|
||||
|
||||
// Verify content was updated (should be text-options dashboard now)
|
||||
resource, _, err := unstructured.NestedMap(movedObj.Object, "resource")
|
||||
require.NoError(t, err)
|
||||
dryRun, _, err := unstructured.NestedMap(resource, "dryRun")
|
||||
require.NoError(t, err)
|
||||
title, _, err := unstructured.NestedString(dryRun, "spec", "title")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "Text options", title, "content should be updated to text-options dashboard")
|
||||
require.Equal(t, "Text options", title, "content should be updated")
|
||||
|
||||
// Check it has the expected UID from text-options.json
|
||||
name, _, err := unstructured.NestedString(dryRun, "metadata", "name")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "WZ7AhQiVz", name, "should have the UID from text-options.json")
|
||||
|
||||
// Verify source file no longer exists
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved", "simple-move.json")
|
||||
require.Error(t, err, "source file should no longer exist")
|
||||
|
||||
// Sync and verify the updated dashboard exists in Grafana
|
||||
helper.SyncAndWait(t, repo, nil)
|
||||
const textOptionsUID = "WZ7AhQiVz" // UID from text-options.json
|
||||
updatedDashboard, err := helper.DashboardsV1.Resource.Get(ctx, textOptionsUID, metav1.GetOptions{})
|
||||
require.NoError(t, err, "updated dashboard should exist in Grafana")
|
||||
|
||||
// Verify the original dashboard was deleted from Grafana
|
||||
_, err = helper.DashboardsV1.Resource.Get(ctx, allPanelsUID, metav1.GetOptions{})
|
||||
require.Error(t, err, "original dashboard should be deleted from Grafana")
|
||||
require.True(t, apierrors.IsNotFound(err))
|
||||
|
||||
// Verify the new dashboard has the updated content
|
||||
updatedTitle, _, err := unstructured.NestedString(updatedDashboard.Object, "spec", "title")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "Text options", updatedTitle)
|
||||
// Source file should not exist anymore
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", sourcePath)
|
||||
require.Error(t, err, "source file should not exist after move")
|
||||
})
|
||||
|
||||
t.Run("move directory", func(t *testing.T) {
|
||||
t.Skip("Skip as implementation is broken and leaves dashboards behind in the move")
|
||||
// FIXME: https://github.com/grafana/git-ui-sync-project/issues/379
|
||||
// The current implementation of moving directories is flawed.
|
||||
// It will be deprecated in favor of queuing a move job
|
||||
t.Run("move directory on configured branch should return MethodNotAllowed", func(t *testing.T) {
|
||||
// Create some files in a directory first using existing testdata files
|
||||
helper.CopyToProvisioningPath(t, "testdata/timeline-demo.json", "source-dir/timeline-demo.json")
|
||||
helper.CopyToProvisioningPath(t, "testdata/text-options.json", "source-dir/text-options.json")
|
||||
@@ -338,7 +296,7 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) {
|
||||
const sourceDir = "source-dir/"
|
||||
const targetDir = "moved-dir/"
|
||||
|
||||
// Move directory using helper function
|
||||
// Move directory using helper function (no ref = configured branch)
|
||||
resp := helper.postFilesRequest(t, repo, filesPostOptions{
|
||||
targetPath: targetDir,
|
||||
originalPath: sourceDir,
|
||||
@@ -346,20 +304,11 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) {
|
||||
})
|
||||
// nolint:errcheck
|
||||
defer resp.Body.Close()
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
require.NoError(t, err, "should read response body")
|
||||
t.Logf("Response Body: %s", string(body))
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode, "directory move should succeed")
|
||||
require.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode, "directory move on configured branch should return MethodNotAllowed")
|
||||
|
||||
// Verify source directory no longer exists
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "source-dir")
|
||||
require.Error(t, err, "source directory should no longer exist")
|
||||
|
||||
// Verify target directory and files exist
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved-dir", "timeline-demo.json")
|
||||
require.NoError(t, err, "moved timeline-demo.json should exist")
|
||||
_, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved-dir", "text-options.json")
|
||||
require.NoError(t, err, "moved text-options.json should exist")
|
||||
// Verify files in source directory still exist (operation was rejected)
|
||||
_, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "source-dir", "timeline-demo.json")
|
||||
require.NoError(t, err, "file in source directory should still exist after rejected move")
|
||||
})
|
||||
|
||||
t.Run("error cases", func(t *testing.T) {
|
||||
@@ -566,7 +515,7 @@ func TestIntegrationProvisioning_FilesOwnershipProtection(t *testing.T) {
|
||||
})
|
||||
|
||||
t.Run("DELETE resource owned by different repository - should fail", func(t *testing.T) {
|
||||
// Create a file manually in the second repo which is already in first one
|
||||
// Create a file manually in the second repo which has UID from first repo
|
||||
helper.CopyToProvisioningPath(t, "testdata/all-panels.json", "repo2/conflicting-delete.json")
|
||||
printFileTree(t, helper.ProvisioningPath)
|
||||
|
||||
@@ -590,10 +539,7 @@ func TestIntegrationProvisioning_FilesOwnershipProtection(t *testing.T) {
|
||||
}
|
||||
|
||||
// Verify it returns BadRequest (400) for ownership conflicts
|
||||
if !apierrors.IsBadRequest(err) {
|
||||
t.Errorf("Expected BadRequest error but got: %T - %v", err, err)
|
||||
return
|
||||
}
|
||||
require.True(t, apierrors.IsBadRequest(err), "Expected BadRequest error but got: %T - %v", err, err)
|
||||
|
||||
// Check error message contains ownership conflict information
|
||||
errorMsg := err.Error()
|
||||
@@ -607,7 +553,7 @@ func TestIntegrationProvisioning_FilesOwnershipProtection(t *testing.T) {
|
||||
targetPath: "moved-dashboard.json",
|
||||
originalPath: path.Join("dashboard2.json"),
|
||||
message: "attempt to move file from different repository",
|
||||
body: string(helper.LoadFile("testdata/all-panels.json")), // Content to move with the conflicting UID
|
||||
body: string(helper.LoadFile("testdata/all-panels.json")), // Content with the conflicting UID
|
||||
})
|
||||
// nolint:errcheck
|
||||
defer resp.Body.Close()
|
||||
|
||||
@@ -786,7 +786,7 @@ func TestIntegrationProvisioning_ImportAllPanelsFromLocalRepository(t *testing.T
|
||||
v, _, _ := unstructured.NestedString(obj.Object, "metadata", "annotations", utils.AnnoKeyUpdatedBy)
|
||||
require.Equal(t, "access-policy:provisioning", v)
|
||||
|
||||
// Should not be able to directly delete the managed resource
|
||||
// Should be able to directly delete the managed resource
|
||||
err = helper.DashboardsV1.Resource.Delete(ctx, allPanels, metav1.DeleteOptions{})
|
||||
require.NoError(t, err, "user can delete")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user