feat(grafanactl): add scratch folder support for user-writable dashboards

[mmazur]

https://redhat.atlassian.net/browse/AROSLSRE-1631

Summary

• Adds scratchFolders config option to grafana-dashboards in the observability config
• During grafanactl sync dashboards, scratch folders are created (or found), Viewer role gets Edit permission, and dashboards older than maxAge are auto-deleted based on creation time
• Dashboards in subfolders are also expired; empty subfolders are cleaned up recursively (leaf-first) in the same sync pass
• Dashboard/folder deletion errors are logged as warnings and never fail the pipeline; permission and folder-creation errors are fatal

Details

Config example:

grafana-dashboards:
  scratchFolders:
  - name: Scratchpad (regularly autodeleted)
    maxAge: 168h

New/modified files:

• config/config.go — ScratchFolder type with MaxAge() duration parser
• internal/grafana/client.go — GetFolderPermissions, UpdateFolderPermissions, SearchFolders, DeleteFolderByUID
• internal/grafana/syncer.go — Injectable clock, calls syncScratchFolders from Sync()
• internal/grafana/scratch.go — Scratch folder sync logic
• internal/grafana/scratch_test.go — 20 unit tests

Testing

Tested end-to-end against a personal Azure Managed Grafana instance (mmazurtesting) deployed via make pipeline/Grafana with a config override. All tests run using grafanactl sync dashboards pointed at the test instance.

• T1 — First run creates folder and sets permissions: Ran sync on a fresh instance. Verified "Scratchpad (regularly autodeleted)" folder appeared in GET /api/folders and permissions showed Viewer=Edit, Editor=Edit, Admin=Admin via GET /api/folders/<uid>/permissions. ✅
• T2 — Second run reuses existing folder: Ran sync again. Log showed "Scratch folder already exists" with the same UID, no new folder created, permissions re-applied without error. ✅
• T3 — Expired dashboards are deleted: Created a dashboard in the Scratchpad folder via the Grafana API, set maxAge: 1s in observability.yaml, waited 10 seconds, ran sync. Log showed "Deleting expired scratch dashboard" with the correct title and UID. Verified dashboard gone via API. ✅
• T4 — Non-expired dashboards are kept: Restored maxAge: 168h, created a fresh dashboard, ran sync immediately. Log showed "Scratch dashboard not expired". Dashboard confirmed still present. ✅
• T5 — Dashboards in other folders untouched: Counted dashboards per folder via GET /api/search before and after scratch sync operations. All non-scratch folders (Azure Managed Prometheus, SRE, infra, etc.) retained their exact dashboard counts throughout all test runs. ✅
• T6 — Expired dashboards in subfolders are deleted: Created a subfolder "my-team" inside Scratchpad (via POST /api/folders with parentUid), added a dashboard, set maxAge: 1s, waited 10 seconds, ran sync. Log showed "Deleting expired scratch dashboard" for the subfolder dashboard. ✅
• T7 — Empty subfolders are cleaned up: After T6, the now-empty "my-team" subfolder was deleted in the same sync pass. Log showed "Deleting empty scratch subfolder". ✅
• T8 — Non-empty subfolders are kept: Created subfolder "keep-me" with a fresh dashboard, ran sync with maxAge: 168h. Log showed "Scratch dashboard not expired", subfolder confirmed still present, no folder deletions. ✅
• T9 — Nested empty subfolder cleanup is leaf-first: Created two-level nesting (parent → child) with a dashboard in child only. Expired and synced. Both dashboards deleted, then on the cleanup pass: child folder deleted first, parent folder deleted second. Root Scratchpad folder preserved. ✅
• T10 — Root scratch folder is never deleted: After all dashboards and subfolders were removed, the Scratchpad root folder persisted. Confirmed via GET /api/folders. ✅
• T11 — Dry run makes no changes: Created an expired dashboard and an empty subfolder, ran sync with --dry-run. Log showed "DRY_RUN: Would delete expired scratch dashboard", "DRY_RUN: Would delete empty scratch subfolder", and "DRY_RUN: Would set permissions on scratch folder". Verified via API that dashboard and subfolder both still existed afterward. ✅
• T12 — Error resilience (unit tests): TestSyncScratchFolders_DashboardDeleteErrorIsNonFatal, TestSyncScratchFolders_MetadataErrorIsNonFatal, and TestSyncScratchFolders_FolderDeleteErrorIsNonFatal confirm that individual deletion failures are logged and skipped without failing the overall sync. TestSyncScratchFolders_PermissionErrorIsFatal and TestSyncScratchFolders_CreateFolderErrorIsFatal confirm that permission and folder-creation failures are fatal. All 20 unit tests pass. ✅

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds “scratch folder” support to grafanactl sync dashboards so users can create/edit dashboards in designated Grafana folders that are automatically permissioned and periodically cleaned up (expired dashboards removed; empty subfolders deleted leaf-first).

Changes:

• Introduces scratchFolders in the observability config (name, maxAge) with duration parsing/validation.
• Extends the Grafana client wrapper with folder permission, folder search, and folder deletion operations.
• Implements scratch-folder sync: ensure folder exists, set Viewer→Edit permissions, delete expired dashboards (including in subfolders), and recursively delete empty subfolders; includes unit tests and injectable clock.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tools/grafanactl/internal/grafana/syncer.go	Injects a clock and wires scratch-folder sync into the main dashboard sync flow.
tools/grafanactl/internal/grafana/scratch.go	Implements scratch-folder creation/permissioning and expiry/cleanup logic.
tools/grafanactl/internal/grafana/scratch_test.go	Adds unit tests covering scratch-folder behavior, edge cases, and error handling.
tools/grafanactl/internal/grafana/client.go	Adds folder permission/search/delete helpers needed by scratch sync.
tools/grafanactl/config/config.go	Adds ScratchFolder config type and maxAge parsing/validation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[janboll]

just a few nits, please check

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tools/grafanactl/internal/grafana/syncer.go:105

• Sync() already calls fetchExistingState() (ListFolders/ListDashboards). Calling syncScratchFolders() immediately afterwards triggers another full folder + dashboard listing, which can significantly increase API calls and latency (and can contribute to rate-limit / timeout issues on large instances). Consider passing existingFolders / existingDashboards into scratch sync (and only separately fetching folder parent relationships via SearchFolders) so the sync pass reuses the data it already loaded.

	if err := s.syncScratchFolders(ctx); err != nil {
		return fmt.Errorf("failed to sync scratch folders: %w", err)
	}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

tools/grafanactl/internal/grafana/scratch.go:214

• PR description says folder deletion errors are logged as warnings. This is currently logged at error level even though deletion failures are explicitly non-fatal; consider lowering to Info (or equivalent) with an explicit warning message to match the stated behavior.

	logger.Info("Deleting empty scratch subfolder")
	if err := client.DeleteFolderByUID(ctx, uid); err != nil {
		logger.Error(err, "Failed to delete empty scratch subfolder, continuing")
		return false
	}

tools/grafanactl/internal/grafana/scratch.go:27

• Importing k8s.io/apimachinery just to use util/sets significantly increases the dependency footprint of grafanactl for a small convenience. A plain map[string]struct{} (or a small local set type) would avoid bringing in apimachinery as a direct dependency.

	"time"

	"github.com/go-logr/logr"
	"github.com/grafana-tools/sdk"

	"k8s.io/apimachinery/pkg/util/sets"

	"github.com/Azure/ARO-Tools/tools/grafanactl/config"

tools/grafanactl/internal/grafana/scratch.go:153

• PR description says deletion errors are logged as warnings and should not look like fatal failures. Here the error is logged at error level even though the code continues, which may trigger alerting/noise. Consider logging at Info (or equivalent) with an explicit warning message instead.

This issue also appears on line 210 of the same file.

			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.Insert(db.UID)