From e4f79e2e19e132bebdb096a41c801c7e4963da37 Mon Sep 17 00:00:00 2001 From: Rafael Bortolon Paulovic Date: Fri, 14 Nov 2025 14:11:05 +0100 Subject: [PATCH] fix: delete provisioned dashboard in unified if forceDeleteRules is set (#113839) --- .../dashboards/service/dashboard_service.go | 2 +- pkg/tests/apis/folder/folders_test.go | 138 ++++++++++++++++++ pkg/tests/apis/folder/testdata/dashboard.json | 39 +++++ pkg/tests/apis/helper.go | 8 +- pkg/tests/testinfra/testinfra.go | 4 + 5 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 pkg/tests/apis/folder/testdata/dashboard.json diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 7e4dd89dc7d..8c159b608be 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -1697,7 +1697,7 @@ func (dr *DashboardServiceImpl) DeleteInFolders(ctx context.Context, orgID int64 } for _, dash := range dashes { - errDel := dr.DeleteDashboard(ctx, dash.ID, dash.UID, orgID) + errDel := dr.deleteDashboard(ctx, dash.ID, dash.UID, orgID, false) if errDel != nil { dr.log.Error("failed to delete dashboard inside folder", "dashboardUID", dash.UID, "folderUIDs", folderUIDs, "error", errDel) } diff --git a/pkg/tests/apis/folder/folders_test.go b/pkg/tests/apis/folder/folders_test.go index 3fe36daa1e0..7e7d5dba6c5 100644 --- a/pkg/tests/apis/folder/folders_test.go +++ b/pkg/tests/apis/folder/folders_test.go @@ -6,12 +6,16 @@ import ( "encoding/json" "fmt" "net/http" + "os" + "path/filepath" "slices" "testing" "time" "github.com/google/uuid" + "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1" "github.com/prometheus/common/model" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1888,3 +1892,137 @@ func TestIntegrationDeleteNestedFoldersPostorder(t *testing.T) { }) } } + +// Test deleting folder with provisioned dashboard has proper handling with forceDeleteRules +func TestIntegrationDeleteFolderWithProvisionedDashboards(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + if !db.IsTestDbSQLite() { + t.Skip("test only on sqlite for now") + } + + for mode := 0; mode <= 5; mode++ { + t.Run(fmt.Sprintf("Mode %d: Delete provisioned folders and dashboards", mode), func(t *testing.T) { + modeDw := grafanarest.DualWriterMode(mode) + ops := testinfra.GrafanaOpts{ + DisableAnonymous: true, + AppModeProduction: true, + APIServerStorageType: "unified", + UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ + folders.RESOURCEGROUP: { + DualWriterMode: modeDw, + }, + "dashboards.dashboard.grafana.app": { + DualWriterMode: modeDw, + }, + }, + EnableFeatureToggles: []string{ + featuremgmt.FlagUnifiedStorageSearch, + }, + } + // Setup Grafana with provisioning + ops.Dir, ops.DirPath = testinfra.CreateGrafDir(t, ops) + // Create provisioning directories + provDashboardsDir := fmt.Sprintf("%s/conf/provisioning/dashboards", ops.Dir) + provDashboardsCfg := fmt.Sprintf("%s/dev.yaml", provDashboardsDir) + blob := []byte(fmt.Sprintf(` +apiVersion: 1 + +providers: +- name: 'provisioned dashboards' + type: file + orgId: 1 + folder: 'GrafanaCloud' + options: + path: %s`, provDashboardsDir)) + err := os.WriteFile(provDashboardsCfg, blob, 0o644) + require.NoError(t, err) + input, err := os.ReadFile(filepath.Join("testdata/dashboard.json")) + require.NoError(t, err) + provDashboardFile := filepath.Join(provDashboardsDir, "dashboard.json") + err = os.WriteFile(provDashboardFile, input, 0o644) + require.NoError(t, err) + + helper := apis.NewK8sTestHelper(t, ops) + + client := helper.GetResourceClient(apis.ResourceClientArgs{ + User: helper.Org1.Admin, + GVR: gvr, + }) + + var folderUID string + var dashboardUID string + require.EventuallyWithT(t, func(collect *assert.CollectT) { + resp := apis.DoRequest(helper, apis.RequestParams{ + User: client.Args.User, + Method: http.MethodGet, + Path: fmt.Sprintf("/apis/dashboard.grafana.app/v0alpha1/namespaces/%s/search?query=dashboard&limit=50&type=dashboard", client.Args.Namespace), + }, &map[string]interface{}{}) + var list v0alpha1.SearchResults + require.NotNil(t, resp.Response) + assert.Equal(t, http.StatusOK, resp.Response.StatusCode) + assert.NoError(t, json.Unmarshal(resp.Body, &list)) + assert.Equal(collect, list.TotalHits, int64(1), "Dashboard should be ready") + for _, d := range list.Hits { + folderUID = d.Folder + dashboardUID = d.Name + } + }, 10*time.Second, 25*time.Millisecond) + + _, err = client.Resource.Get(context.Background(), folderUID, metav1.GetOptions{}) + require.NoError(t, err, "folder %s should exist", folderUID) + // Verify dashboards exist + verifyDashboardExists := func(shouldExist bool) { + getDash := apis.DoRequest(helper, apis.RequestParams{ + User: client.Args.User, + Method: http.MethodGet, + Path: fmt.Sprintf("/apis/dashboard.grafana.app/v1beta1/namespaces/default/dashboards/%s", dashboardUID), + }, &map[string]interface{}{}) + if shouldExist { + require.Equal(t, http.StatusOK, getDash.Response.StatusCode, "dashboard %s should exist", dashboardUID) + } else { + require.Equal(t, http.StatusNotFound, getDash.Response.StatusCode, "dashboard %s should not exist", dashboardUID) + } + } + + verifyDashboardExists(true) + + t.Run("Deletion should fail when forceDeleteRules=false with provisioned dashboards", func(t *testing.T) { + // Attempt to delete the parent folder without forceDeleteRules + parentDeleteNoForce := apis.DoRequest(helper, apis.RequestParams{ + User: client.Args.User, + Method: http.MethodDelete, + Path: fmt.Sprintf("/api/folders/%s?forceDeleteRules=false", folderUID), + }, &folder.Folder{}) + + // Should fail because provisioned dashboards cannot be deleted without forceDeleteRules + require.Equal(t, http.StatusBadRequest, parentDeleteNoForce.Response.StatusCode, "deletion should fail without forceDeleteRules") + require.Contains(t, string(parentDeleteNoForce.Body), dashboards.ErrDashboardCannotDeleteProvisionedDashboard.Reason, "error message should indicate provisioned dashboards cannot be deleted") + // Verify folders still exist + _, err := client.Resource.Get(context.Background(), folderUID, metav1.GetOptions{}) + require.NoError(t, err, "parent folder %d should still exist", folderUID) + // Verify dashboard still exist + verifyDashboardExists(true) + }) + + t.Run("Deletion should succeed when forceDeleteRules=true and delete provisioned dashboards", func(t *testing.T) { + _, err = client.Resource.Get(context.Background(), folderUID, metav1.GetOptions{}) + require.NoError(t, err, "parent folder %d should still exist", folderUID) + // Delete the parent folder with forceDeleteRules=true + parentDelete := apis.DoRequest(helper, apis.RequestParams{ + User: client.Args.User, + Method: http.MethodDelete, + Path: fmt.Sprintf("/api/folders/%s?forceDeleteRules=true", folderUID), + }, &folder.Folder{}) + + require.Equal(t, http.StatusOK, parentDelete.Response.StatusCode, "deletion should succeed with forceDeleteRules") + + // Verify folders was deleted + _, err := client.Resource.Get(context.Background(), folderUID, metav1.GetOptions{}) + require.Error(t, err, "parent folder %s should not exist", folderUID) + // Verify provisioned dashboard is deleted + verifyDashboardExists(false) + }) + }) + } +} diff --git a/pkg/tests/apis/folder/testdata/dashboard.json b/pkg/tests/apis/folder/testdata/dashboard.json new file mode 100644 index 00000000000..0a07e7efa5e --- /dev/null +++ b/pkg/tests/apis/folder/testdata/dashboard.json @@ -0,0 +1,39 @@ +{ + "annotations": { + "list": [ + { + "builtIn": 1, + "datasource": { + "type": "grafana", + "uid": "-- Grafana --" + }, + "enable": true, + "hide": true, + "iconColor": "rgba(0, 211, 255, 1)", + "name": "Annotations & Alerts", + "type": "dashboard" + } + ] + }, + "editable": true, + "fiscalYearStartMonth": 0, + "graphTooltip": 0, + "id": 1013, + "links": [], + "panels": [], + "preload": false, + "schemaVersion": 42, + "tags": [], + "templating": { + "list": [] + }, + "time": { + "from": "now-6h", + "to": "now" + }, + "timepicker": {}, + "timezone": "browser", + "title": "dashboard", + "uid": "ad6hz5l", + "version": 1 +} diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index 4eca0e024b3..33964d9efa0 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -88,7 +88,13 @@ func NewK8sTestHelper(t *testing.T, opts testinfra.GrafanaOpts) *K8sTestHelper { // Always enable `FlagAppPlatformGrpcClientAuth` for k8s integration tests, as this is the desired behavior. // The flag only exists to support the transition from the old to the new behavior in dev/ops/prod. opts.EnableFeatureToggles = append(opts.EnableFeatureToggles, featuremgmt.FlagAppPlatformGrpcClientAuth) - dir, path := testinfra.CreateGrafDir(t, opts) + var ( + dir = opts.Dir + path = opts.DirPath + ) + if opts.Dir == "" && opts.DirPath == "" { + dir, path = testinfra.CreateGrafDir(t, opts) + } listenerAddress, env := testinfra.StartGrafanaEnv(t, dir, path) c := &K8sTestHelper{ diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 8f00449ac4b..9b0baf4f0ba 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -632,6 +632,10 @@ type GrafanaOpts struct { DisableControllers bool SecretsManagerEnableDBMigrations bool + // Allow creating grafana dir beforehand + Dir string + DirPath string + // When "unified-grpc" is selected it will also start the grpc server APIServerStorageType options.StorageType