Skip to content

Commit 4e9d99e

Browse files
denikIsaac
denik
and
Isaac
committed
bundle: always error on references to Terraform-only fields
The warning branch of validate.TFOnlyReferences was only reachable with DATABRICKS_BUNDLE_ENGINE=terraform (or bundle.engine: terraform), and such commands fail with the engine removal error anyway. Drop the engine lookup and always report an error. Reject an invalid or removed DATABRICKS_BUNDLE_ENGINE before initialization so the removal error is the first thing users see. bundle.engine: terraform is still reported by validate.ValidateEngine, which runs before TFOnlyReferences. Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 7eb7fef commit 4e9d99e

6 files changed

Lines changed: 34 additions & 52 deletions

File tree

‎acceptance/bundle/resource_deps/tf_path_only_error/out.validate-terraform-env.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Error: the Terraform deployment engine has been removed in Databricks CLI v1.20.0
2+
3+
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.
4+
See https://docs.databricks.com/dev-tools/bundles/direct for details
5+
6+
Name: test-bundle
7+
Target: default
8+
9+
Found 1 error

‎acceptance/bundle/resource_deps/tf_path_only_error/script‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,6 @@ errcode $CLI bundle plan > out.plan.txt 2>&1
33
errcode $CLI bundle deploy > out.deploy.txt 2>&1
44
print_telemetry_bool_values > out.telemetry.txt
55
rm "$OUT_REQUESTS"
6+
7+
# The engine removal error comes before the Terraform-only reference error.
8+
DATABRICKS_BUNDLE_ENGINE=terraform musterr $CLI bundle validate > out.validate-terraform-env.txt 2>&1

‎bundle/config/validate/tf_only_references.go‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"strings"
77

88
"github.com/databricks/cli/bundle"
9-
"github.com/databricks/cli/bundle/config/engine"
109
"github.com/databricks/cli/bundle/terraform_dabs_map"
1110
"github.com/databricks/cli/libs/diag"
1211
"github.com/databricks/cli/libs/dyn"
@@ -18,10 +17,7 @@ type tfOnlyReferences struct{}
1817

1918
// TFOnlyReferences validates that no cross-resource references point to
2019
// Terraform-only fields (fields that exist in TF schema but have no DABs equivalent).
21-
//
22-
// In direct mode this is an error because the direct engine cannot resolve such
23-
// references at deploy time. In Terraform mode it is a warning because the
24-
// reference will break if the bundle is later migrated to the direct engine.
20+
// The direct engine cannot resolve such references at deploy time.
2521
func TFOnlyReferences() bundle.Mutator {
2622
return &tfOnlyReferences{}
2723
}
@@ -30,16 +26,7 @@ func (m *tfOnlyReferences) Name() string {
3026
return "validate:tf_only_references"
3127
}
3228

33-
func (m *tfOnlyReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
34-
// Resolve effective engine: config takes precedence over env var.
35-
effectiveEngine := b.Config.Bundle.Engine
36-
if effectiveEngine == engine.EngineNotSet {
37-
if envEngine, err := engine.FromEnv(ctx); err == nil {
38-
effectiveEngine = envEngine
39-
}
40-
}
41-
isDirect := effectiveEngine.IsDirect()
42-
29+
func (m *tfOnlyReferences) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
4330
var diags diag.Diagnostics
4431

4532
// Walk the entire config looking for ${resources.*} references.
@@ -52,7 +39,7 @@ func (m *tfOnlyReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
5239
if !strings.HasPrefix(r, "resources.") {
5340
continue
5441
}
55-
if d := checkTFOnlyReference(r, v.Location(), isDirect); d != nil {
42+
if d := checkTFOnlyReference(r, v.Location()); d != nil {
5643
diags = append(diags, *d)
5744
}
5845
}
@@ -69,7 +56,7 @@ func (m *tfOnlyReferences) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
6956
// checkTFOnlyReference checks a single reference string like
7057
// "resources.jobs.src.always_running" and returns a diagnostic when it refers
7158
// to a TF-only field, or nil otherwise.
72-
func checkTFOnlyReference(ref string, loc dyn.Location, isDirect bool) *diag.Diagnostic {
59+
func checkTFOnlyReference(ref string, loc dyn.Location) *diag.Diagnostic {
7360
p, err := dyn.NewPathFromString(ref)
7461
// Need at least resources.<group>.<name>.<field>
7562
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
9279
return nil
9380
}
9481

95-
if isDirect {
96-
return &diag.Diagnostic{
97-
Severity: diag.Error,
98-
Summary: fmt.Sprintf("%q: Terraform-only field; cross-resource references to Terraform-only fields are not supported by the direct engine", ref),
99-
Locations: []dyn.Location{loc},
100-
}
101-
}
102-
10382
return &diag.Diagnostic{
104-
Severity: diag.Warning,
105-
Summary: fmt.Sprintf("%q: Terraform-only field; this reference will not work with the direct engine", ref),
83+
Severity: diag.Error,
84+
Summary: fmt.Sprintf("%q: Terraform-only field; cross-resource references to Terraform-only fields are not supported by the direct engine", ref),
10685
Locations: []dyn.Location{loc},
10786
}
10887
}

‎bundle/config/validate/tf_only_references_test.go‎

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,10 @@ import (
55

66
"github.com/databricks/cli/bundle"
77
"github.com/databricks/cli/bundle/config"
8-
"github.com/databricks/cli/bundle/config/engine"
98
"github.com/databricks/cli/bundle/config/resources"
109
"github.com/databricks/cli/bundle/internal/bundletest"
1110
"github.com/databricks/cli/libs/diag"
1211
"github.com/databricks/cli/libs/dyn"
13-
"github.com/databricks/cli/libs/env"
1412
"github.com/databricks/databricks-sdk-go/service/jobs"
1513
"github.com/stretchr/testify/assert"
1614
"github.com/stretchr/testify/require"
@@ -31,43 +29,27 @@ func makeBundle(t *testing.T) *bundle.Bundle {
3129
return b
3230
}
3331

34-
func TestTFOnlyReferences_DirectError(t *testing.T) {
32+
func TestTFOnlyReferences_Error(t *testing.T) {
3533
b := makeBundle(t)
3634
bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) {
3735
return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.always_running}"))
3836
})
3937

40-
ctx := env.Set(t.Context(), engine.EnvVar, "direct")
41-
diags := TFOnlyReferences().Apply(ctx, b)
38+
diags := TFOnlyReferences().Apply(t.Context(), b)
4239
require.Len(t, diags, 1)
4340
assert.Equal(t, diag.Error, diags[0].Severity)
4441
assert.Contains(t, diags[0].Summary, "resources.jobs.src.always_running")
4542
assert.Contains(t, diags[0].Summary, "Terraform-only field")
4643
}
4744

48-
func TestTFOnlyReferences_TerraformWarning(t *testing.T) {
49-
b := makeBundle(t)
50-
bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) {
51-
return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.always_running}"))
52-
})
53-
54-
ctx := env.Set(t.Context(), engine.EnvVar, "terraform")
55-
diags := TFOnlyReferences().Apply(ctx, b)
56-
require.Len(t, diags, 1)
57-
assert.Equal(t, diag.Warning, diags[0].Severity)
58-
assert.Contains(t, diags[0].Summary, "resources.jobs.src.always_running")
59-
assert.Contains(t, diags[0].Summary, "Terraform-only field")
60-
}
61-
6245
func TestTFOnlyReferences_NormalReference(t *testing.T) {
6346
b := makeBundle(t)
6447
bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) {
6548
// "name" is not a TF-only field; no diagnostic expected.
6649
return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.name}"))
6750
})
6851

69-
ctx := env.Set(t.Context(), engine.EnvVar, "direct")
70-
diags := TFOnlyReferences().Apply(ctx, b)
52+
diags := TFOnlyReferences().Apply(t.Context(), b)
7153
assert.Empty(t, diags)
7254
}
7355

@@ -78,7 +60,6 @@ func TestTFOnlyReferences_RenamedField(t *testing.T) {
7860
return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.git_source[0].branch}"))
7961
})
8062

81-
ctx := env.Set(t.Context(), engine.EnvVar, "direct")
82-
diags := TFOnlyReferences().Apply(ctx, b)
63+
diags := TFOnlyReferences().Apply(t.Context(), b)
8364
assert.Empty(t, diags)
8465
}

‎bundle/phases/initialize.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ func Initialize(ctx context.Context, b *bundle.Bundle) {
201201
mutator.ValidateJobRunTriggers(),
202202

203203
// Reads (dynamic): * (strings) (searches for ${resources.*} references)
204-
// Warns (TF engine) or errors (direct engine) when a cross-resource reference
205-
// points to a Terraform-only field with no DABs equivalent.
204+
// Errors when a cross-resource reference points to a Terraform-only field
205+
// with no DABs equivalent.
206206
validate.TFOnlyReferences(),
207207

208208
// Reads (typed): b.Config.Permissions (checks if current user or their groups have CAN_MANAGE permissions)

‎cmd/bundle/utils/process.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,16 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle
166166
cmd.SetContext(ctx)
167167
}
168168

169+
// Reject an invalid or removed DATABRICKS_BUNDLE_ENGINE before initialization, so
170+
// the removal error comes before any initialize-time validation (such as
171+
// validate.TFOnlyReferences). A terraform bundle.engine is reported during
172+
// initialization by validate.ValidateEngine, with its location.
173+
if b.Config.Bundle.Engine == engine.EngineNotSet {
174+
if _, err := ResolveEngineSetting(ctx, b); err != nil {
175+
return b, nil, err
176+
}
177+
}
178+
169179
if !opts.SkipInitialize {
170180
t0 := time.Now()
171181
phases.Initialize(ctx, b)

0 commit comments

Comments
 (0)