Refactor validate_direct_only_resources and approval log loops (#5215)

## Changes

Two independent refactors in `bundle/`:

1. **`bundle/config/mutator/validate_direct_only_resources.go`** — drop
the local `directOnlyResource` struct (whose `pluralName`/`singularName`
fields actually held `SingularTitle`/`SingularName` values) and the
per-type `getResources` closures. Iterate
`b.Config.Resources.AllResources()` and key off `PluralName` via a small
`map[string]bool` instead, reading `SingularTitle`/`SingularName` from
the canonical `ResourceDescription`. Adds a unit test covering
direct/terraform engines × the three direct-only resource types.

2. **`bundle/phases/{deploy,destroy}.go`** — collapse the eight (deploy)
and seven (destroy) near-identical `if len(xActions) != 0 {
LogString(message); for _ { Log(action) } }` blocks into a table-driven
helper `logApprovalGroups` in a new `bundle/phases/approval.go`. The
deploy version also replaces the eight-clause `len(...) == 0 &&`
early-return with a single `total == 0` check returned from the helper.
The schema child-resource skip (deploy only) and trailing blank lines
(destroy only) are preserved via per-group `skipChildren`/`trailingGap`
flags. The outer "all deletes" preamble in destroy stays as-is — it's
structurally different.

Net diff: −215 source LOC, +110 test LOC.

## Why

Both files had grown into mechanical repetition.
`validate_direct_only_resources.go` re-declared resource metadata that
already lives on each resource's `ResourceDescription()` and named the
fields incorrectly. The approval functions repeated the same
eight-/seven-times pattern inline, with an opaque eight-clause boolean
expression for the early-return.

There is one minor user-visible wording change: for `external_locations`
and `vector_search_endpoints`, the Detail message now reads `... use
external_location resources.` / `... use vector_search_endpoint
resources.` (snake_case, from `SingularName`) instead of `... use
external location resources.` / `... use vector search endpoint
resources.` (the old struct's hand-written field with spaces). The
catalog message is byte-identical. There are no acceptance tests
covering the other two messages.

## Tests

- New unit test `validate_direct_only_resources_test.go` (table-driven
over the three direct-only types).
- Existing acceptance test
`bundle/validate/catalog_requires_direct_mode` passes byte-identically.
- `bundle/destroy/...`, `bundle/deploy/...` (excluding the pre-existing
`spark-jar-task` Java env failure that also fails on `main`),
`bundle/resources/grants/schemas/...`, `bundle/config-remote-sync/...`,
and `bundle/user_agent/simple` all pass byte-identically.
- `./task fmt`, `./task checks`, `./task lint` clean.

_This PR was written by Claude Code._

Author: janniklasrose

bundle/config/mutator/validate_direct_only_resources.go

@@ -6,56 +6,11 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config/engine"
+	"github.com/databricks/cli/bundle/deploy/terraform"
+	"github.com/databricks/cli/bundle/direct/dresources"
 	"github.com/databricks/cli/libs/diag"
 )
 
-type directOnlyResource struct {
-	resourceType string
-	pluralName   string
-	singularName string
-	getResources func(*bundle.Bundle) map[string]any
-}
-
-// Resources that are only supported in direct deployment mode
-var directOnlyResources = []directOnlyResource{
-	{
-		resourceType: "catalogs",
-		pluralName:   "Catalog",
-		singularName: "catalog",
-		getResources: func(b *bundle.Bundle) map[string]any {
-			result := make(map[string]any)
-			for k, v := range b.Config.Resources.Catalogs {
-				result[k] = v
-			}
-			return result
-		},
-	},
-	{
-		resourceType: "external_locations",
-		pluralName:   "External Location",
-		singularName: "external location",
-		getResources: func(b *bundle.Bundle) map[string]any {
-			result := make(map[string]any)
-			for k, v := range b.Config.Resources.ExternalLocations {
-				result[k] = v
-			}
-			return result
-		},
-	},
-	{
-		resourceType: "vector_search_endpoints",
-		pluralName:   "Vector Search Endpoint",
-		singularName: "vector search endpoint",
-		getResources: func(b *bundle.Bundle) map[string]any {
-			result := make(map[string]any)
-			for k, v := range b.Config.Resources.VectorSearchEndpoints {
-				result[k] = v
-			}
-			return result
-		},
-	},
-}
-
 type validateDirectOnlyResources struct {
 	engine engine.EngineType
 }
@@ -70,26 +25,37 @@ func (m *validateDirectOnlyResources) Name() string {
 	return "ValidateDirectOnlyResources"
 }
 
+// isDirectOnly reports whether a resource type (by PluralName) is supported only
+// by the direct engine — present in dresources.SupportedResources but absent
+// from terraform.GroupToTerraformName.
+func isDirectOnly(pluralName string) bool {
+	_, hasDirect := dresources.SupportedResources[pluralName]
+	_, hasTerraform := terraform.GroupToTerraformName[pluralName]
+	return hasDirect && !hasTerraform
+}
+
 func (m *validateDirectOnlyResources) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	if m.engine.IsDirect() {
 		return nil
 	}
 
 	var diags diag.Diagnostics
-
-	for _, resource := range directOnlyResources {
-		resourceMap := resource.getResources(b)
-		if len(resourceMap) > 0 {
-			diags = diags.Append(diag.Diagnostic{
-				Severity: diag.Error,
-				Summary:  resource.pluralName + " resources are only supported with direct deployment mode",
-				Detail: fmt.Sprintf("%s resources require direct deployment mode. "+
-					"Please set the DATABRICKS_BUNDLE_ENGINE environment variable to 'direct' to use %s resources.\n"+
-					"Learn more at https://docs.databricks.com/dev-tools/bundles/direct",
-					resource.pluralName, resource.singularName),
-				Locations: b.Config.GetLocations("resources." + resource.resourceType),
-			})
+	for _, group := range b.Config.Resources.AllResources() {
+		if len(group.Resources) == 0 {
+			continue
+		}
+		if !isDirectOnly(group.Description.PluralName) {
+			continue
 		}
+		diags = diags.Append(diag.Diagnostic{
+			Severity: diag.Error,
+			Summary:  group.Description.SingularTitle + " resources are only supported with direct deployment mode",
+			Detail: fmt.Sprintf("%s resources require direct deployment mode. "+
+				"Please set the DATABRICKS_BUNDLE_ENGINE environment variable to 'direct' to use %s resources.\n"+
+				"Learn more at https://docs.databricks.com/dev-tools/bundles/direct",
+				group.Description.SingularTitle, group.Description.SingularName),
+			Locations: b.Config.GetLocations("resources." + group.Description.PluralName),
+		})
 	}
 
 	return diags

bundle/config/mutator/validate_direct_only_resources_test.go

@@ -0,0 +1,110 @@
+package mutator_test
+
+import (
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/bundle/config/engine"
+	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/bundle/config/resources"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidateDirectOnlyResourcesDirectEngineReturnsNil(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Catalogs: map[string]*resources.Catalog{
+					"my_catalog": {},
+				},
+			},
+		},
+	}
+
+	diags := bundle.Apply(t.Context(), b, mutator.ValidateDirectOnlyResources(engine.EngineDirect))
+	assert.Empty(t, diags)
+}
+
+func TestValidateDirectOnlyResourcesTerraformEngineNoDirectOnlyReturnsNil(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Resources: config.Resources{
+				Jobs: map[string]*resources.Job{
+					"my_job": {},
+				},
+			},
+		},
+	}
+
+	diags := bundle.Apply(t.Context(), b, mutator.ValidateDirectOnlyResources(engine.EngineTerraform))
+	assert.Empty(t, diags)
+}
+
+func TestValidateDirectOnlyResourcesTerraformEngineDirectOnlyEmitsError(t *testing.T) {
+	cases := []struct {
+		name            string
+		bundle          *bundle.Bundle
+		expectedSummary string
+		expectedDetail  string
+	}{
+		{
+			name: "catalogs",
+			bundle: &bundle.Bundle{
+				Config: config.Root{
+					Resources: config.Resources{
+						Catalogs: map[string]*resources.Catalog{
+							"my_catalog": {},
+						},
+					},
+				},
+			},
+			expectedSummary: "Catalog resources are only supported with direct deployment mode",
+			expectedDetail: "Catalog resources require direct deployment mode. " +
+				"Please set the DATABRICKS_BUNDLE_ENGINE environment variable to 'direct' to use catalog resources.\n" +
+				"Learn more at https://docs.databricks.com/dev-tools/bundles/direct",
+		},
+		{
+			name: "external_locations",
+			bundle: &bundle.Bundle{
+				Config: config.Root{
+					Resources: config.Resources{
+						ExternalLocations: map[string]*resources.ExternalLocation{
+							"my_location": {},
+						},
+					},
+				},
+			},
+			expectedSummary: "External Location resources are only supported with direct deployment mode",
+			expectedDetail: "External Location resources require direct deployment mode. " +
+				"Please set the DATABRICKS_BUNDLE_ENGINE environment variable to 'direct' to use external_location resources.\n" +
+				"Learn more at https://docs.databricks.com/dev-tools/bundles/direct",
+		},
+		{
+			name: "vector_search_endpoints",
+			bundle: &bundle.Bundle{
+				Config: config.Root{
+					Resources: config.Resources{
+						VectorSearchEndpoints: map[string]*resources.VectorSearchEndpoint{
+							"my_endpoint": {},
+						},
+					},
+				},
+			},
+			expectedSummary: "Vector Search Endpoint resources are only supported with direct deployment mode",
+			expectedDetail: "Vector Search Endpoint resources require direct deployment mode. " +
+				"Please set the DATABRICKS_BUNDLE_ENGINE environment variable to 'direct' to use vector_search_endpoint resources.\n" +
+				"Learn more at https://docs.databricks.com/dev-tools/bundles/direct",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			diags := bundle.Apply(t.Context(), tc.bundle, mutator.ValidateDirectOnlyResources(engine.EngineTerraform))
+			if assert.Len(t, diags, 1) {
+				assert.Equal(t, tc.expectedSummary, diags[0].Summary)
+				assert.Equal(t, tc.expectedDetail, diags[0].Detail)
+			}
+		})
+	}
+}

bundle/phases/approval.go

@@ -0,0 +1,41 @@
+package phases
+
+import (
+	"context"
+
+	"github.com/databricks/cli/bundle/deployplan"
+	"github.com/databricks/cli/libs/cmdio"
+)
+
+// approvalGroup describes one resource type that needs explicit user consent
+// before a destructive action is applied.
+type approvalGroup struct {
+	group        string // matches config.GetResourceTypeFromKey, e.g. "schemas"
+	message      string // banner shown above the action list
+	skipChildren bool   // skip actions where IsChildResource() is true
+}
+
+// logApprovalGroups filters actions per group and prints non-empty groups.
+// If trailingNewline is true, an empty line is printed after each non-empty group.
+// Returns the total number of matched actions across all groups.
+func logApprovalGroups(ctx context.Context, actions []deployplan.Action, groups []approvalGroup, trailingNewline bool, types ...deployplan.ActionType) int {
+	total := 0
+	for _, g := range groups {
+		matched := filterGroup(actions, g.group, types...)
+		if len(matched) == 0 {
+			continue
+		}
+		total += len(matched)
+		cmdio.LogString(ctx, g.message)
+		for _, a := range matched {
+			if g.skipChildren && a.IsChildResource() {
+				continue
+			}
+			cmdio.Log(ctx, a)
+		}
+		if trailingNewline {
+			cmdio.LogString(ctx, "")
+		}
+	}
+	return total
+}

bundle/phases/deploy.go

@@ -26,6 +26,17 @@ import (
 	"github.com/databricks/cli/libs/sync"
 )
 
+var deployApprovalGroups = []approvalGroup{
+	{group: "schemas", message: deleteOrRecreateSchemaMessage, skipChildren: true},
+	{group: "pipelines", message: deleteOrRecreatePipelineMessage},
+	{group: "volumes", message: deleteOrRecreateVolumeMessage},
+	{group: "dashboards", message: deleteOrRecreateDashboardMessage},
+	{group: "database_instances", message: deleteOrRecreateDatabaseInstanceMessage},
+	{group: "synced_database_tables", message: deleteOrRecreateSyncedDatabaseTableMessage},
+	{group: "postgres_projects", message: deleteOrRecreatePostgresProjectMessage},
+	{group: "postgres_branches", message: deleteOrRecreatePostgresBranchMessage},
+}
+
 func approvalForDeploy(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) (bool, error) {
 	actions := plan.GetActions()
 
@@ -34,90 +45,12 @@ func approvalForDeploy(ctx context.Context, b *bundle.Bundle, plan *deployplan.P
 		return false, err
 	}
 
-	types := []deployplan.ActionType{deployplan.Recreate, deployplan.Delete}
-	schemaActions := filterGroup(actions, "schemas", types...)
-	pipelineActions := filterGroup(actions, "pipelines", types...)
-	volumeActions := filterGroup(actions, "volumes", types...)
-	dashboardActions := filterGroup(actions, "dashboards", types...)
-	databaseInstanceActions := filterGroup(actions, "database_instances", types...)
-	syncedDatabaseTableActions := filterGroup(actions, "synced_database_tables", types...)
-	postgresProjectActions := filterGroup(actions, "postgres_projects", types...)
-	postgresBranchActions := filterGroup(actions, "postgres_branches", types...)
-
-	// We don't need to display any prompts in this case.
-	if len(schemaActions) == 0 && len(pipelineActions) == 0 && len(volumeActions) == 0 && len(dashboardActions) == 0 &&
-		len(databaseInstanceActions) == 0 && len(syncedDatabaseTableActions) == 0 &&
-		len(postgresProjectActions) == 0 && len(postgresBranchActions) == 0 {
+	total := logApprovalGroups(ctx, actions, deployApprovalGroups, false, deployplan.Recreate, deployplan.Delete)
+	if total == 0 {
+		// No destructive actions in any tracked group: skip the prompt.
 		return true, nil
 	}
 
-	// One or more UC schema resources will be deleted or recreated.
-	if len(schemaActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreateSchemaMessage)
-		for _, action := range schemaActions {
-			if action.IsChildResource() {
-				continue
-			}
-			cmdio.Log(ctx, action)
-		}
-	}
-
-	// One or more pipelines is being recreated.
-	if len(pipelineActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreatePipelineMessage)
-		for _, action := range pipelineActions {
-			cmdio.Log(ctx, action)
-		}
-	}
-
-	// One or more volumes is being recreated.
-	if len(volumeActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreateVolumeMessage)
-		for _, action := range volumeActions {
-			cmdio.Log(ctx, action)
-		}
-	}
-
-	// One or more dashboards is being recreated.
-	if len(dashboardActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreateDashboardMessage)
-		for _, action := range dashboardActions {
-			cmdio.Log(ctx, action)
-		}
-	}
-
-	// One or more database instances is being deleted or recreated.
-	if len(databaseInstanceActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreateDatabaseInstanceMessage)
-		for _, action := range databaseInstanceActions {
-			cmdio.Log(ctx, action)
-		}
-	}
-
-	// One or more synced database tables is being deleted or recreated.
-	if len(syncedDatabaseTableActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreateSyncedDatabaseTableMessage)
-		for _, action := range syncedDatabaseTableActions {
-			cmdio.Log(ctx, action)
-		}
-	}
-
-	// One or more Lakebase projects is being deleted or recreated.
-	if len(postgresProjectActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreatePostgresProjectMessage)
-		for _, action := range postgresProjectActions {
-			cmdio.Log(ctx, action)
-		}
-	}
-
-	// One or more Lakebase branches is being deleted or recreated.
-	if len(postgresBranchActions) != 0 {
-		cmdio.LogString(ctx, deleteOrRecreatePostgresBranchMessage)
-		for _, action := range postgresBranchActions {
-			cmdio.Log(ctx, action)
-		}
-	}
-
 	if b.AutoApprove {
 		return true, nil
 	}
@@ -127,12 +60,7 @@ func approvalForDeploy(ctx context.Context, b *bundle.Bundle, plan *deployplan.P
 	}
 
 	cmdio.LogString(ctx, "")
-	approved, err := cmdio.AskYesOrNo(ctx, "Would you like to proceed?")
-	if err != nil {
-		return false, err
-	}
-
-	return approved, nil
+	return cmdio.AskYesOrNo(ctx, "Would you like to proceed?")
 }
 
 func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, targetEngine engine.EngineType) {