fix: delete provisioned dashboard in unified if forceDeleteRules is set (#113839)

fix: cleanup legacy resource if it is created in legacy during dual update (#113753)

Co-authored-by: Mustafa Sencer Özcan <32759850+mustafasencer@users.noreply.github.com>

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)
 		}

pkg/services/ngalert/notifier/alertmanager.go

@@ -233,7 +233,7 @@ func (am *alertmanager) SaveAndApplyConfig(ctx context.Context, cfg *apimodels.P
 		}
 
 		err = am.Store.SaveAlertmanagerConfigurationWithCallback(ctx, cmd, func() error {
-			_, err = am.applyConfig(ctx, cfg, LogInvalidReceivers) // fail if the autogen config is invalid
+			_, err = am.applyConfig(ctx, cfg, ErrorOnInvalidReceivers) // fail if the autogen config is invalid
 			return err
 		})
 		if err != nil {
@@ -259,7 +259,7 @@ func (am *alertmanager) ApplyConfig(ctx context.Context, dbCfg *ngmodels.AlertCo
 		// Since we will now update last_applied when autogen changes even if the user-created config remains the same.
 		// To fix this however, the local alertmanager needs to be able to tell the difference between user-created and
 		// autogen config, which may introduce cross-cutting complexity.
-		configChanged, err := am.applyConfig(ctx, cfg, ErrorOnInvalidReceivers)
+		configChanged, err := am.applyConfig(ctx, cfg, LogInvalidReceivers)
 		if err != nil {
 			outerErr = fmt.Errorf("unable to apply configuration: %w", err)
 			return

pkg/storage/legacysql/dualwrite/dualwriter.go

@@ -419,6 +419,15 @@ func (d *dualWriter) Update(ctx context.Context, name string, objInfo rest.Updat
 	// If we want to check unified errors just run it in foreground.
 	if _, _, err := d.unified.Update(ctx, name, unifiedInfo, createValidation, updateValidation, unifiedForceCreate, options); err != nil {
 		log.With("objectInfo", objectInfo(objFromLegacy)).Error("failed to UPDATE in unified storage", "err", err)
+		// cleanup the legacy object if we created it there
+		if createdLegacy {
+			go func(ctxBg context.Context, cancel context.CancelFunc) {
+				defer cancel()
+				if _, asyncDelete, err := d.legacy.Delete(ctxBg, name, nil, &metav1.DeleteOptions{}); err != nil {
+					log.With("name", name).Error("failed to CLEANUP object in legacy storage after unified storage update failure", "err", err, "asyncDelete", asyncDelete)
+				}
+			}(context.WithTimeout(context.WithoutCancel(ctx), backgroundReqTimeout))
+		}
 		return nil, false, err
 	}
 	return objFromLegacy, createdLegacy, nil

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)
+			})
+		})
+	}
+}

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
+}

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{

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