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(),