bundle: drop engine parameters that are always direct

bundle/config/mutator/resourcemutator/secret_scope_fixups.go

@@ -7,20 +7,17 @@ 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"
 	"github.com/databricks/cli/libs/iamutil"
 	"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.

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)

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

bundle/deploy/terraform/check_dashboards_modified_remotely.go → bundle/deploy/check_dashboards_modified_remotely.go

@@ -1,12 +1,11 @@
-package terraform
+package deploy
 
 import (
 	"context"
 	"fmt"
 	"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}
 }

bundle/deploy/terraform/check_dashboards_modified_remotely_test.go → 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,21 +51,22 @@ 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)
 }
 
 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,15 +81,15 @@ 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)
 }
 
 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)
 }