From 727e82efc73f57981b6bcd916d27a913947dd4b3 Mon Sep 17 00:00:00 2001 From: Mariusz Mazur Date: Tue, 28 Jul 2026 13:21:09 +0200 Subject: [PATCH 1/3] feat(grafanactl): add scratch folder support for user-writable dashboards Add a "scratchFolders" config option that creates Grafana folders where Viewers get Edit permission, allowing them to create/save/delete dashboards. Dashboards are auto-deleted after a configurable maxAge (based on creation time). Empty subfolders are cleaned up recursively. Cleanup errors (dashboard deletion, folder deletion, permission updates) are logged as warnings and never fail the pipeline. Co-Authored-By: Claude Opus 4.6 (1M context) --- tools/grafanactl/config/config.go | 19 + tools/grafanactl/internal/grafana/client.go | 37 ++ tools/grafanactl/internal/grafana/scratch.go | 239 ++++++++ .../internal/grafana/scratch_test.go | 569 ++++++++++++++++++ tools/grafanactl/internal/grafana/syncer.go | 7 + 5 files changed, 871 insertions(+) create mode 100644 tools/grafanactl/internal/grafana/scratch.go create mode 100644 tools/grafanactl/internal/grafana/scratch_test.go diff --git a/tools/grafanactl/config/config.go b/tools/grafanactl/config/config.go index 851a6ec0..38f6f1a3 100644 --- a/tools/grafanactl/config/config.go +++ b/tools/grafanactl/config/config.go @@ -17,6 +17,7 @@ package config import ( "fmt" "os" + "time" "sigs.k8s.io/yaml" ) @@ -30,6 +31,7 @@ type ObservabilityConfig struct { type GrafanaDashboardsConfig struct { AzureManagedFolders []string `json:"azureManagedFolders"` DashboardFolders []DashboardFolder `json:"dashboardFolders"` + ScratchFolders []ScratchFolder `json:"scratchFolders,omitempty"` } // DashboardFolder represents a folder containing dashboards to sync @@ -38,6 +40,23 @@ type DashboardFolder struct { Path string `json:"path"` } +// ScratchFolder represents a user-writable folder where dashboards are auto-deleted after MaxAge. +type ScratchFolder struct { + Name string `json:"name"` + MaxAgeRaw string `json:"maxAge"` +} + +func (f ScratchFolder) MaxAge() (time.Duration, error) { + d, err := time.ParseDuration(f.MaxAgeRaw) + if err != nil { + return 0, fmt.Errorf("invalid maxAge %q for scratch folder %q: %w", f.MaxAgeRaw, f.Name, err) + } + if d <= 0 { + return 0, fmt.Errorf("maxAge for scratch folder %q must be positive, got %s", f.Name, d) + } + return d, nil +} + // LoadFromFile reads and parses the observability config from a file func LoadFromFile(path string) (*ObservabilityConfig, error) { data, err := os.ReadFile(path) diff --git a/tools/grafanactl/internal/grafana/client.go b/tools/grafanactl/internal/grafana/client.go index 417d23f5..093280c2 100644 --- a/tools/grafanactl/internal/grafana/client.go +++ b/tools/grafanactl/internal/grafana/client.go @@ -173,3 +173,40 @@ func (c *Client) DeleteDashboardByUID(ctx context.Context, uid string) error { return nil } + +// GetFolderPermissions returns the permission list for a folder. +func (c *Client) GetFolderPermissions(ctx context.Context, folderUID string) ([]sdk.FolderPermission, error) { + perms, err := c.grafanaClient.GetFolderPermissions(ctx, folderUID) + if err != nil { + return nil, fmt.Errorf("failed to get permissions for folder %q: %w", folderUID, err) + } + return perms, nil +} + +// UpdateFolderPermissions replaces the full permission list for a folder. +func (c *Client) UpdateFolderPermissions(ctx context.Context, folderUID string, permissions ...sdk.FolderPermission) error { + _, err := c.grafanaClient.UpdateFolderPermissions(ctx, folderUID, permissions...) + if err != nil { + return fmt.Errorf("failed to update permissions for folder %q: %w", folderUID, err) + } + return nil +} + +// SearchFolders returns all folders visible in the Grafana instance via the search API. +// Unlike ListFolders, search results include FolderUID which identifies parent folders. +func (c *Client) SearchFolders(ctx context.Context) ([]sdk.FoundBoard, error) { + results, err := c.grafanaClient.Search(ctx, sdk.SearchType(sdk.SearchTypeFolder)) + if err != nil { + return nil, fmt.Errorf("failed to search folders: %w", err) + } + return results, nil +} + +// DeleteFolderByUID removes a folder by its UID. +func (c *Client) DeleteFolderByUID(ctx context.Context, uid string) error { + _, err := c.grafanaClient.DeleteFolderByUID(ctx, uid) + if err != nil { + return fmt.Errorf("failed to delete folder %q: %w", uid, err) + } + return nil +} diff --git a/tools/grafanactl/internal/grafana/scratch.go b/tools/grafanactl/internal/grafana/scratch.go new file mode 100644 index 00000000..f924b17a --- /dev/null +++ b/tools/grafanactl/internal/grafana/scratch.go @@ -0,0 +1,239 @@ +// Copyright 2025 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grafana + +import ( + "context" + "fmt" + "time" + + "github.com/go-logr/logr" + "github.com/grafana-tools/sdk" + + "github.com/Azure/ARO-Tools/tools/grafanactl/config" +) + +type scratchGrafanaClient interface { + ListFolders(ctx context.Context) ([]sdk.Folder, error) + CreateFolder(ctx context.Context, title string) (sdk.Folder, error) + UpdateFolderPermissions(ctx context.Context, folderUID string, permissions ...sdk.FolderPermission) error + ListDashboards(ctx context.Context) ([]sdk.FoundBoard, error) + GetDashboardByUID(ctx context.Context, uid string) (sdk.Board, sdk.BoardProperties, error) + DeleteDashboardByUID(ctx context.Context, uid string) error + SearchFolders(ctx context.Context) ([]sdk.FoundBoard, error) + DeleteFolderByUID(ctx context.Context, uid string) error +} + +var scratchFolderPermissions = []sdk.FolderPermission{ + {Role: "Viewer", Permission: sdk.PermissionEdit}, + {Role: "Editor", Permission: sdk.PermissionEdit}, + {Role: "Admin", Permission: sdk.PermissionAdmin}, +} + +func (s *DashboardSyncer) syncScratchFolders(ctx context.Context) error { + if len(s.config.GrafanaDashboards.ScratchFolders) == 0 { + return nil + } + return syncScratchFolders(ctx, s.client, s.config.GrafanaDashboards.ScratchFolders, s.dryRun, s.now) +} + +func syncScratchFolders(ctx context.Context, client scratchGrafanaClient, folders []config.ScratchFolder, dryRun bool, now func() time.Time) error { + logger := logr.FromContextOrDiscard(ctx) + + existingFolders, err := client.ListFolders(ctx) + if err != nil { + return fmt.Errorf("failed to list folders for scratch sync: %w", err) + } + + allDashboards, err := client.ListDashboards(ctx) + if err != nil { + return fmt.Errorf("failed to list dashboards for scratch sync: %w", err) + } + + allSearchFolders, err := client.SearchFolders(ctx) + if err != nil { + return fmt.Errorf("failed to search folders for scratch sync: %w", err) + } + + for _, sf := range folders { + maxAge, err := sf.MaxAge() + if err != nil { + return err + } + + if err := syncOneScratchFolder(ctx, client, sf.Name, maxAge, existingFolders, allDashboards, allSearchFolders, dryRun, now); err != nil { + return fmt.Errorf("failed to sync scratch folder %q: %w", sf.Name, err) + } + logger.Info("Synced scratch folder", "name", sf.Name, "maxAge", maxAge) + } + + return nil +} + +// collectScratchFolderUIDs returns a set of UIDs that belong to the scratch folder tree: +// the root folder itself plus all nested subfolders (recursively). +func collectScratchFolderUIDs(rootUID string, allSearchFolders []sdk.FoundBoard) map[string]bool { + uids := map[string]bool{rootUID: true} + changed := true + for changed { + changed = false + for _, f := range allSearchFolders { + if f.Type != "dash-folder" { + continue + } + if uids[f.FolderUID] && !uids[f.UID] { + uids[f.UID] = true + changed = true + } + } + } + return uids +} + +func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name string, maxAge time.Duration, existingFolders []sdk.Folder, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, dryRun bool, now func() time.Time) error { + logger := logr.FromContextOrDiscard(ctx) + + folder, err := findOrCreateFolder(ctx, client, name, existingFolders, dryRun) + if err != nil { + return err + } + + if dryRun { + logger.Info("DRY_RUN: Would set permissions on scratch folder", "name", name) + } else { + if err := client.UpdateFolderPermissions(ctx, folder.UID, scratchFolderPermissions...); err != nil { + return fmt.Errorf("failed to set permissions on folder %q: %w", name, err) + } + logger.Info("Set permissions on scratch folder", "name", name) + } + + scratchUIDs := collectScratchFolderUIDs(folder.UID, allSearchFolders) + + cutoff := now().Add(-maxAge) + for _, db := range allDashboards { + if !scratchUIDs[db.FolderUID] { + continue + } + + _, props, err := client.GetDashboardByUID(ctx, db.UID) + if err != nil { + logger.Error(err, "Failed to get metadata for scratch dashboard, skipping", "title", db.Title, "uid", db.UID) + continue + } + + if !props.Created.Before(cutoff) { + logger.V(1).Info("Scratch dashboard not expired", "title", db.Title, "uid", db.UID, "created", props.Created, "cutoff", cutoff) + continue + } + + if dryRun { + logger.Info("DRY_RUN: Would delete expired scratch dashboard", "title", db.Title, "uid", db.UID, "created", props.Created) + } else { + logger.Info("Deleting expired scratch dashboard", "title", db.Title, "uid", db.UID, "created", props.Created) + if err := client.DeleteDashboardByUID(ctx, db.UID); err != nil { + logger.Error(err, "Failed to delete expired scratch dashboard, continuing", "title", db.Title, "uid", db.UID) + } + } + } + + deleteEmptySubfolders(ctx, client, folder.UID, allDashboards, allSearchFolders, scratchUIDs, dryRun) + + return nil +} + +// deleteEmptySubfolders removes subfolders of the scratch folder that contain no +// dashboards (after expiry deletion). Processes leaf-first so nested empty trees +// are fully removed. +func deleteEmptySubfolders(ctx context.Context, client scratchGrafanaClient, rootUID string, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, scratchUIDs map[string]bool, dryRun bool) { + logger := logr.FromContextOrDiscard(ctx) + + // Build parent→children map for subfolders only (exclude root). + children := make(map[string][]sdk.FoundBoard) + for _, f := range allSearchFolders { + if f.Type != "dash-folder" || !scratchUIDs[f.UID] || f.UID == rootUID { + continue + } + children[f.FolderUID] = append(children[f.FolderUID], f) + } + + // Count remaining dashboards per folder. We only care about folders in the + // scratch tree. A dashboard is "remaining" if it wasn't deleted above — since + // we don't mutate allDashboards, we re-check membership; deleted dashboards + // are gone from Grafana but still in our snapshot. That's fine: the worst + // case is we leave a folder for one more sync cycle. + dashCount := make(map[string]int) + for _, db := range allDashboards { + if scratchUIDs[db.FolderUID] { + dashCount[db.FolderUID]++ + } + } + + // Recursively delete leaf-first, starting from direct children of root. + for _, child := range children[rootUID] { + deleteEmptyRecursive(ctx, client, child.UID, children, dashCount, dryRun, logger) + } +} + +// deleteEmptyRecursive walks the subfolder tree depth-first and deletes folders +// that are empty (no dashboards and no remaining children after recursion). +// Returns true if the folder at uid was deleted (or would be in dry-run). +func deleteEmptyRecursive(ctx context.Context, client scratchGrafanaClient, uid string, children map[string][]sdk.FoundBoard, dashCount map[string]int, dryRun bool, logger logr.Logger) bool { + hasChildren := false + for _, child := range children[uid] { + if !deleteEmptyRecursive(ctx, client, child.UID, children, dashCount, dryRun, logger) { + hasChildren = true + } + } + + if hasChildren || dashCount[uid] > 0 { + return false + } + + if dryRun { + logger.Info("DRY_RUN: Would delete empty scratch subfolder", "uid", uid) + return true + } + + logger.Info("Deleting empty scratch subfolder", "uid", uid) + if err := client.DeleteFolderByUID(ctx, uid); err != nil { + logger.Error(err, "Failed to delete empty scratch subfolder, continuing", "uid", uid) + return false + } + return true +} + +func findOrCreateFolder(ctx context.Context, client scratchGrafanaClient, name string, existingFolders []sdk.Folder, dryRun bool) (sdk.Folder, error) { + logger := logr.FromContextOrDiscard(ctx) + + for _, f := range existingFolders { + if f.Title == name { + logger.V(1).Info("Scratch folder already exists", "name", name, "uid", f.UID) + return f, nil + } + } + + if dryRun { + logger.Info("DRY_RUN: Would create scratch folder", "name", name) + return sdk.Folder{Title: name, UID: "dry-run-" + name}, nil + } + + folder, err := client.CreateFolder(ctx, name) + if err != nil { + return sdk.Folder{}, fmt.Errorf("failed to create scratch folder %q: %w", name, err) + } + + logger.Info("Created scratch folder", "name", name, "uid", folder.UID) + return folder, nil +} diff --git a/tools/grafanactl/internal/grafana/scratch_test.go b/tools/grafanactl/internal/grafana/scratch_test.go new file mode 100644 index 00000000..33adaf2e --- /dev/null +++ b/tools/grafanactl/internal/grafana/scratch_test.go @@ -0,0 +1,569 @@ +// Copyright 2025 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grafana + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/grafana-tools/sdk" + + "github.com/Azure/ARO-Tools/tools/grafanactl/config" +) + +type mockScratchClient struct { + folders []sdk.Folder + dashboards []sdk.FoundBoard + searchFolders []sdk.FoundBoard + dashboardsByUID map[string]sdk.BoardProperties + createdFolders []string + updatedPermissions map[string][]sdk.FolderPermission + deletedDashboardUIDs []string + deletedFolderUIDs []string + + listFoldersErr error + createFolderErr error + updatePermsErr error + listDashboardsErr error + searchFoldersErr error + getDashboardErr error + deleteDashboardErr error + deleteFolderErr error +} + +func newMockScratchClient() *mockScratchClient { + return &mockScratchClient{ + dashboardsByUID: make(map[string]sdk.BoardProperties), + updatedPermissions: make(map[string][]sdk.FolderPermission), + } +} + +func (m *mockScratchClient) ListFolders(_ context.Context) ([]sdk.Folder, error) { + if m.listFoldersErr != nil { + return nil, m.listFoldersErr + } + return m.folders, nil +} + +func (m *mockScratchClient) CreateFolder(_ context.Context, title string) (sdk.Folder, error) { + if m.createFolderErr != nil { + return sdk.Folder{}, m.createFolderErr + } + m.createdFolders = append(m.createdFolders, title) + f := sdk.Folder{Title: title, UID: "uid-" + title, ID: len(m.folders) + 1} + m.folders = append(m.folders, f) + return f, nil +} + +func (m *mockScratchClient) UpdateFolderPermissions(_ context.Context, folderUID string, permissions ...sdk.FolderPermission) error { + if m.updatePermsErr != nil { + return m.updatePermsErr + } + m.updatedPermissions[folderUID] = permissions + return nil +} + +func (m *mockScratchClient) ListDashboards(_ context.Context) ([]sdk.FoundBoard, error) { + if m.listDashboardsErr != nil { + return nil, m.listDashboardsErr + } + return m.dashboards, nil +} + +func (m *mockScratchClient) SearchFolders(_ context.Context) ([]sdk.FoundBoard, error) { + if m.searchFoldersErr != nil { + return nil, m.searchFoldersErr + } + return m.searchFolders, nil +} + +func (m *mockScratchClient) GetDashboardByUID(_ context.Context, uid string) (sdk.Board, sdk.BoardProperties, error) { + if m.getDashboardErr != nil { + return sdk.Board{}, sdk.BoardProperties{}, m.getDashboardErr + } + props, ok := m.dashboardsByUID[uid] + if !ok { + return sdk.Board{}, sdk.BoardProperties{}, fmt.Errorf("dashboard %q not found", uid) + } + return sdk.Board{UID: uid}, props, nil +} + +func (m *mockScratchClient) DeleteDashboardByUID(_ context.Context, uid string) error { + if m.deleteDashboardErr != nil { + return m.deleteDashboardErr + } + m.deletedDashboardUIDs = append(m.deletedDashboardUIDs, uid) + return nil +} + +func (m *mockScratchClient) DeleteFolderByUID(_ context.Context, uid string) error { + if m.deleteFolderErr != nil { + return m.deleteFolderErr + } + m.deletedFolderUIDs = append(m.deletedFolderUIDs, uid) + return nil +} + +func TestSyncScratchFolders_CreatesFolder(t *testing.T) { + client := newMockScratchClient() + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.createdFolders) != 1 || client.createdFolders[0] != "Scratchpad" { + t.Fatalf("expected folder 'Scratchpad' to be created, got %v", client.createdFolders) + } +} + +func TestSyncScratchFolders_ReuseExistingFolder(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "existing-uid", ID: 42}} + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.createdFolders) != 0 { + t.Fatalf("expected no folder creation, got %v", client.createdFolders) + } + + perms, ok := client.updatedPermissions["existing-uid"] + if !ok { + t.Fatal("expected permissions to be set on existing-uid") + } + if len(perms) != 3 { + t.Fatalf("expected 3 permission entries, got %d", len(perms)) + } +} + +func TestSyncScratchFolders_SetsPermissions(t *testing.T) { + client := newMockScratchClient() + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + folderUID := "uid-Scratchpad" + perms := client.updatedPermissions[folderUID] + if len(perms) != 3 { + t.Fatalf("expected 3 permission entries, got %d", len(perms)) + } + + expected := map[string]sdk.PermissionType{ + "Viewer": sdk.PermissionEdit, + "Editor": sdk.PermissionEdit, + "Admin": sdk.PermissionAdmin, + } + for _, p := range perms { + want, ok := expected[p.Role] + if !ok { + t.Errorf("unexpected role %q in permissions", p.Role) + continue + } + if p.Permission != want { + t.Errorf("role %q: got permission %d, want %d", p.Role, p.Permission, want) + } + } +} + +func TestSyncScratchFolders_DeletesExpiredDashboards(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.dashboards = []sdk.FoundBoard{ + {UID: "old-dash", Title: "Old Dashboard", FolderUID: "scratch-uid"}, + {UID: "new-dash", Title: "New Dashboard", FolderUID: "scratch-uid"}, + {UID: "other-dash", Title: "Other Dashboard", FolderUID: "other-folder"}, + } + client.dashboardsByUID["old-dash"] = sdk.BoardProperties{ + Created: now.Add(-8 * 24 * time.Hour), + } + client.dashboardsByUID["new-dash"] = sdk.BoardProperties{ + Created: now.Add(-1 * 24 * time.Hour), + } + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.deletedDashboardUIDs) != 1 { + t.Fatalf("expected 1 deletion, got %d: %v", len(client.deletedDashboardUIDs), client.deletedDashboardUIDs) + } + if client.deletedDashboardUIDs[0] != "old-dash" { + t.Fatalf("expected 'old-dash' to be deleted, got %q", client.deletedDashboardUIDs[0]) + } +} + +func TestSyncScratchFolders_DeletesExpiredDashboardsInSubfolders(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.searchFolders = []sdk.FoundBoard{ + {UID: "scratch-uid", Title: "Scratchpad", Type: "dash-folder"}, + {UID: "sub-1", Title: "subfolder1", Type: "dash-folder", FolderUID: "scratch-uid"}, + } + client.dashboards = []sdk.FoundBoard{ + {UID: "root-dash", Title: "Root Dashboard", FolderUID: "scratch-uid"}, + {UID: "sub-dash", Title: "Sub Dashboard", FolderUID: "sub-1"}, + } + client.dashboardsByUID["root-dash"] = sdk.BoardProperties{Created: now.Add(-8 * 24 * time.Hour)} + client.dashboardsByUID["sub-dash"] = sdk.BoardProperties{Created: now.Add(-8 * 24 * time.Hour)} + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.deletedDashboardUIDs) != 2 { + t.Fatalf("expected 2 deletions, got %d: %v", len(client.deletedDashboardUIDs), client.deletedDashboardUIDs) + } +} + +func TestSyncScratchFolders_DeletesEmptySubfolders(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.searchFolders = []sdk.FoundBoard{ + {UID: "scratch-uid", Title: "Scratchpad", Type: "dash-folder"}, + {UID: "empty-sub", Title: "empty", Type: "dash-folder", FolderUID: "scratch-uid"}, + } + // No dashboards anywhere + client.dashboards = nil + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.deletedFolderUIDs) != 1 || client.deletedFolderUIDs[0] != "empty-sub" { + t.Fatalf("expected empty-sub to be deleted, got %v", client.deletedFolderUIDs) + } +} + +func TestSyncScratchFolders_DeletesNestedEmptySubfoldersLeafFirst(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.searchFolders = []sdk.FoundBoard{ + {UID: "scratch-uid", Title: "Scratchpad", Type: "dash-folder"}, + {UID: "parent-sub", Title: "parent", Type: "dash-folder", FolderUID: "scratch-uid"}, + {UID: "child-sub", Title: "child", Type: "dash-folder", FolderUID: "parent-sub"}, + } + client.dashboards = nil + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.deletedFolderUIDs) != 2 { + t.Fatalf("expected 2 folder deletions, got %d: %v", len(client.deletedFolderUIDs), client.deletedFolderUIDs) + } + // Leaf first + if client.deletedFolderUIDs[0] != "child-sub" { + t.Fatalf("expected child-sub deleted first, got %q", client.deletedFolderUIDs[0]) + } + if client.deletedFolderUIDs[1] != "parent-sub" { + t.Fatalf("expected parent-sub deleted second, got %q", client.deletedFolderUIDs[1]) + } +} + +func TestSyncScratchFolders_KeepsNonEmptySubfolders(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.searchFolders = []sdk.FoundBoard{ + {UID: "scratch-uid", Title: "Scratchpad", Type: "dash-folder"}, + {UID: "has-dash-sub", Title: "hasdash", Type: "dash-folder", FolderUID: "scratch-uid"}, + } + client.dashboards = []sdk.FoundBoard{ + {UID: "fresh-dash", Title: "Fresh Dashboard", FolderUID: "has-dash-sub"}, + } + client.dashboardsByUID["fresh-dash"] = sdk.BoardProperties{ + Created: now.Add(-1 * time.Hour), // not expired + } + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.deletedFolderUIDs) != 0 { + t.Fatalf("expected no folder deletions, got %v", client.deletedFolderUIDs) + } +} + +func TestSyncScratchFolders_DoesNotDeleteRootScratchFolder(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.searchFolders = []sdk.FoundBoard{ + {UID: "scratch-uid", Title: "Scratchpad", Type: "dash-folder"}, + } + client.dashboards = nil + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + for _, uid := range client.deletedFolderUIDs { + if uid == "scratch-uid" { + t.Fatal("root scratch folder should not be deleted") + } + } +} + +func TestSyncScratchFolders_ExpiryBoundary(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + maxAge := 168 * time.Hour + + client.dashboards = []sdk.FoundBoard{ + {UID: "exactly-at-boundary", Title: "Boundary", FolderUID: "scratch-uid"}, + {UID: "one-second-before", Title: "Just Expired", FolderUID: "scratch-uid"}, + {UID: "one-second-after", Title: "Not Expired", FolderUID: "scratch-uid"}, + } + client.dashboardsByUID["exactly-at-boundary"] = sdk.BoardProperties{ + Created: now.Add(-maxAge), + } + client.dashboardsByUID["one-second-before"] = sdk.BoardProperties{ + Created: now.Add(-maxAge - time.Second), + } + client.dashboardsByUID["one-second-after"] = sdk.BoardProperties{ + Created: now.Add(-maxAge + time.Second), + } + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.deletedDashboardUIDs) != 1 { + t.Fatalf("expected 1 deletion, got %d: %v", len(client.deletedDashboardUIDs), client.deletedDashboardUIDs) + } + if client.deletedDashboardUIDs[0] != "one-second-before" { + t.Fatalf("expected 'one-second-before' to be deleted, got %q", client.deletedDashboardUIDs[0]) + } +} + +func TestSyncScratchFolders_DryRun(t *testing.T) { + client := newMockScratchClient() + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.searchFolders = []sdk.FoundBoard{ + {UID: "dry-run-Scratchpad", Title: "Scratchpad", Type: "dash-folder"}, + {UID: "empty-sub", Title: "empty", Type: "dash-folder", FolderUID: "dry-run-Scratchpad"}, + } + client.dashboards = []sdk.FoundBoard{ + {UID: "old-dash", Title: "Old Dashboard", FolderUID: "dry-run-Scratchpad"}, + } + client.dashboardsByUID["old-dash"] = sdk.BoardProperties{ + Created: now.Add(-8 * 24 * time.Hour), + } + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, true, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.createdFolders) != 0 { + t.Fatalf("expected no folder creation in dry-run, got %v", client.createdFolders) + } + if len(client.updatedPermissions) != 0 { + t.Fatalf("expected no permission updates in dry-run, got %v", client.updatedPermissions) + } + if len(client.deletedDashboardUIDs) != 0 { + t.Fatalf("expected no dashboard deletions in dry-run, got %v", client.deletedDashboardUIDs) + } + if len(client.deletedFolderUIDs) != 0 { + t.Fatalf("expected no folder deletions in dry-run, got %v", client.deletedFolderUIDs) + } +} + +func TestSyncScratchFolders_IgnoresDashboardsInOtherFolders(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.dashboards = []sdk.FoundBoard{ + {UID: "other-dash", Title: "Other Dashboard", FolderUID: "other-folder"}, + } + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(client.deletedDashboardUIDs) != 0 { + t.Fatalf("expected no deletions, got %v", client.deletedDashboardUIDs) + } +} + +func TestSyncScratchFolders_DashboardDeleteErrorIsNonFatal(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.dashboards = []sdk.FoundBoard{ + {UID: "old-dash", Title: "Old Dashboard", FolderUID: "scratch-uid"}, + } + client.dashboardsByUID["old-dash"] = sdk.BoardProperties{ + Created: now.Add(-8 * 24 * time.Hour), + } + client.deleteDashboardErr = fmt.Errorf("delete failed") + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("expected no error (non-fatal), got %v", err) + } +} + +func TestSyncScratchFolders_MetadataErrorIsNonFatal(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + client.dashboards = []sdk.FoundBoard{ + {UID: "bad-dash", Title: "Bad Dashboard", FolderUID: "scratch-uid"}, + } + client.getDashboardErr = fmt.Errorf("API error") + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("expected no error (non-fatal), got %v", err) + } +} + +func TestSyncScratchFolders_FolderDeleteErrorIsNonFatal(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + client.searchFolders = []sdk.FoundBoard{ + {UID: "scratch-uid", Title: "Scratchpad", Type: "dash-folder"}, + {UID: "empty-sub", Title: "empty", Type: "dash-folder", FolderUID: "scratch-uid"}, + } + client.dashboards = nil + client.deleteFolderErr = fmt.Errorf("folder delete failed") + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("expected no error (non-fatal), got %v", err) + } +} + +func TestSyncScratchFolders_PermissionErrorIsFatal(t *testing.T) { + client := newMockScratchClient() + client.folders = []sdk.Folder{{Title: "Scratchpad", UID: "scratch-uid"}} + client.updatePermsErr = fmt.Errorf("forbidden") + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err == nil { + t.Fatal("expected error on permission update failure, got nil") + } +} + +func TestSyncScratchFolders_ErrorOnInvalidMaxAge(t *testing.T) { + client := newMockScratchClient() + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "not-a-duration"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err == nil { + t.Fatal("expected error for invalid maxAge, got nil") + } +} + +func TestSyncScratchFolders_CreateFolderErrorIsFatal(t *testing.T) { + client := newMockScratchClient() + client.createFolderErr = fmt.Errorf("permission denied") + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} + err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + if err == nil { + t.Fatal("expected error on folder creation failure, got nil") + } +} + +func TestSyncScratchFolders_NoScratchFolders(t *testing.T) { + client := newMockScratchClient() + now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) + + err := syncScratchFolders(context.Background(), client, nil, false, func() time.Time { return now }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestCollectScratchFolderUIDs(t *testing.T) { + allFolders := []sdk.FoundBoard{ + {UID: "root", Title: "Root", Type: "dash-folder"}, + {UID: "child-1", Title: "Child 1", Type: "dash-folder", FolderUID: "root"}, + {UID: "child-2", Title: "Child 2", Type: "dash-folder", FolderUID: "root"}, + {UID: "grandchild", Title: "Grandchild", Type: "dash-folder", FolderUID: "child-1"}, + {UID: "unrelated", Title: "Unrelated", Type: "dash-folder", FolderUID: "other-root"}, + } + + uids := collectScratchFolderUIDs("root", allFolders) + + expected := map[string]bool{"root": true, "child-1": true, "child-2": true, "grandchild": true} + if len(uids) != len(expected) { + t.Fatalf("expected %d UIDs, got %d: %v", len(expected), len(uids), uids) + } + for uid := range expected { + if !uids[uid] { + t.Errorf("expected UID %q in set", uid) + } + } + if uids["unrelated"] { + t.Error("unrelated folder should not be in set") + } +} diff --git a/tools/grafanactl/internal/grafana/syncer.go b/tools/grafanactl/internal/grafana/syncer.go index 71e4285c..2b2ea92e 100644 --- a/tools/grafanactl/internal/grafana/syncer.go +++ b/tools/grafanactl/internal/grafana/syncer.go @@ -21,6 +21,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/go-logr/logr" "github.com/grafana-tools/sdk" @@ -34,6 +35,7 @@ type DashboardSyncer struct { config *config.ObservabilityConfig configDir string dryRun bool + now func() time.Time } // ValidationIssue represents a validation error or warning for a dashboard. @@ -65,6 +67,7 @@ func NewDashboardSyncer(client *Client, cfg *config.ObservabilityConfig, configF config: cfg, configDir: filepath.Dir(configFilePath), dryRun: dryRun, + now: time.Now, } } @@ -97,6 +100,10 @@ func (s *DashboardSyncer) Sync(ctx context.Context) error { return fmt.Errorf("failed to delete stale dashboards: %w", err) } + if err := s.syncScratchFolders(ctx); err != nil { + return fmt.Errorf("failed to sync scratch folders: %w", err) + } + // Report validation issues reportValidationIssues(ctx, validationErrors, validationWarnings) From b7ed9f4902a5924f8e1632fa86e251ce1ec3a6ca Mon Sep 17 00:00:00 2001 From: Mariusz Mazur Date: Tue, 28 Jul 2026 16:44:02 +0200 Subject: [PATCH 2/3] fix(grafanactl): delete empty scratch subfolders in the same sync pass Track which dashboards were deleted during expiry and exclude them from the folder dashboard count, so subfolders emptied by expiry are cleaned up immediately rather than requiring a second sync cycle. Co-Authored-By: Claude Opus 4.6 --- tools/grafanactl/internal/grafana/scratch.go | 15 +++++++-------- tools/grafanactl/internal/grafana/scratch_test.go | 6 +++++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/tools/grafanactl/internal/grafana/scratch.go b/tools/grafanactl/internal/grafana/scratch.go index f924b17a..6737dc3b 100644 --- a/tools/grafanactl/internal/grafana/scratch.go +++ b/tools/grafanactl/internal/grafana/scratch.go @@ -121,6 +121,7 @@ func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name scratchUIDs := collectScratchFolderUIDs(folder.UID, allSearchFolders) + deletedDashboards := make(map[string]bool) cutoff := now().Add(-maxAge) for _, db := range allDashboards { if !scratchUIDs[db.FolderUID] { @@ -140,15 +141,18 @@ func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name if dryRun { logger.Info("DRY_RUN: Would delete expired scratch dashboard", "title", db.Title, "uid", db.UID, "created", props.Created) + deletedDashboards[db.UID] = true } else { logger.Info("Deleting expired scratch dashboard", "title", db.Title, "uid", db.UID, "created", props.Created) if err := client.DeleteDashboardByUID(ctx, db.UID); err != nil { logger.Error(err, "Failed to delete expired scratch dashboard, continuing", "title", db.Title, "uid", db.UID) + } else { + deletedDashboards[db.UID] = true } } } - deleteEmptySubfolders(ctx, client, folder.UID, allDashboards, allSearchFolders, scratchUIDs, dryRun) + deleteEmptySubfolders(ctx, client, folder.UID, allDashboards, allSearchFolders, scratchUIDs, deletedDashboards, dryRun) return nil } @@ -156,7 +160,7 @@ func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name // deleteEmptySubfolders removes subfolders of the scratch folder that contain no // dashboards (after expiry deletion). Processes leaf-first so nested empty trees // are fully removed. -func deleteEmptySubfolders(ctx context.Context, client scratchGrafanaClient, rootUID string, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, scratchUIDs map[string]bool, dryRun bool) { +func deleteEmptySubfolders(ctx context.Context, client scratchGrafanaClient, rootUID string, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, scratchUIDs map[string]bool, deletedDashboards map[string]bool, dryRun bool) { logger := logr.FromContextOrDiscard(ctx) // Build parent→children map for subfolders only (exclude root). @@ -168,14 +172,9 @@ func deleteEmptySubfolders(ctx context.Context, client scratchGrafanaClient, roo children[f.FolderUID] = append(children[f.FolderUID], f) } - // Count remaining dashboards per folder. We only care about folders in the - // scratch tree. A dashboard is "remaining" if it wasn't deleted above — since - // we don't mutate allDashboards, we re-check membership; deleted dashboards - // are gone from Grafana but still in our snapshot. That's fine: the worst - // case is we leave a folder for one more sync cycle. dashCount := make(map[string]int) for _, db := range allDashboards { - if scratchUIDs[db.FolderUID] { + if scratchUIDs[db.FolderUID] && !deletedDashboards[db.UID] { dashCount[db.FolderUID]++ } } diff --git a/tools/grafanactl/internal/grafana/scratch_test.go b/tools/grafanactl/internal/grafana/scratch_test.go index 33adaf2e..d5800259 100644 --- a/tools/grafanactl/internal/grafana/scratch_test.go +++ b/tools/grafanactl/internal/grafana/scratch_test.go @@ -244,7 +244,11 @@ func TestSyncScratchFolders_DeletesExpiredDashboardsInSubfolders(t *testing.T) { } if len(client.deletedDashboardUIDs) != 2 { - t.Fatalf("expected 2 deletions, got %d: %v", len(client.deletedDashboardUIDs), client.deletedDashboardUIDs) + t.Fatalf("expected 2 dashboard deletions, got %d: %v", len(client.deletedDashboardUIDs), client.deletedDashboardUIDs) + } + + if len(client.deletedFolderUIDs) != 1 || client.deletedFolderUIDs[0] != "sub-1" { + t.Fatalf("expected sub-1 folder to be deleted in the same sync pass, got %v", client.deletedFolderUIDs) } } From b118afb642427f53c06c236891e9934638f7dd3c Mon Sep 17 00:00:00 2001 From: Mariusz Mazur Date: Wed, 29 Jul 2026 11:21:56 +0200 Subject: [PATCH 3/3] refactor(grafanactl): address review nits on scratch folder code - Use sets.Set[string] instead of map[string]bool for deleted dashboards - Pass time.Time value instead of func() time.Time to helper functions - Pull logger from context instead of passing it as a parameter, use WithValues to attach uid as a structured field Co-Authored-By: Claude Opus 4.6 --- tools/grafanactl/go.mod | 2 +- tools/grafanactl/internal/grafana/scratch.go | 36 +++++++++--------- .../internal/grafana/scratch_test.go | 38 +++++++++---------- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/tools/grafanactl/go.mod b/tools/grafanactl/go.mod index f8c26e69..e5837a5c 100644 --- a/tools/grafanactl/go.mod +++ b/tools/grafanactl/go.mod @@ -12,6 +12,7 @@ require ( github.com/grafana-tools/sdk v0.0.0-20220919052116-6562121319fc github.com/hashicorp/go-retryablehttp v0.7.8 github.com/spf13/cobra v1.10.2 + k8s.io/apimachinery v0.35.3 k8s.io/utils v0.0.0-20260319190234-28399d86e0b5 sigs.k8s.io/yaml v1.6.0 ) @@ -41,5 +42,4 @@ require ( golang.org/x/net v0.55.0 // indirect golang.org/x/sys v0.45.0 // indirect golang.org/x/text v0.37.0 // indirect - k8s.io/apimachinery v0.35.3 // indirect ) diff --git a/tools/grafanactl/internal/grafana/scratch.go b/tools/grafanactl/internal/grafana/scratch.go index 6737dc3b..329f052e 100644 --- a/tools/grafanactl/internal/grafana/scratch.go +++ b/tools/grafanactl/internal/grafana/scratch.go @@ -22,6 +22,8 @@ import ( "github.com/go-logr/logr" "github.com/grafana-tools/sdk" + "k8s.io/apimachinery/pkg/util/sets" + "github.com/Azure/ARO-Tools/tools/grafanactl/config" ) @@ -46,10 +48,10 @@ func (s *DashboardSyncer) syncScratchFolders(ctx context.Context) error { if len(s.config.GrafanaDashboards.ScratchFolders) == 0 { return nil } - return syncScratchFolders(ctx, s.client, s.config.GrafanaDashboards.ScratchFolders, s.dryRun, s.now) + return syncScratchFolders(ctx, s.client, s.config.GrafanaDashboards.ScratchFolders, s.dryRun, s.now()) } -func syncScratchFolders(ctx context.Context, client scratchGrafanaClient, folders []config.ScratchFolder, dryRun bool, now func() time.Time) error { +func syncScratchFolders(ctx context.Context, client scratchGrafanaClient, folders []config.ScratchFolder, dryRun bool, now time.Time) error { logger := logr.FromContextOrDiscard(ctx) existingFolders, err := client.ListFolders(ctx) @@ -102,7 +104,7 @@ func collectScratchFolderUIDs(rootUID string, allSearchFolders []sdk.FoundBoard) return uids } -func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name string, maxAge time.Duration, existingFolders []sdk.Folder, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, dryRun bool, now func() time.Time) error { +func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name string, maxAge time.Duration, existingFolders []sdk.Folder, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, dryRun bool, now time.Time) error { logger := logr.FromContextOrDiscard(ctx) folder, err := findOrCreateFolder(ctx, client, name, existingFolders, dryRun) @@ -121,8 +123,8 @@ func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name scratchUIDs := collectScratchFolderUIDs(folder.UID, allSearchFolders) - deletedDashboards := make(map[string]bool) - cutoff := now().Add(-maxAge) + deletedDashboards := sets.New[string]() + cutoff := now.Add(-maxAge) for _, db := range allDashboards { if !scratchUIDs[db.FolderUID] { continue @@ -141,13 +143,13 @@ func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name if dryRun { logger.Info("DRY_RUN: Would delete expired scratch dashboard", "title", db.Title, "uid", db.UID, "created", props.Created) - deletedDashboards[db.UID] = true + deletedDashboards.Insert(db.UID) } else { logger.Info("Deleting expired scratch dashboard", "title", db.Title, "uid", db.UID, "created", props.Created) if err := client.DeleteDashboardByUID(ctx, db.UID); err != nil { logger.Error(err, "Failed to delete expired scratch dashboard, continuing", "title", db.Title, "uid", db.UID) } else { - deletedDashboards[db.UID] = true + deletedDashboards.Insert(db.UID) } } } @@ -160,9 +162,7 @@ func syncOneScratchFolder(ctx context.Context, client scratchGrafanaClient, name // deleteEmptySubfolders removes subfolders of the scratch folder that contain no // dashboards (after expiry deletion). Processes leaf-first so nested empty trees // are fully removed. -func deleteEmptySubfolders(ctx context.Context, client scratchGrafanaClient, rootUID string, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, scratchUIDs map[string]bool, deletedDashboards map[string]bool, dryRun bool) { - logger := logr.FromContextOrDiscard(ctx) - +func deleteEmptySubfolders(ctx context.Context, client scratchGrafanaClient, rootUID string, allDashboards []sdk.FoundBoard, allSearchFolders []sdk.FoundBoard, scratchUIDs map[string]bool, deletedDashboards sets.Set[string], dryRun bool) { // Build parent→children map for subfolders only (exclude root). children := make(map[string][]sdk.FoundBoard) for _, f := range allSearchFolders { @@ -174,24 +174,26 @@ func deleteEmptySubfolders(ctx context.Context, client scratchGrafanaClient, roo dashCount := make(map[string]int) for _, db := range allDashboards { - if scratchUIDs[db.FolderUID] && !deletedDashboards[db.UID] { + if scratchUIDs[db.FolderUID] && !deletedDashboards.Has(db.UID) { dashCount[db.FolderUID]++ } } // Recursively delete leaf-first, starting from direct children of root. for _, child := range children[rootUID] { - deleteEmptyRecursive(ctx, client, child.UID, children, dashCount, dryRun, logger) + deleteEmptyRecursive(ctx, client, child.UID, children, dashCount, dryRun) } } // deleteEmptyRecursive walks the subfolder tree depth-first and deletes folders // that are empty (no dashboards and no remaining children after recursion). // Returns true if the folder at uid was deleted (or would be in dry-run). -func deleteEmptyRecursive(ctx context.Context, client scratchGrafanaClient, uid string, children map[string][]sdk.FoundBoard, dashCount map[string]int, dryRun bool, logger logr.Logger) bool { +func deleteEmptyRecursive(ctx context.Context, client scratchGrafanaClient, uid string, children map[string][]sdk.FoundBoard, dashCount map[string]int, dryRun bool) bool { + logger := logr.FromContextOrDiscard(ctx).WithValues("uid", uid) + hasChildren := false for _, child := range children[uid] { - if !deleteEmptyRecursive(ctx, client, child.UID, children, dashCount, dryRun, logger) { + if !deleteEmptyRecursive(ctx, client, child.UID, children, dashCount, dryRun) { hasChildren = true } } @@ -201,13 +203,13 @@ func deleteEmptyRecursive(ctx context.Context, client scratchGrafanaClient, uid } if dryRun { - logger.Info("DRY_RUN: Would delete empty scratch subfolder", "uid", uid) + logger.Info("DRY_RUN: Would delete empty scratch subfolder") return true } - logger.Info("Deleting empty scratch subfolder", "uid", uid) + logger.Info("Deleting empty scratch subfolder") if err := client.DeleteFolderByUID(ctx, uid); err != nil { - logger.Error(err, "Failed to delete empty scratch subfolder, continuing", "uid", uid) + logger.Error(err, "Failed to delete empty scratch subfolder, continuing") return false } return true diff --git a/tools/grafanactl/internal/grafana/scratch_test.go b/tools/grafanactl/internal/grafana/scratch_test.go index d5800259..403a8c3b 100644 --- a/tools/grafanactl/internal/grafana/scratch_test.go +++ b/tools/grafanactl/internal/grafana/scratch_test.go @@ -123,7 +123,7 @@ func TestSyncScratchFolders_CreatesFolder(t *testing.T) { folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -139,7 +139,7 @@ func TestSyncScratchFolders_ReuseExistingFolder(t *testing.T) { folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -162,7 +162,7 @@ func TestSyncScratchFolders_SetsPermissions(t *testing.T) { folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -208,7 +208,7 @@ func TestSyncScratchFolders_DeletesExpiredDashboards(t *testing.T) { } folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -238,7 +238,7 @@ func TestSyncScratchFolders_DeletesExpiredDashboardsInSubfolders(t *testing.T) { client.dashboardsByUID["sub-dash"] = sdk.BoardProperties{Created: now.Add(-8 * 24 * time.Hour)} folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -265,7 +265,7 @@ func TestSyncScratchFolders_DeletesEmptySubfolders(t *testing.T) { client.dashboards = nil folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -288,7 +288,7 @@ func TestSyncScratchFolders_DeletesNestedEmptySubfoldersLeafFirst(t *testing.T) client.dashboards = nil folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -322,7 +322,7 @@ func TestSyncScratchFolders_KeepsNonEmptySubfolders(t *testing.T) { } folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -343,7 +343,7 @@ func TestSyncScratchFolders_DoesNotDeleteRootScratchFolder(t *testing.T) { client.dashboards = nil folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -377,7 +377,7 @@ func TestSyncScratchFolders_ExpiryBoundary(t *testing.T) { } folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -406,7 +406,7 @@ func TestSyncScratchFolders_DryRun(t *testing.T) { } folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, true, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, true, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -435,7 +435,7 @@ func TestSyncScratchFolders_IgnoresDashboardsInOtherFolders(t *testing.T) { } folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -459,7 +459,7 @@ func TestSyncScratchFolders_DashboardDeleteErrorIsNonFatal(t *testing.T) { client.deleteDashboardErr = fmt.Errorf("delete failed") folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("expected no error (non-fatal), got %v", err) } @@ -476,7 +476,7 @@ func TestSyncScratchFolders_MetadataErrorIsNonFatal(t *testing.T) { folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("expected no error (non-fatal), got %v", err) } @@ -495,7 +495,7 @@ func TestSyncScratchFolders_FolderDeleteErrorIsNonFatal(t *testing.T) { client.deleteFolderErr = fmt.Errorf("folder delete failed") folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err != nil { t.Fatalf("expected no error (non-fatal), got %v", err) } @@ -508,7 +508,7 @@ func TestSyncScratchFolders_PermissionErrorIsFatal(t *testing.T) { now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err == nil { t.Fatal("expected error on permission update failure, got nil") } @@ -519,7 +519,7 @@ func TestSyncScratchFolders_ErrorOnInvalidMaxAge(t *testing.T) { now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "not-a-duration"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err == nil { t.Fatal("expected error for invalid maxAge, got nil") } @@ -531,7 +531,7 @@ func TestSyncScratchFolders_CreateFolderErrorIsFatal(t *testing.T) { now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) folders := []config.ScratchFolder{{Name: "Scratchpad", MaxAgeRaw: "168h"}} - err := syncScratchFolders(context.Background(), client, folders, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, folders, false, now) if err == nil { t.Fatal("expected error on folder creation failure, got nil") } @@ -541,7 +541,7 @@ func TestSyncScratchFolders_NoScratchFolders(t *testing.T) { client := newMockScratchClient() now := time.Date(2025, 7, 28, 12, 0, 0, 0, time.UTC) - err := syncScratchFolders(context.Background(), client, nil, false, func() time.Time { return now }) + err := syncScratchFolders(context.Background(), client, nil, false, now) if err != nil { t.Fatalf("unexpected error: %v", err) }