Downgrade remote modification check for dashboards to warning during planning (#3760)

## Why
This check should not block planning. Thus, this should be downgraded to
a warning during planning.

## Tests
New unit test.

---------

Co-authored-by: Claude <[email protected]>

Author: shreyas-goenka

acceptance/bundle/resources/dashboards/detect-change/output.txt

@@ -48,6 +48,23 @@ Deployment complete!
 }
 
 === Try to redeploy the bundle and confirm that the out of band modification is detected:
+>>> [CLI] bundle plan
+Warning: dashboard "file_reference" has been modified remotely
+  at resources.dashboards.file_reference
+  in databricks.yml:10:7
+
+This dashboard has been modified remotely since the last bundle deployment.
+These modifications are untracked and will be overwritten on deploy.
+
+Make sure that the local dashboard definition matches what you intend to deploy
+before proceeding with the deployment.
+
+Run `databricks bundle deploy --force` to bypass this error.
+
+update dashboards.file_reference
+
+Plan: 0 to add, 1 to change, 0 to delete, 0 unchanged
+
 >>> errcode [CLI] bundle deploy
 Error: dashboard "file_reference" has been modified remotely
   at resources.dashboards.file_reference

acceptance/bundle/resources/dashboards/detect-change/script

@@ -28,6 +28,7 @@ DASHBOARD_JSON=$($CLI bundle summary --output json | jq '{serialized_dashboard:
 $CLI lakeview update "${RESOURCE_ID}" --json "${DASHBOARD_JSON}" | jq '{lifecycle_state}'
 
 title "Try to redeploy the bundle and confirm that the out of band modification is detected:"
+trace $CLI bundle plan
 trace errcode $CLI bundle deploy
 
 title "Redeploy the bundle with the --force flag and confirm that the out of band modification is ignored:"

bundle/deploy/terraform/check_dashboards_modified_remotely.go

@@ -33,7 +33,9 @@ func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle) ([]dashbo
 	return dashboards, nil
 }
 
-type checkDashboardsModifiedRemotely struct{}
+type checkDashboardsModifiedRemotely struct {
+	isPlan bool
+}
 
 func (l *checkDashboardsModifiedRemotely) Name() string {
 	return "CheckDashboardsModifiedRemotely"
@@ -87,8 +89,14 @@ func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.B
 			continue
 		}
 
+		// Downgrade this to a warning in plan mode.
+		severity := diag.Error
+		if l.isPlan {
+			severity = diag.Warning
+		}
+
 		diags = diags.Append(diag.Diagnostic{
-			Severity: diag.Error,
+			Severity: severity,
 			Summary:  fmt.Sprintf("dashboard %q has been modified remotely", dashboard.Name),
 			Detail: "" +
 				"This dashboard has been modified remotely since the last bundle deployment.\n" +
@@ -107,6 +115,6 @@ func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.B
 	return diags
 }
 
-func CheckDashboardsModifiedRemotely() *checkDashboardsModifiedRemotely {
-	return &checkDashboardsModifiedRemotely{}
+func CheckDashboardsModifiedRemotely(isPlan bool) *checkDashboardsModifiedRemotely {
+	return &checkDashboardsModifiedRemotely{isPlan: isPlan}
 }

bundle/deploy/terraform/check_dashboards_modified_remotely_test.go

@@ -53,13 +53,13 @@ func TestCheckDashboardsModifiedRemotely_NoDashboards(t *testing.T) {
 		},
 	}
 
-	diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely())
+	diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely(false))
 	assert.Empty(t, diags)
 }
 
 func TestCheckDashboardsModifiedRemotely_FirstDeployment(t *testing.T) {
 	b := mockDashboardBundle(t)
-	diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely())
+	diags := bundle.Apply(context.Background(), b, CheckDashboardsModifiedRemotely(false))
 	assert.Empty(t, diags)
 }
 
@@ -82,7 +82,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateNoChange(t *testing.T) {
 	b.SetWorkpaceClient(m.WorkspaceClient)
 
 	// No changes, so no diags.
-	diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely())
+	diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false))
 	assert.Empty(t, diags)
 }
 
@@ -105,7 +105,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateChange(t *testing.T) {
 	b.SetWorkpaceClient(m.WorkspaceClient)
 
 	// The dashboard has changed, so expect an error.
-	diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely())
+	diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false))
 	if assert.Len(t, diags, 1) {
 		assert.Equal(t, diag.Error, diags[0].Severity)
 		assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary)
@@ -128,13 +128,39 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateFailureToGet(t *testing.T)
 	b.SetWorkpaceClient(m.WorkspaceClient)
 
 	// Unable to get the dashboard, so expect an error.
-	diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely())
+	diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false))
 	if assert.Len(t, diags, 1) {
 		assert.Equal(t, diag.Error, diags[0].Severity)
 		assert.Equal(t, `failed to get dashboard "dash1"`, diags[0].Summary)
 	}
 }
 
+func TestCheckDashboardsModifiedRemotely_ExistingStateChangePlanMode(t *testing.T) {
+	ctx := context.Background()
+
+	b := mockDashboardBundle(t)
+	writeFakeDashboardState(t, ctx, b)
+
+	// Mock the call to the API.
+	m := mocks.NewMockWorkspaceClient(t)
+	dashboardsAPI := m.GetMockLakeviewAPI()
+	dashboardsAPI.EXPECT().
+		GetByDashboardId(mock.Anything, "id1").
+		Return(&dashboards.Dashboard{
+			DisplayName: "My Special Dashboard",
+			Etag:        "1234",
+		}, nil).
+		Once()
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	// The dashboard has changed, but in plan mode expect a warning instead of an error.
+	diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(true))
+	if assert.Len(t, diags, 1) {
+		assert.Equal(t, diag.Warning, diags[0].Severity)
+		assert.Equal(t, `dashboard "dash1" has been modified remotely`, diags[0].Summary)
+	}
+}
+
 func writeFakeDashboardState(t *testing.T, ctx context.Context, b *bundle.Bundle) {
 	path, err := b.StateLocalPath(ctx)
 	require.NoError(t, err)

bundle/phases/deploy.go

@@ -153,7 +153,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand
 		bundle.ApplyContext(ctx, b, lock.Release(lock.GoalDeploy))
 	}()
 
-	libs := deployPrepare(ctx, b)
+	libs := deployPrepare(ctx, b, false)
 	if logdiag.HasError(ctx) {
 		return
 	}
@@ -243,7 +243,7 @@ func planWithoutPrepare(ctx context.Context, b *bundle.Bundle) *deployplan.Plan
 }
 
 func Plan(ctx context.Context, b *bundle.Bundle) *deployplan.Plan {
-	deployPrepare(ctx, b)
+	deployPrepare(ctx, b, true)
 	if logdiag.HasError(ctx) {
 		return nil
 	}

bundle/phases/plan.go

@@ -19,10 +19,10 @@ import (
 
 // deployPrepare is common set of mutators between "bundle plan" and "bundle deploy".
 // This function does not make any mutations in the workspace remotely, only in-memory bundle config mutations
-func deployPrepare(ctx context.Context, b *bundle.Bundle) map[string][]libraries.LocationToUpdate {
+func deployPrepare(ctx context.Context, b *bundle.Bundle, isPlan bool) map[string][]libraries.LocationToUpdate {
 	bundle.ApplySeqContext(ctx, b,
 		statemgmt.StatePull(),
-		terraform.CheckDashboardsModifiedRemotely(),
+		terraform.CheckDashboardsModifiedRemotely(isPlan),
 		deploy.StatePull(),
 		mutator.ValidateGitDetails(),
 		terraform.CheckRunningResource(),