From c5ed2780abb8cf2c38ec67f1962876395ea96c9a Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Fri, 12 Sep 2025 14:02:13 -0600 Subject: [PATCH] Provisioning: Fix deletion order (#111043) --- .../provisioning/controller/finalizers.go | 36 ++++++++-- .../controller/finalizers_test.go | 66 +++++++++++++++++++ 2 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 pkg/registry/apis/provisioning/controller/finalizers_test.go diff --git a/pkg/registry/apis/provisioning/controller/finalizers.go b/pkg/registry/apis/provisioning/controller/finalizers.go index 3980db5ec6f..72c2209911c 100644 --- a/pkg/registry/apis/provisioning/controller/finalizers.go +++ b/pkg/registry/apis/provisioning/controller/finalizers.go @@ -160,14 +160,36 @@ func sortResourceListForDeletion(list *provisioning.ResourceList) { // Sort by the following logic: // - Put folders at the end so that we empty them first. // - Sort folders by depth so that we remove the deepest first + // - If the repo is created within a folder in grafana, make sure that folder is last. sort.Slice(list.Items, func(i, j int) bool { - switch { - case list.Items[i].Group != folders.RESOURCE: - return true - case list.Items[j].Group != folders.RESOURCE: - return false - default: - return len(strings.Split(list.Items[i].Path, "/")) > len(strings.Split(list.Items[j].Path, "/")) + isFolderI := list.Items[i].Group == folders.GroupVersion.Group + isFolderJ := list.Items[j].Group == folders.GroupVersion.Group + + // non-folders always go first in the order of deletion. + if isFolderI != isFolderJ { + return !isFolderI } + + // if both are not folders, keep order (doesn't matter) + if !isFolderI && !isFolderJ { + return false + } + + hasFolderI := list.Items[i].Folder != "" + hasFolderJ := list.Items[j].Folder != "" + // if one folder is in the root (i.e. does not have a folder specified), put that last + if hasFolderI != hasFolderJ { + return hasFolderI + } + + // if both are nested folder, sort by depth, with the deepest one being first + depthI := len(strings.Split(list.Items[i].Path, "/")) + depthJ := len(strings.Split(list.Items[j].Path, "/")) + if depthI != depthJ { + return depthI > depthJ + } + + // otherwise, keep order (doesn't matter) + return false }) } diff --git a/pkg/registry/apis/provisioning/controller/finalizers_test.go b/pkg/registry/apis/provisioning/controller/finalizers_test.go new file mode 100644 index 00000000000..a44e595afbf --- /dev/null +++ b/pkg/registry/apis/provisioning/controller/finalizers_test.go @@ -0,0 +1,66 @@ +package controller + +import ( + "testing" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/stretchr/testify/assert" +) + +func TestSortResourceListForDeletion(t *testing.T) { + testCases := []struct { + name string + input provisioning.ResourceList + expected provisioning.ResourceList + }{ + { + name: "Non-folder items first, folders sorted by depth", + input: provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + {Group: "dashboard.grafana.app", Path: "dashboard1.json"}, + {Group: "folder.grafana.app", Path: "folder1"}, + {Group: "folder.grafana.app", Path: "folder1/subfolder1/subfolder2", Folder: "subfolder1"}, + {Group: "dashboard.grafana.app", Path: "dashboard2.json"}, + {Group: "folder.grafana.app", Path: "folder2"}, + {Group: "folder.grafana.app", Path: "folder1/subfolder1", Folder: "folder1"}, + }, + }, + expected: provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + {Group: "dashboard.grafana.app", Path: "dashboard1.json"}, + {Group: "dashboard.grafana.app", Path: "dashboard2.json"}, + {Group: "folder.grafana.app", Path: "folder1/subfolder1/subfolder2", Folder: "subfolder1"}, + {Group: "folder.grafana.app", Path: "folder1/subfolder1", Folder: "folder1"}, + {Group: "folder.grafana.app", Path: "folder1"}, + {Group: "folder.grafana.app", Path: "folder2"}, + }, + }, + }, + { + name: "Folders without parent should be last", + input: provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + {Group: "folder.grafana.app", Path: "folder1"}, + {Group: "folder.grafana.app", Path: "folder2", Folder: "folder1"}, // if a repo is created with a folder in grafana (here folder1), the path will not have /, but the folder will be set + {Group: "folder.grafana.app", Path: "folder2/subfolder1", Folder: "folder2"}, + {Group: "folder.grafana.app", Path: "folder3", Folder: "folder1"}, + }, + }, + expected: provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + {Group: "folder.grafana.app", Path: "folder2/subfolder1", Folder: "folder2"}, + {Group: "folder.grafana.app", Path: "folder2", Folder: "folder1"}, + {Group: "folder.grafana.app", Path: "folder3", Folder: "folder1"}, + {Group: "folder.grafana.app", Path: "folder1"}, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + sortResourceListForDeletion(&tc.input) + assert.Equal(t, tc.expected, tc.input) + }) + } +}