diff --git a/acceptance/bundle/resource_deps/tf_path_only_error/out.validate-terraform-env.txt b/acceptance/bundle/resource_deps/tf_path_only_error/out.validate-terraform-env.txt new file mode 100644 index 00000000000..054469a2e1f --- /dev/null +++ b/acceptance/bundle/resource_deps/tf_path_only_error/out.validate-terraform-env.txt @@ -0,0 +1,9 @@ +Error: the Terraform deployment engine has been removed in Databricks CLI v1.20.0 + +Remove the "bundle.engine" setting (or set it to "direct") to deploy with the direct engine; existing Terraform state is migrated automatically. To keep using Terraform, revert to Databricks CLI v1.19.x. +See https://docs.databricks.com/dev-tools/bundles/direct for details + +Name: test-bundle +Target: default + +Found 1 error diff --git a/acceptance/bundle/resource_deps/tf_path_only_error/script b/acceptance/bundle/resource_deps/tf_path_only_error/script index 3a32a4f1e31..4387cf756d9 100644 --- a/acceptance/bundle/resource_deps/tf_path_only_error/script +++ b/acceptance/bundle/resource_deps/tf_path_only_error/script @@ -3,3 +3,6 @@ errcode $CLI bundle plan > out.plan.txt 2>&1 errcode $CLI bundle deploy > out.deploy.txt 2>&1 print_telemetry_bool_values > out.telemetry.txt rm "$OUT_REQUESTS" + +# The engine removal error comes before the Terraform-only reference error. +DATABRICKS_BUNDLE_ENGINE=terraform musterr $CLI bundle validate > out.validate-terraform-env.txt 2>&1 diff --git a/bundle/config/validate/tf_only_references.go b/bundle/config/validate/tf_only_references.go index 90de6d12468..60eb87e4249 100644 --- a/bundle/config/validate/tf_only_references.go +++ b/bundle/config/validate/tf_only_references.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config/engine" "github.com/databricks/cli/bundle/terraform_dabs_map" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" @@ -18,10 +17,7 @@ type tfOnlyReferences struct{} // TFOnlyReferences validates that no cross-resource references point to // Terraform-only fields (fields that exist in TF schema but have no DABs equivalent). -// -// In direct mode this is an error because the direct engine cannot resolve such -// references at deploy time. In Terraform mode it is a warning because the -// reference will break if the bundle is later migrated to the direct engine. +// The direct engine cannot resolve such references at deploy time. func TFOnlyReferences() bundle.Mutator { return &tfOnlyReferences{} } @@ -30,16 +26,7 @@ func (m *tfOnlyReferences) Name() string { return "validate:tf_only_references" } -func (m *tfOnlyReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - // Resolve effective engine: config takes precedence over env var. - effectiveEngine := b.Config.Bundle.Engine - if effectiveEngine == engine.EngineNotSet { - if envEngine, err := engine.FromEnv(ctx); err == nil { - effectiveEngine = envEngine - } - } - isDirect := effectiveEngine.IsDirect() - +func (m *tfOnlyReferences) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { var diags diag.Diagnostics // Walk the entire config looking for ${resources.*} references. @@ -52,7 +39,7 @@ func (m *tfOnlyReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia if !strings.HasPrefix(r, "resources.") { continue } - if d := checkTFOnlyReference(r, v.Location(), isDirect); d != nil { + if d := checkTFOnlyReference(r, v.Location()); d != nil { diags = append(diags, *d) } } @@ -69,7 +56,7 @@ func (m *tfOnlyReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia // checkTFOnlyReference checks a single reference string like // "resources.jobs.src.always_running" and returns a diagnostic when it refers // to a TF-only field, or nil otherwise. -func checkTFOnlyReference(ref string, loc dyn.Location, isDirect bool) *diag.Diagnostic { +func checkTFOnlyReference(ref string, loc dyn.Location) *diag.Diagnostic { p, err := dyn.NewPathFromString(ref) // Need at least resources... if err != nil || len(p) < 4 || p[0].Key() != "resources" { @@ -92,17 +79,9 @@ func checkTFOnlyReference(ref string, loc dyn.Location, isDirect bool) *diag.Dia return nil } - if isDirect { - return &diag.Diagnostic{ - Severity: diag.Error, - Summary: fmt.Sprintf("%q: Terraform-only field; cross-resource references to Terraform-only fields are not supported by the direct engine", ref), - Locations: []dyn.Location{loc}, - } - } - return &diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("%q: Terraform-only field; this reference will not work with the direct engine", ref), + Severity: diag.Error, + Summary: fmt.Sprintf("%q: Terraform-only field; cross-resource references to Terraform-only fields are not supported by the direct engine", ref), Locations: []dyn.Location{loc}, } } diff --git a/bundle/config/validate/tf_only_references_test.go b/bundle/config/validate/tf_only_references_test.go index fdf81e3c75b..7b994cf864e 100644 --- a/bundle/config/validate/tf_only_references_test.go +++ b/bundle/config/validate/tf_only_references_test.go @@ -5,12 +5,10 @@ import ( "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/bundle/internal/bundletest" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/env" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -31,34 +29,19 @@ func makeBundle(t *testing.T) *bundle.Bundle { return b } -func TestTFOnlyReferences_DirectError(t *testing.T) { +func TestTFOnlyReferences_Error(t *testing.T) { b := makeBundle(t) bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.always_running}")) }) - ctx := env.Set(t.Context(), engine.EnvVar, "direct") - diags := TFOnlyReferences().Apply(ctx, b) + diags := TFOnlyReferences().Apply(t.Context(), b) require.Len(t, diags, 1) assert.Equal(t, diag.Error, diags[0].Severity) assert.Contains(t, diags[0].Summary, "resources.jobs.src.always_running") assert.Contains(t, diags[0].Summary, "Terraform-only field") } -func TestTFOnlyReferences_TerraformWarning(t *testing.T) { - b := makeBundle(t) - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.always_running}")) - }) - - ctx := env.Set(t.Context(), engine.EnvVar, "terraform") - diags := TFOnlyReferences().Apply(ctx, b) - require.Len(t, diags, 1) - assert.Equal(t, diag.Warning, diags[0].Severity) - assert.Contains(t, diags[0].Summary, "resources.jobs.src.always_running") - assert.Contains(t, diags[0].Summary, "Terraform-only field") -} - func TestTFOnlyReferences_NormalReference(t *testing.T) { b := makeBundle(t) bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { @@ -66,8 +49,7 @@ func TestTFOnlyReferences_NormalReference(t *testing.T) { return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.name}")) }) - ctx := env.Set(t.Context(), engine.EnvVar, "direct") - diags := TFOnlyReferences().Apply(ctx, b) + diags := TFOnlyReferences().Apply(t.Context(), b) assert.Empty(t, diags) } @@ -78,7 +60,6 @@ func TestTFOnlyReferences_RenamedField(t *testing.T) { return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.git_source[0].branch}")) }) - ctx := env.Set(t.Context(), engine.EnvVar, "direct") - diags := TFOnlyReferences().Apply(ctx, b) + diags := TFOnlyReferences().Apply(t.Context(), b) assert.Empty(t, diags) } diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 38577109b26..e9a0591fb68 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -201,8 +201,8 @@ func Initialize(ctx context.Context, b *bundle.Bundle) { mutator.ValidateJobRunTriggers(), // Reads (dynamic): * (strings) (searches for ${resources.*} references) - // Warns (TF engine) or errors (direct engine) when a cross-resource reference - // points to a Terraform-only field with no DABs equivalent. + // Errors when a cross-resource reference points to a Terraform-only field + // with no DABs equivalent. validate.TFOnlyReferences(), // Reads (typed): b.Config.Permissions (checks if current user or their groups have CAN_MANAGE permissions) diff --git a/cmd/bundle/utils/process.go b/cmd/bundle/utils/process.go index cccb1d48911..87828b018bb 100644 --- a/cmd/bundle/utils/process.go +++ b/cmd/bundle/utils/process.go @@ -167,6 +167,16 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle cmd.SetContext(ctx) } + // Reject an invalid or removed DATABRICKS_BUNDLE_ENGINE before initialization, so + // the removal error comes before any initialize-time validation (such as + // validate.TFOnlyReferences). A terraform bundle.engine is reported during + // initialization by validate.ValidateEngine, with its location. + if b.Config.Bundle.Engine == engine.EngineNotSet { + if _, err := ResolveEngineSetting(ctx, b); err != nil { + return b, nil, err + } + } + if !opts.SkipInitialize { t0 := time.Now() phases.Initialize(ctx, b) @@ -198,10 +208,11 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle } } - // Resolve engine setting up front so a garbage DATABRICKS_BUNDLE_ENGINE - // value fails every bundle command instead of only the ones that read - // state. The resolver is cheap (config lookup + env var read); no reason - // to gate it on state-touching options. + // Resolve the engine used for state and telemetry. An invalid or removed + // DATABRICKS_BUNDLE_ENGINE was already rejected before initialization and a + // bad bundle.engine by validate.ValidateEngine, so this only fails for + // commands that skip initialization (bundle run -- ) with a bad + // bundle.engine. requiredEngine, err := ResolveEngineSetting(ctx, b) if err != nil { return b, nil, err