From 29c7f0b3fb3f8bc0c7bdb43d8ce1d75bca5def8b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 5 Oct 2026 21:47:44 +0200 Subject: [PATCH] bundle: drop engine parameters that are always direct After migration the phases always run on the direct engine, so drop the engine argument from Deploy, deployCore, RunPlan, Destroy, destroyCore, PreDeployChecks, pipelineDeletionCascades, SecretScopeFixups, CheckRunningResource, CheckDashboardsModifiedRemotely and ExtractChanges, together with the branches on it. CheckDashboardsModifiedRemotely moves from bundle/deploy/terraform to bundle/deploy. bind/unbind and apps import now pass the bundle resource group straight to phases.Bind/Unbind instead of round-tripping through Terraform resource names; BindOptions moves next to phases.Bind. Co-authored-by: Isaac --- .../resourcemutator/secret_scope_fixups.go | 15 +-- bundle/configsync/diff.go | 15 ++- bundle/configsync/diff_test.go | 29 +++--- .../check_dashboards_modified_remotely.go | 28 ++---- ...check_dashboards_modified_remotely_test.go | 92 +++++-------------- bundle/deploy/terraform/import.go | 8 -- bundle/phases/bind.go | 25 +++-- bundle/phases/deploy.go | 36 +++----- bundle/phases/destroy.go | 17 ++-- bundle/phases/destroy_test.go | 3 +- bundle/phases/plan.go | 20 ++-- bundle/phases/plan_test.go | 20 +--- bundle/statemgmt/check_running_resources.go | 24 +---- bundle/statemgmt/direct_migration.go | 2 +- cmd/apps/import.go | 15 +-- cmd/bundle/config_remote_sync.go | 2 +- cmd/bundle/deployment/bind_resource.go | 15 +-- cmd/bundle/deployment/migrate.go | 2 +- cmd/bundle/deployment/unbind.go | 8 +- cmd/bundle/destroy.go | 2 +- cmd/bundle/plan.go | 4 +- cmd/bundle/utils/process.go | 4 +- 22 files changed, 118 insertions(+), 268 deletions(-) rename bundle/deploy/{terraform => }/check_dashboards_modified_remotely.go (81%) rename bundle/deploy/{terraform => }/check_dashboards_modified_remotely_test.go (71%) delete mode 100644 bundle/deploy/terraform/import.go diff --git a/bundle/config/mutator/resourcemutator/secret_scope_fixups.go b/bundle/config/mutator/resourcemutator/secret_scope_fixups.go index e85584da3f0..4b1330d220a 100644 --- a/bundle/config/mutator/resourcemutator/secret_scope_fixups.go +++ b/bundle/config/mutator/resourcemutator/secret_scope_fixups.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" @@ -15,12 +14,10 @@ import ( "github.com/databricks/databricks-sdk-go/service/iam" ) -type secretScopeFixups struct { - engine engine.EngineType -} +type secretScopeFixups struct{} -func SecretScopeFixups(engine engine.EngineType) bundle.Mutator { - return &secretScopeFixups{engine: engine} +func SecretScopeFixups() bundle.Mutator { + return &secretScopeFixups{} } func (m *secretScopeFixups) Name() string { @@ -123,12 +120,6 @@ func collapsePermissions(scope *resources.SecretScope) error { func (m *secretScopeFixups) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // Secret scopes by default have the current user as a MANAGE ACL. We need to add it to the client ACL list // to prevent a phantom persistent diff. - // We do not need to do this in terraform because terraform naively always applies the config during ACL - // creation without checking if the ACL already exists. - // https://github.com/databricks/terraform-provider-databricks/blob/5cb5d3fa46bc4843be1a4c4bce89296eaa2e14fc/secrets/resource_secret_acl.go#L43 - if !m.engine.IsDirect() { - return nil - } // Secret scopes assigns the create MANAGE ACL on it by default. So we always add it to // the client ACL list as a default. diff --git a/bundle/configsync/diff.go b/bundle/configsync/diff.go index 149f629f46b..e42f0a1d83a 100644 --- a/bundle/configsync/diff.go +++ b/bundle/configsync/diff.go @@ -7,7 +7,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" @@ -139,8 +138,8 @@ func isPermissionsOrGrantsSubResource(resourceKey string) bool { } // ExtractChanges extracts the map of remote-vs-config changes from a deploy -// plan. engine selects the LocalEdit comparison below. -func ExtractChanges(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, engine engine.EngineType) (Changes, error) { +// plan. +func ExtractChanges(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) (Changes, error) { changes := make(Changes) for resourceKey, entry := range plan.Plan { @@ -170,12 +169,10 @@ func ExtractChanges(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan if change.Operation == OperationSkip { continue } - // On the direct engine the state snapshot holds real per-field - // values, so New != Old means the local config diverged from the - // last deploy and this change overwrites that pending edit. The - // terraform sync snapshot stores empty per-resource state, so this - // comparison is meaningless there and is skipped. - if engine.IsDirect() && !structdiff.IsEqual(changeDesc.Old, changeDesc.New) { + // The state snapshot holds real per-field values, so New != Old + // means the local config diverged from the last deploy and this + // change overwrites that pending edit. + if !structdiff.IsEqual(changeDesc.Old, changeDesc.New) { change.LocalEdit = true } change.Value = stripNamePrefix(fullPath, change.Value, b.Config.Presets.NamePrefix) diff --git a/bundle/configsync/diff_test.go b/bundle/configsync/diff_test.go index bb1acc682ba..b16885f6b71 100644 --- a/bundle/configsync/diff_test.go +++ b/bundle/configsync/diff_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deployplan" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -283,23 +282,19 @@ func TestExtractChangesSkipsPermissionsAndGrants(t *testing.T) { }, }} - for _, eng := range []engine.EngineType{engine.EngineDirect, engine.EngineTerraform} { - t.Run(string(eng), func(t *testing.T) { - changes, err := ExtractChanges(ctx, &bundle.Bundle{}, plan, eng) - require.NoError(t, err) + changes, err := ExtractChanges(ctx, &bundle.Bundle{}, plan) + require.NoError(t, err) - _, hasPerms := changes["resources.jobs.my_job.permissions"] - assert.False(t, hasPerms, "permissions sub-resource must be skipped") - _, hasGrants := changes["resources.schemas.my_schema.grants"] - assert.False(t, hasGrants, "grants sub-resource must be skipped") + _, hasPerms := changes["resources.jobs.my_job.permissions"] + assert.False(t, hasPerms, "permissions sub-resource must be skipped") + _, hasGrants := changes["resources.schemas.my_schema.grants"] + assert.False(t, hasGrants, "grants sub-resource must be skipped") - job, hasJob := changes["resources.jobs.my_job"] - require.True(t, hasJob, "regular resource changes must be kept") - assert.Contains(t, job, "description") + job, hasJob := changes["resources.jobs.my_job"] + require.True(t, hasJob, "regular resource changes must be kept") + assert.Contains(t, job, "description") - namedPerms, hasNamedPerms := changes["resources.jobs.permissions"] - require.True(t, hasNamedPerms, "a resource named permissions must not be skipped") - assert.Contains(t, namedPerms, "description") - }) - } + namedPerms, hasNamedPerms := changes["resources.jobs.permissions"] + require.True(t, hasNamedPerms, "a resource named permissions must not be skipped") + assert.Contains(t, namedPerms, "description") } diff --git a/bundle/deploy/terraform/check_dashboards_modified_remotely.go b/bundle/deploy/check_dashboards_modified_remotely.go similarity index 81% rename from bundle/deploy/terraform/check_dashboards_modified_remotely.go rename to bundle/deploy/check_dashboards_modified_remotely.go index ed873618ed1..559206b876c 100644 --- a/bundle/deploy/terraform/check_dashboards_modified_remotely.go +++ b/bundle/deploy/check_dashboards_modified_remotely.go @@ -1,4 +1,4 @@ -package terraform +package deploy import ( "context" @@ -6,7 +6,6 @@ import ( "strings" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/libs/agent" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" @@ -18,17 +17,8 @@ type dashboardState struct { ETag string } -func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle, directDeployment bool) ([]dashboardState, error) { - var state ExportedResourcesMap - var err error - if directDeployment { - state = b.DeploymentBundle.ExportState(ctx) - } else { - state, err = ParseResourcesState(ctx, b) - if err != nil { - return nil, err - } - } +func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle) []dashboardState { + state := b.DeploymentBundle.ExportState(ctx) var dashboards []dashboardState for resourceKey, instance := range state { @@ -50,12 +40,11 @@ func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle, directDep }) } - return dashboards, nil + return dashboards } type checkDashboardsModifiedRemotely struct { isPlan bool - engine engine.EngineType } func (l *checkDashboardsModifiedRemotely) Name() string { @@ -73,10 +62,7 @@ func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.B return nil } - dashboards, err := collectDashboardsFromState(ctx, b, l.engine.IsDirect()) - if err != nil { - return diag.FromErr(err) - } + dashboards := collectDashboardsFromState(ctx, b) var diags diag.Diagnostics for _, dashboard := range dashboards { @@ -131,6 +117,6 @@ func (l *checkDashboardsModifiedRemotely) Apply(ctx context.Context, b *bundle.B return diags } -func CheckDashboardsModifiedRemotely(isPlan bool, engine engine.EngineType) *checkDashboardsModifiedRemotely { - return &checkDashboardsModifiedRemotely{isPlan: isPlan, engine: engine} +func CheckDashboardsModifiedRemotely(isPlan bool) *checkDashboardsModifiedRemotely { + return &checkDashboardsModifiedRemotely{isPlan: isPlan} } diff --git a/bundle/deploy/terraform/check_dashboards_modified_remotely_test.go b/bundle/deploy/check_dashboards_modified_remotely_test.go similarity index 71% rename from bundle/deploy/terraform/check_dashboards_modified_remotely_test.go rename to bundle/deploy/check_dashboards_modified_remotely_test.go index 6270e4229fe..797fff59669 100644 --- a/bundle/deploy/terraform/check_dashboards_modified_remotely_test.go +++ b/bundle/deploy/check_dashboards_modified_remotely_test.go @@ -1,15 +1,15 @@ -package terraform +package deploy import ( - "context" + "encoding/json" "errors" + "path/filepath" "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/resources" - "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/bundle/direct/dstate" "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/dashboards" @@ -51,13 +51,14 @@ func TestCheckDashboardsModifiedRemotely_NoDashboards(t *testing.T) { }, } - diags := bundle.Apply(t.Context(), b, CheckDashboardsModifiedRemotely(false, engine.EngineTerraform)) + diags := bundle.Apply(t.Context(), b, CheckDashboardsModifiedRemotely(false)) assert.Empty(t, diags) } func TestCheckDashboardsModifiedRemotely_FirstDeployment(t *testing.T) { b := mockDashboardBundle(t) - diags := bundle.Apply(t.Context(), b, CheckDashboardsModifiedRemotely(false, engine.EngineTerraform)) + openDashboardState(t, b, dstate.NewDatabase("lineage", 1)) + diags := bundle.Apply(t.Context(), b, CheckDashboardsModifiedRemotely(false)) assert.Empty(t, diags) } @@ -65,7 +66,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateNoChange(t *testing.T) { ctx := t.Context() b := mockDashboardBundle(t) - writeFakeDashboardState(t, ctx, b) + writeFakeDashboardState(t, b) // Mock the call to the API. m := mocks.NewMockWorkspaceClient(t) @@ -80,7 +81,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateNoChange(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) // No changes, so no diags. - diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false, engine.EngineTerraform)) + diags := bundle.Apply(ctx, b, CheckDashboardsModifiedRemotely(false)) assert.Empty(t, diags) } @@ -88,7 +89,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateChange(t *testing.T) { ctx := t.Context() b := mockDashboardBundle(t) - writeFakeDashboardState(t, ctx, b) + writeFakeDashboardState(t, b) // Mock the call to the API. m := mocks.NewMockWorkspaceClient(t) @@ -103,7 +104,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(false, engine.EngineTerraform)) + 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) @@ -114,7 +115,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateFailureToGet(t *testing.T) ctx := t.Context() b := mockDashboardBundle(t) - writeFakeDashboardState(t, ctx, b) + writeFakeDashboardState(t, b) // Mock the call to the API. m := mocks.NewMockWorkspaceClient(t) @@ -126,7 +127,7 @@ 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(false, engine.EngineTerraform)) + 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) @@ -137,7 +138,7 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateChangePlanMode(t *testing. ctx := t.Context() b := mockDashboardBundle(t) - writeFakeDashboardState(t, ctx, b) + writeFakeDashboardState(t, b) // Mock the call to the API. m := mocks.NewMockWorkspaceClient(t) @@ -152,64 +153,21 @@ func TestCheckDashboardsModifiedRemotely_ExistingStateChangePlanMode(t *testing. 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, engine.EngineTerraform)) + 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 := b.StateFilenameTerraform(ctx) - - // Write fake state file. - testutil.WriteFile(t, path, ` - { - "version": 4, - "terraform_version": "1.5.5", - "resources": [ - { - "mode": "managed", - "type": "databricks_dashboard", - "name": "dash1", - "instances": [ - { - "schema_version": 0, - "attributes": { - "etag": "1000", - "id": "id1" - } - } - ] - }, - { - "mode": "managed", - "type": "databricks_job", - "name": "job", - "instances": [ - { - "schema_version": 0, - "attributes": { - "id": "1234" - } - } - ] - }, - { - "mode": "managed", - "type": "databricks_dashboard", - "name": "dash2", - "instances": [ - { - "schema_version": 0, - "attributes": { - "etag": "1001", - "id": "id2" - } - } - ] - } - ] - } - `) +func openDashboardState(t *testing.T, b *bundle.Bundle, data dstate.Database) { + b.DeploymentBundle.StateDB.OpenWithData(filepath.Join(t.TempDir(), "resources.json"), data) +} + +func writeFakeDashboardState(t *testing.T, b *bundle.Bundle) { + data := dstate.NewDatabase("lineage", 1) + data.State["resources.dashboards.dash1"] = dstate.ResourceEntry{ID: "id1", State: json.RawMessage(`{"etag": "1000"}`)} + data.State["resources.jobs.job"] = dstate.ResourceEntry{ID: "1234", State: json.RawMessage(`{}`)} + data.State["resources.dashboards.dash2"] = dstate.ResourceEntry{ID: "id2", State: json.RawMessage(`{"etag": "1001"}`)} + openDashboardState(t, b, data) } diff --git a/bundle/deploy/terraform/import.go b/bundle/deploy/terraform/import.go deleted file mode 100644 index c15b486bf2d..00000000000 --- a/bundle/deploy/terraform/import.go +++ /dev/null @@ -1,8 +0,0 @@ -package terraform - -type BindOptions struct { - AutoApprove bool - ResourceType string - ResourceKey string - ResourceId string -} diff --git a/bundle/phases/bind.go b/bundle/phases/bind.go index 383feb3417b..d8ff6aeaf3e 100644 --- a/bundle/phases/bind.go +++ b/bundle/phases/bind.go @@ -11,7 +11,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deploy/lock" - "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/statemgmt" "github.com/databricks/cli/libs/agent" @@ -20,7 +19,15 @@ import ( "github.com/databricks/cli/libs/logdiag" ) -func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions, stateDesc *statemgmt.StateDesc) { +type BindOptions struct { + AutoApprove bool + // Group is the bundle resource group, e.g. "jobs". + Group string + ResourceKey string + ResourceId string +} + +func Bind(ctx context.Context, b *bundle.Bundle, opts *BindOptions, stateDesc *statemgmt.StateDesc) { log.Info(ctx, "Phase: bind") engine := stateDesc.Engine @@ -46,11 +53,7 @@ func Bind(ctx context.Context, b *bundle.Bundle, opts *terraform.BindOptions, st } // Direct engine: import into temp state, run plan, check for changes. - groupName, ok := terraform.TerraformToGroupName[opts.ResourceType] - if !ok { - groupName = opts.ResourceType - } - resourceKey := fmt.Sprintf("resources.%s.%s", groupName, opts.ResourceKey) + resourceKey := fmt.Sprintf("resources.%s.%s", opts.Group, opts.ResourceKey) _, statePath := b.StateFilenameDirect(ctx) result, err := b.DeploymentBundle.Bind(ctx, b.WorkspaceClient(ctx), &b.Config, statePath, resourceKey, opts.ResourceId) @@ -116,7 +119,7 @@ func jsonDump(ctx context.Context, v any, field string) string { return string(b) } -func Unbind(ctx context.Context, b *bundle.Bundle, tfResourceType, resourceKey string, engine engine.EngineType) { +func Unbind(ctx context.Context, b *bundle.Bundle, group, resourceKey string, engine engine.EngineType) { log.Info(ctx, "Phase: unbind") bundle.ApplyContext(ctx, b, lock.Acquire(lock.GoalUnbind)) @@ -135,11 +138,7 @@ func Unbind(ctx context.Context, b *bundle.Bundle, tfResourceType, resourceKey s return } - groupName, ok := terraform.TerraformToGroupName[tfResourceType] - if !ok { - groupName = tfResourceType - } - fullResourceKey := fmt.Sprintf("resources.%s.%s", groupName, resourceKey) + fullResourceKey := fmt.Sprintf("resources.%s.%s", group, resourceKey) _, statePath := b.StateFilenameDirect(ctx) err := b.DeploymentBundle.Unbind(ctx, statePath, fullResourceKey) if err != nil { diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index d96f7603b30..39583c97cbd 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -76,7 +76,7 @@ func approvalForDeploy(ctx context.Context, b *bundle.Bundle, plan *deployplan.P return cmdio.AskYesOrNo(ctx, "Would you like to proceed?") } -func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, stateEngine engine.EngineType) { +func deployCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) { // Apply resources and capture post-apply state. Finalize flushes the WAL to disk and // returns the state; called even if Apply failed so partial progress is saved. b.DeploymentBundle.Apply(ctx, b.WorkspaceClient(ctx), plan, reportPerResource(b)) @@ -154,10 +154,9 @@ func uploadLibraries(ctx context.Context, b *bundle.Bundle, libs map[string][]li // The deploy phase deploys artifacts and resources. // If readPlanPath is provided, the plan is loaded from that file instead of being calculated. -// stateEngine is the engine the resolved state file uses; requestedEngine is -// what bundle.engine / DATABRICKS_BUNDLE_ENGINE asked for and may differ (used -// only by the post-deploy migration check). -func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHandler, stateEngine engine.EngineType, requestedEngine engine.EngineSetting, libs map[string][]libraries.LocationToUpdate, plan *deployplan.Plan, dmsDeployment *bundledeployments.Deployment) { +// requestedEngine is what bundle.engine / DATABRICKS_BUNDLE_ENGINE asked for (used only +// by the migration telemetry in CommitMigration). +func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHandler, requestedEngine engine.EngineSetting, libs map[string][]libraries.LocationToUpdate, plan *deployplan.Plan, dmsDeployment *bundledeployments.Deployment) { log.Info(ctx, "Phase: deploy") // Core mutators that CRUD resources and modify deployment state. These @@ -183,10 +182,6 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand }() immutable := b.IsImmutableFolder() - if immutable && !stateEngine.IsDirect() { - logdiag.LogError(ctx, errors.New("experimental.immutable_folder is only supported with the direct deployment engine")) - return - } if !immutable { if plan != nil { @@ -256,7 +251,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand if plan == nil { // State is already open for read by process.go (or by Migrate) - plan = RunPlan(ctx, b, stateEngine) + plan = RunPlan(ctx, b) } // Stop before opening the WAL for write if planning failed. UpgradeToWrite @@ -302,14 +297,12 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand } } - if stateEngine.IsDirect() { - // Upgrade from read (opened by process.go, or by Migrate) to write mode. After approval - // and the migration commit above, so a declined deploy never opens a WAL it must discard, - // and the migration's tf+1 push lands before this advances the WAL header to tf+2. - if err := b.DeploymentBundle.StateDB.UpgradeToWrite(); err != nil { - logdiag.LogError(ctx, err) - return - } + // Upgrade from read (opened by process.go, or by Migrate) to write mode. After approval + // and the migration commit above, so a declined deploy never opens a WAL it must discard, + // and the migration's tf+1 push lands before this advances the WAL header to tf+2. + if err := b.DeploymentBundle.StateDB.UpgradeToWrite(); err != nil { + logdiag.LogError(ctx, err) + return } if planFromFile { @@ -330,8 +323,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand // Create the deployment now that the plan is approved, so a declined deploy leaves none behind. // A first deploy's id did not exist at plan time - the version and any existing id were stamped // then - so stamp the one just created into the plan the apply reads. - // IsDirect first: the state must be open to read its features, and only the direct engine opens it. - if stateEngine.IsDirect() && b.DeploymentBundle.StateDB.IsDeploymentMetadataService() { + if b.DeploymentBundle.StateDB.IsDeploymentMetadataService() { firstDeploy := b.DeploymentBundle.StateDB.DeploymentID == "" createOrUpdateDeployment(ctx, b, dmsDeployment) if logdiag.HasError(ctx) { @@ -360,7 +352,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand } } - deployCore(ctx, b, plan, stateEngine) + deployCore(ctx, b, plan) if logdiag.HasError(ctx) { return @@ -379,7 +371,7 @@ func Deploy(ctx context.Context, b *bundle.Bundle, outputHandler sync.OutputHand bundle.ApplyContext(ctx, b, scripts.Execute(config.ScriptPostDeploy)) } -func RunPlan(ctx context.Context, b *bundle.Bundle, engine engine.EngineType) *deployplan.Plan { +func RunPlan(ctx context.Context, b *bundle.Bundle) *deployplan.Plan { plan, err := b.DeploymentBundle.CalculatePlan(ctx, b.WorkspaceClient(ctx), &b.Config) if err != nil { logdiag.LogError(ctx, err) diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index 6db62c9ff01..d8ed0f75c02 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -11,7 +11,6 @@ import ( "slices" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deploy/files" "github.com/databricks/cli/bundle/deploy/lock" "github.com/databricks/cli/bundle/deployplan" @@ -51,12 +50,12 @@ var destroyApprovalGroups = []approvalGroup{ // logPipelineDeleteApproval prints the pipeline deletions. If cascade_on_destroy is true, we will include // a note that datasets will be deleted as well. -func logPipelineDeleteApproval(ctx context.Context, b *bundle.Bundle, actions []deployplan.Action, engine engine.EngineType, quiet bool) error { +func logPipelineDeleteApproval(ctx context.Context, b *bundle.Bundle, actions []deployplan.Action, quiet bool) error { pipelineDeletes := filterGroup(actions, "pipelines", deployplan.Delete) var cascading, retaining []deployplan.Action for _, a := range pipelineDeletes { - cascade, err := pipelineDeletionCascades(b, a, engine) + cascade, err := pipelineDeletionCascades(b, a) if err != nil { return err } @@ -86,7 +85,7 @@ func logPipelineDeleteApproval(ctx context.Context, b *bundle.Bundle, actions [] return nil } -func approvalForDestroy(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, engine engine.EngineType) (bool, error) { +func approvalForDestroy(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) (bool, error) { deleteActions := plan.GetActions() // Deletes that only clean up the state (already gone remotely, or no delete @@ -122,7 +121,7 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle, plan *deployplan. logApprovalGroups(ctx, deleteActions, destroyApprovalGroups, true, deployplan.Delete) } // Called even when quiet: the cascade lookup can fail, and that error must surface. - if err := logPipelineDeleteApproval(ctx, b, deleteActions, engine, quiet); err != nil { + if err := logPipelineDeleteApproval(ctx, b, deleteActions, quiet); err != nil { return false, err } @@ -138,7 +137,7 @@ func approvalForDestroy(ctx context.Context, b *bundle.Bundle, plan *deployplan. return cmdio.AskYesOrNo(ctx, "Would you like to proceed?") } -func destroyCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, engine engine.EngineType) { +func destroyCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) { // Not reported per resource: destroy names them up front for consent and then // reports only a count, so there is no per-resource output to report into. b.DeploymentBundle.Apply(ctx, b.WorkspaceClient(ctx), plan, false) @@ -270,7 +269,7 @@ func removeEmptyDirs(dir string, depth int) (bool, error) { } // The destroy phase deletes artifacts and resources. -func Destroy(ctx context.Context, b *bundle.Bundle, engine engine.EngineType) { +func Destroy(ctx context.Context, b *bundle.Bundle) { log.Info(ctx, "Phase: destroy") ok, err := assertRootPathExists(ctx, b) @@ -313,7 +312,7 @@ func Destroy(ctx context.Context, b *bundle.Bundle, engine engine.EngineType) { return } - hasApproval, err := approvalForDestroy(ctx, b, plan, engine) + hasApproval, err := approvalForDestroy(ctx, b, plan) if err != nil { logdiag.LogError(ctx, err) return @@ -364,7 +363,7 @@ func Destroy(ctx context.Context, b *bundle.Bundle, engine engine.EngineType) { return } } - destroyCore(ctx, b, plan, engine) + destroyCore(ctx, b, plan) } else { // A prepared terraform→direct migration lives only in memory (nothing durable was // written), so a declined destroy just drops it and leaves the terraform state on diff --git a/bundle/phases/destroy_test.go b/bundle/phases/destroy_test.go index 4c770ab531a..c3294e5d878 100644 --- a/bundle/phases/destroy_test.go +++ b/bundle/phases/destroy_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" @@ -37,7 +36,7 @@ func TestApprovalForDestroyQuietWhilePrompting(t *testing.T) { b := &bundle.Bundle{Quiet: bundle.QuietAll} b.Config.Workspace.RootPath = "/Workspace/Users/me/.bundle/x" - approved, err := approvalForDestroy(ctx, b, plan, engine.EngineTerraform) + approved, err := approvalForDestroy(ctx, b, plan) require.NoError(t, err) assert.True(t, approved) diff --git a/bundle/phases/plan.go b/bundle/phases/plan.go index 3446690fbdd..4be81a102d2 100644 --- a/bundle/phases/plan.go +++ b/bundle/phases/plan.go @@ -7,11 +7,9 @@ import ( "fmt" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/mutator/resourcemutator" "github.com/databricks/cli/bundle/deploy" - "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/bundle/statemgmt" @@ -20,13 +18,13 @@ import ( // PreDeployChecks is common set of mutators between "bundle plan" and "bundle deploy". // Note, it is not run in "bundle migrate" so it must not modify the config -func PreDeployChecks(ctx context.Context, b *bundle.Bundle, isPlan bool, engine engine.EngineType) { +func PreDeployChecks(ctx context.Context, b *bundle.Bundle, isPlan bool) { bundle.ApplySeqContext(ctx, b, - terraform.CheckDashboardsModifiedRemotely(isPlan, engine), - resourcemutator.SecretScopeFixups(engine), + deploy.CheckDashboardsModifiedRemotely(isPlan), + resourcemutator.SecretScopeFixups(), deploy.StatePull(), mutator.ValidateGitDetails(), - statemgmt.CheckRunningResource(engine), + statemgmt.CheckRunningResource(), ) } @@ -34,14 +32,8 @@ func PreDeployChecks(ctx context.Context, b *bundle.Bundle, isPlan bool, engine // also deletes its datasets (MVs, STs, Views). This is the server default (cascade) unless // cascade_on_destroy is explicitly set to false. // -// Currently, this feature is only supported by the direct engine. We will read from the persisted -// state to determine the value. For the Terraform engine, this parameter cannot be configured, so -// there is no state to read from and we return the default of true. -func pipelineDeletionCascades(b *bundle.Bundle, action deployplan.Action, engine engine.EngineType) (bool, error) { - if !engine.IsDirect() { - return true, nil - } - +// The value is read from the persisted state. +func pipelineDeletionCascades(b *bundle.Bundle, action deployplan.Action) (bool, error) { entry, ok := b.DeploymentBundle.StateDB.GetResourceEntry(action.ResourceKey) if !ok || len(entry.State) == 0 { return true, nil diff --git a/bundle/phases/plan_test.go b/bundle/phases/plan_test.go index dd95de77542..772f7401f9b 100644 --- a/bundle/phases/plan_test.go +++ b/bundle/phases/plan_test.go @@ -7,7 +7,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/direct/dstate" "github.com/databricks/cli/libs/dyn" @@ -25,59 +24,42 @@ func TestPipelineDeletionCascades(t *testing.T) { data.State["resources.pipelines.cascade_empty"] = dstate.ResourceEntry{ID: "4"} b.DeploymentBundle.StateDB.OpenWithData(filepath.Join(t.TempDir(), "resources.json"), data) - // A bundle whose direct state DB was never opened, as happens under the terraform - // engine. pipelineDeletionCascades must not touch the state DB there. - bTerraform := &bundle.Bundle{} - tests := []struct { name string bundle *bundle.Bundle - engine engine.EngineType resourceKey string want bool }{ { name: "cascade_on_destroy explicitly true", bundle: b, - engine: engine.EngineDirect, resourceKey: "resources.pipelines.cascade_true", want: true, }, { name: "cascade_on_destroy explicitly false", bundle: b, - engine: engine.EngineDirect, resourceKey: "resources.pipelines.cascade_false", want: false, }, { name: "cascade_on_destroy unset defaults to cascade", bundle: b, - engine: engine.EngineDirect, resourceKey: "resources.pipelines.cascade_unset", want: true, }, { name: "empty persisted state defaults to cascade", bundle: b, - engine: engine.EngineDirect, resourceKey: "resources.pipelines.cascade_empty", want: true, }, { name: "missing state entry defaults to cascade", bundle: b, - engine: engine.EngineDirect, resourceKey: "resources.pipelines.does_not_exist", want: true, }, - { - name: "terraform engine defaults to cascade without reading state", - bundle: bTerraform, - engine: engine.EngineTerraform, - resourceKey: "resources.pipelines.cascade_false", - want: true, - }, } for _, tt := range tests { @@ -86,7 +68,7 @@ func TestPipelineDeletionCascades(t *testing.T) { ResourceKey: tt.resourceKey, ActionType: deployplan.Delete, } - cascade, err := pipelineDeletionCascades(tt.bundle, action, tt.engine) + cascade, err := pipelineDeletionCascades(tt.bundle, action) require.NoError(t, err) assert.Equal(t, tt.want, cascade) }) diff --git a/bundle/statemgmt/check_running_resources.go b/bundle/statemgmt/check_running_resources.go index 2ad5adf6da8..63681bd5eb9 100644 --- a/bundle/statemgmt/check_running_resources.go +++ b/bundle/statemgmt/check_running_resources.go @@ -7,8 +7,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/engine" - "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" @@ -26,9 +24,7 @@ func (e ErrResourceIsRunning) Error() string { return fmt.Sprintf("%s %s is running", e.resourceType, e.resourceId) } -type checkRunningResources struct { - engine engine.EngineType -} +type checkRunningResources struct{} func (l *checkRunningResources) Name() string { return "check-running-resources" @@ -39,28 +35,16 @@ func (l *checkRunningResources) Apply(ctx context.Context, b *bundle.Bundle) dia return nil } - var err error - var state ExportedResourcesMap - - if l.engine.IsDirect() { - state = b.DeploymentBundle.ExportState(ctx) - } else { - state, err = terraform.ParseResourcesState(ctx, b) - if err != nil { - return diag.FromErr(err) - } - } - w := b.WorkspaceClient(ctx) - err = checkAnyResourceRunning(ctx, w, state) + err := checkAnyResourceRunning(ctx, w, b.DeploymentBundle.ExportState(ctx)) if err != nil { return diag.FromErr(err) } return nil } -func CheckRunningResource(engine engine.EngineType) bundle.Mutator { - return &checkRunningResources{engine: engine} +func CheckRunningResource() bundle.Mutator { + return &checkRunningResources{} } func checkAnyResourceRunning(ctx context.Context, w *databricks.WorkspaceClient, state ExportedResourcesMap) error { diff --git a/bundle/statemgmt/direct_migration.go b/bundle/statemgmt/direct_migration.go index a84b5c1f804..d4772e62f80 100644 --- a/bundle/statemgmt/direct_migration.go +++ b/bundle/statemgmt/direct_migration.go @@ -271,7 +271,7 @@ func convertTFStateToDirect(ctx context.Context, b *bundle.Bundle, tfState *migr // Apply SecretScopeFixups so the config matches what the direct engine expects. // This adds MANAGE ACL for the current user to all secret scopes, ensuring // the migrated state and config agree on .permissions entries. - bundle.ApplyContext(ctx, b, resourcemutator.SecretScopeFixups(engine.EngineDirect)) + bundle.ApplyContext(ctx, b, resourcemutator.SecretScopeFixups()) if logdiag.HasError(ctx) { return tempStatePath, false, nil, errors.New("failed to apply secret scope fixups") } diff --git a/cmd/apps/import.go b/cmd/apps/import.go index ea3fce67e93..079c34c274f 100644 --- a/cmd/apps/import.go +++ b/cmd/apps/import.go @@ -15,7 +15,6 @@ import ( "go.yaml.in/yaml/v3" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/generate" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/bundle/resources" @@ -331,15 +330,11 @@ func runImport(ctx context.Context, w *databricks.WorkspaceClient, appName, outp } // Bind the resource - tfName, ok := terraform.GroupToTerraformName[resource.ResourceDescription().PluralName] - if !ok { - tfName = resource.ResourceDescription().PluralName - } - phases.Bind(ctx, b, &terraform.BindOptions{ - AutoApprove: true, - ResourceType: tfName, - ResourceKey: appKey, - ResourceId: app.Name, + phases.Bind(ctx, b, &phases.BindOptions{ + AutoApprove: true, + Group: resource.ResourceDescription().PluralName, + ResourceKey: appKey, + ResourceId: app.Name, }, stateDesc) if logdiag.HasError(ctx) { return errors.New("failed to bind resource") diff --git a/cmd/bundle/config_remote_sync.go b/cmd/bundle/config_remote_sync.go index 6e562094faf..8c8b270f2f0 100644 --- a/cmd/bundle/config_remote_sync.go +++ b/cmd/bundle/config_remote_sync.go @@ -93,7 +93,7 @@ Examples: return fmt.Errorf("failed to detect changes: %w", err) } - detected, err := configsync.ExtractChanges(ctx, b, plan, stateDesc.Engine) + detected, err := configsync.ExtractChanges(ctx, b, plan) if err != nil { stats.ErrorCategory = protos.BundleConfigRemoteSyncErrorCategoryDetectChangesFailed return fmt.Errorf("failed to extract changes: %w", err) diff --git a/cmd/bundle/deployment/bind_resource.go b/cmd/bundle/deployment/bind_resource.go index 1a3476775c4..ef38c972362 100644 --- a/cmd/bundle/deployment/bind_resource.go +++ b/cmd/bundle/deployment/bind_resource.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" @@ -43,15 +42,11 @@ func BindResource(cmd *cobra.Command, resourceKey, resourceId string, autoApprov return fmt.Errorf("%s with an id '%s' is not found", resource.ResourceDescription().SingularName, resourceId) } - tfName, ok := terraform.GroupToTerraformName[resource.ResourceDescription().PluralName] - if !ok { - tfName = resource.ResourceDescription().PluralName - } - phases.Bind(ctx, b, &terraform.BindOptions{ - AutoApprove: autoApprove, - ResourceType: tfName, - ResourceKey: resourceKey, - ResourceId: resourceId, + phases.Bind(ctx, b, &phases.BindOptions{ + AutoApprove: autoApprove, + Group: resource.ResourceDescription().PluralName, + ResourceKey: resourceKey, + ResourceId: resourceId, }, stateDesc) if logdiag.HasError(ctx) { return root.ErrAlreadyPrinted diff --git a/cmd/bundle/deployment/migrate.go b/cmd/bundle/deployment/migrate.go index 3878370d877..be7b07c9a67 100644 --- a/cmd/bundle/deployment/migrate.go +++ b/cmd/bundle/deployment/migrate.go @@ -151,7 +151,7 @@ To start using direct engine, set "engine: direct" under bundle in your databric // Apply SecretScopeFixups so the config matches what the direct engine expects. // This adds MANAGE ACL for the current user to all secret scopes, ensuring // the migrated state and config agree on .permissions entries. - bundle.ApplyContext(ctx, b, resourcemutator.SecretScopeFixups(engine.EngineDirect)) + bundle.ApplyContext(ctx, b, resourcemutator.SecretScopeFixups()) if logdiag.HasError(ctx) { return root.ErrAlreadyPrinted } diff --git a/cmd/bundle/deployment/unbind.go b/cmd/bundle/deployment/unbind.go index 132022f0ede..585767676ef 100644 --- a/cmd/bundle/deployment/unbind.go +++ b/cmd/bundle/deployment/unbind.go @@ -2,7 +2,6 @@ package deployment import ( "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" @@ -68,12 +67,7 @@ To re-bind the resource later, use: return err } - rd := resource.ResourceDescription() - tfName, ok := terraform.GroupToTerraformName[rd.PluralName] - if !ok { - tfName = rd.PluralName - } - phases.Unbind(ctx, b, tfName, args[0], stateDesc.Engine) + phases.Unbind(ctx, b, resource.ResourceDescription().PluralName, args[0], stateDesc.Engine) if logdiag.HasError(ctx) { return root.ErrAlreadyPrinted } diff --git a/cmd/bundle/destroy.go b/cmd/bundle/destroy.go index 90dec11a1ea..4819ab6b306 100644 --- a/cmd/bundle/destroy.go +++ b/cmd/bundle/destroy.go @@ -82,7 +82,7 @@ func CommandBundleDestroy(cmd *cobra.Command, args []string, autoApprove, forceD AlwaysPull: true, SkipEnforcingDeploymentHistorySetting: true, PostStateFunc: func(ctx context.Context, b *bundle.Bundle, stateDesc *statemgmt.StateDesc) error { - phases.Destroy(ctx, b, stateDesc.Engine) + phases.Destroy(ctx, b) if logdiag.HasError(ctx) { return root.ErrAlreadyPrinted } diff --git a/cmd/bundle/plan.go b/cmd/bundle/plan.go index c5d01c6a98d..1132834bc4d 100644 --- a/cmd/bundle/plan.go +++ b/cmd/bundle/plan.go @@ -58,7 +58,7 @@ It is useful for previewing changes before running 'bundle deploy'.`, }, } - b, stateDesc, err := utils.ProcessBundleRet(cmd, opts) + b, _, err := utils.ProcessBundleRet(cmd, opts) if err != nil { return err } @@ -70,7 +70,7 @@ It is useful for previewing changes before running 'bundle deploy'.`, return root.ErrAlreadyPrinted } } - plan := phases.RunPlan(ctx, b, stateDesc.Engine) + plan := phases.RunPlan(ctx, b) if logdiag.HasError(ctx) { return root.ErrAlreadyPrinted } diff --git a/cmd/bundle/utils/process.go b/cmd/bundle/utils/process.go index f199d982c71..aa7ce653a09 100644 --- a/cmd/bundle/utils/process.go +++ b/cmd/bundle/utils/process.go @@ -465,7 +465,7 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle if opts.PreDeployChecks { downgradeWarningToError := !opts.Deploy - phases.PreDeployChecks(ctx, b, downgradeWarningToError, stateDesc.Engine) + phases.PreDeployChecks(ctx, b, downgradeWarningToError) if logdiag.HasError(ctx) { return b, stateDesc, root.ErrAlreadyPrinted @@ -503,7 +503,7 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle } t3 := time.Now() - phases.Deploy(ctx, b, outputHandler, stateDesc.Engine, requiredEngine, libs, plan, dmsDeployment) + phases.Deploy(ctx, b, outputHandler, requiredEngine, libs, plan, dmsDeployment) b.Metrics.ExecutionTimes = append(b.Metrics.ExecutionTimes, protos.IntMapEntry{ Key: "phases.Deploy", Value: time.Since(t3).Milliseconds(),