From 925f4b3d74c413479dd732228578b0e429470034 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 8 Apr 2026 15:19:39 +0200 Subject: [PATCH 1/5] Add acceptance test with bug behaviour --- .../variable_overrides_in_target/databricks.yml | 9 +++++++++ .../variables/variable_overrides_in_target/output.txt | 3 +++ .../bundle/variables/variable_overrides_in_target/script | 1 + 3 files changed, 13 insertions(+) diff --git a/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml b/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml index 3a26996dd8..eaf86fd95a 100644 --- a/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml +++ b/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml @@ -22,6 +22,11 @@ variables: default: true description: "A boolean variable" + w_lookup: + description: "A variable with a lookup" + lookup: + cluster: "some-test-cluster" # id 4321 + targets: use-default-variable-values: @@ -37,3 +42,7 @@ targets: variables: foo: "overridden_string" baz: false + + override-lookup-variable: + variables: + w_lookup: "my-specific-cluster-id" diff --git a/acceptance/bundle/variables/variable_overrides_in_target/output.txt b/acceptance/bundle/variables/variable_overrides_in_target/output.txt index 106253110f..951fb21a17 100644 --- a/acceptance/bundle/variables/variable_overrides_in_target/output.txt +++ b/acceptance/bundle/variables/variable_overrides_in_target/output.txt @@ -86,3 +86,6 @@ } } } + +>>> [CLI] bundle validate -o json -t override-lookup-variable +"4321" diff --git a/acceptance/bundle/variables/variable_overrides_in_target/script b/acceptance/bundle/variables/variable_overrides_in_target/script index 686b3102a6..421a432cf1 100644 --- a/acceptance/bundle/variables/variable_overrides_in_target/script +++ b/acceptance/bundle/variables/variable_overrides_in_target/script @@ -2,3 +2,4 @@ trace $CLI bundle validate -o json -t use-default-variable-values | jq .resource trace $CLI bundle validate -o json -t override-string-variable | jq .resources trace $CLI bundle validate -o json -t override-int-variable | jq .resources trace $CLI bundle validate -o json -t override-both-bool-and-string-variables | jq .resources +trace $CLI bundle validate -o json -t override-lookup-variable | jq .variables.w_lookup.value From 41916f570aa3dae6d74d0c43a32d7f0cc060d566 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 8 Apr 2026 15:20:38 +0200 Subject: [PATCH 2/5] Add fix & unit test --- bundle/config/root.go | 16 +++++++++ bundle/config/root_test.go | 71 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/bundle/config/root.go b/bundle/config/root.go index 861791fc34..a32ca8ea28 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -347,12 +347,28 @@ func (r *Root) MergeTargetOverrides(name string) error { if vDefault.Kind() != dyn.KindInvalid { defaultPath := varPath.Append(dyn.Key("default")) root, err = dyn.SetByPath(root, defaultPath, vDefault) + if err != nil { + return root, err + } + + // If the target explicitly sets a default value, drop any lookup from the + // root variable definition so SetVariables can assign this default. + lookupPath := varPath.Append(dyn.Key("lookup")) + root, err = dyn.SetByPath(root, lookupPath, dyn.NilValue) } vLookup := variable.Get("lookup") if vLookup.Kind() != dyn.KindInvalid { lookupPath := varPath.Append(dyn.Key("lookup")) root, err = dyn.SetByPath(root, lookupPath, vLookup) + if err != nil { + return root, err + } + + // If the target explicitly sets a lookup, drop any default value from the + // root variable definition so lookup resolution remains authoritative. + defaultPath := varPath.Append(dyn.Key("default")) + root, err = dyn.SetByPath(root, defaultPath, dyn.NilValue) } return root, err diff --git a/bundle/config/root_test.go b/bundle/config/root_test.go index 42fae49d98..902df6cf85 100644 --- a/bundle/config/root_test.go +++ b/bundle/config/root_test.go @@ -170,6 +170,77 @@ func TestRootMergeTargetOverridesWithVariables(t *testing.T) { assert.Equal(t, "complex var", root.Variables["complex"].Description) } +func TestRootMergeTargetOverridesWithVariableDefaultClearsLookup(t *testing.T) { + root := &Root{ + Variables: map[string]*variable.Variable{ + "foo": { + Lookup: &variable.Lookup{ + Cluster: "my-cluster", + }, + }, + }, + Targets: map[string]*Target{ + "development": { + Variables: map[string]*variable.TargetVariable{ + "foo": { + Default: "bar", + }, + }, + }, + }, + } + + require.NoError(t, root.initializeDynamicValue()) + require.NoError(t, root.MergeTargetOverrides("development")) + assert.Equal(t, "bar", root.Variables["foo"].Default) + assert.Nil(t, root.Variables["foo"].Lookup) +} + +func TestRootMergeTargetOverridesWithVariableLookupClearsDefault(t *testing.T) { + root := &Root{ + Variables: map[string]*variable.Variable{ + "foo": { + Default: "bar", + }, + }, + Targets: map[string]*Target{ + "development": { + Variables: map[string]*variable.TargetVariable{ + "foo": { + Lookup: &variable.Lookup{ + Cluster: "my-cluster", + }, + }, + }, + }, + }, + } + + require.NoError(t, root.initializeDynamicValue()) + require.NoError(t, root.MergeTargetOverrides("development")) + assert.Nil(t, root.Variables["foo"].Default) + assert.Equal(t, "my-cluster", root.Variables["foo"].Lookup.Cluster) +} + +func TestRootMergeTargetOverridesWithShorthandVariableOverrideClearsLookup(t *testing.T) { + root, diags := LoadFromBytes("databricks.yml", []byte(` +variables: + my_variable: + lookup: + cluster: "The name of the cluster" + +targets: + my-target: + variables: + my_variable: "foobar" +`)) + require.NoError(t, diags.Error()) + + require.NoError(t, root.MergeTargetOverrides("my-target")) + assert.Equal(t, "foobar", root.Variables["my_variable"].Default) + assert.Nil(t, root.Variables["my_variable"].Lookup) +} + func TestIsFullVariableOverrideDef(t *testing.T) { testCases := []struct { value dyn.Value From 5032338dca8f7d19e4cbcbf4e165bfa9a044cc9a Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 8 Apr 2026 15:21:28 +0200 Subject: [PATCH 3/5] Update acceptance test with correct expected value --- .../bundle/variables/variable_overrides_in_target/output.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/bundle/variables/variable_overrides_in_target/output.txt b/acceptance/bundle/variables/variable_overrides_in_target/output.txt index 951fb21a17..1df78199f6 100644 --- a/acceptance/bundle/variables/variable_overrides_in_target/output.txt +++ b/acceptance/bundle/variables/variable_overrides_in_target/output.txt @@ -88,4 +88,4 @@ } >>> [CLI] bundle validate -o json -t override-lookup-variable -"4321" +"my-specific-cluster-id" From 60c323e5001444034dcb2e20fd2bc22b732af700 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 8 Apr 2026 15:35:45 +0200 Subject: [PATCH 4/5] Also cover lookup override --- .../variables/variable_overrides_in_target/databricks.yml | 8 +++++++- .../variables/variable_overrides_in_target/output.txt | 5 ++++- .../bundle/variables/variable_overrides_in_target/script | 3 ++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml b/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml index eaf86fd95a..9c1512dd87 100644 --- a/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml +++ b/acceptance/bundle/variables/variable_overrides_in_target/databricks.yml @@ -43,6 +43,12 @@ targets: foo: "overridden_string" baz: false - override-lookup-variable: + override-lookup-variable-with-value: variables: w_lookup: "my-specific-cluster-id" + + override-lookup-variable-with-lookup: + variables: + w_lookup: + lookup: + cluster: "some-other-cluster" # id 9876 diff --git a/acceptance/bundle/variables/variable_overrides_in_target/output.txt b/acceptance/bundle/variables/variable_overrides_in_target/output.txt index 1df78199f6..e34e2df930 100644 --- a/acceptance/bundle/variables/variable_overrides_in_target/output.txt +++ b/acceptance/bundle/variables/variable_overrides_in_target/output.txt @@ -87,5 +87,8 @@ } } ->>> [CLI] bundle validate -o json -t override-lookup-variable +>>> [CLI] bundle validate -o json -t override-lookup-variable-with-value "my-specific-cluster-id" + +>>> [CLI] bundle validate -o json -t override-lookup-variable-with-lookup +"9876" diff --git a/acceptance/bundle/variables/variable_overrides_in_target/script b/acceptance/bundle/variables/variable_overrides_in_target/script index 421a432cf1..e814802893 100644 --- a/acceptance/bundle/variables/variable_overrides_in_target/script +++ b/acceptance/bundle/variables/variable_overrides_in_target/script @@ -2,4 +2,5 @@ trace $CLI bundle validate -o json -t use-default-variable-values | jq .resource trace $CLI bundle validate -o json -t override-string-variable | jq .resources trace $CLI bundle validate -o json -t override-int-variable | jq .resources trace $CLI bundle validate -o json -t override-both-bool-and-string-variables | jq .resources -trace $CLI bundle validate -o json -t override-lookup-variable | jq .variables.w_lookup.value +trace $CLI bundle validate -o json -t override-lookup-variable-with-value | jq .variables.w_lookup.value +trace $CLI bundle validate -o json -t override-lookup-variable-with-lookup | jq .variables.w_lookup.value From 1bf27a5d37aba871aa228b2c90ecd4ae00cfd443 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 8 Apr 2026 18:25:02 +0200 Subject: [PATCH 5/5] Remove unit tests --- bundle/config/root_test.go | 71 -------------------------------------- 1 file changed, 71 deletions(-) diff --git a/bundle/config/root_test.go b/bundle/config/root_test.go index 902df6cf85..42fae49d98 100644 --- a/bundle/config/root_test.go +++ b/bundle/config/root_test.go @@ -170,77 +170,6 @@ func TestRootMergeTargetOverridesWithVariables(t *testing.T) { assert.Equal(t, "complex var", root.Variables["complex"].Description) } -func TestRootMergeTargetOverridesWithVariableDefaultClearsLookup(t *testing.T) { - root := &Root{ - Variables: map[string]*variable.Variable{ - "foo": { - Lookup: &variable.Lookup{ - Cluster: "my-cluster", - }, - }, - }, - Targets: map[string]*Target{ - "development": { - Variables: map[string]*variable.TargetVariable{ - "foo": { - Default: "bar", - }, - }, - }, - }, - } - - require.NoError(t, root.initializeDynamicValue()) - require.NoError(t, root.MergeTargetOverrides("development")) - assert.Equal(t, "bar", root.Variables["foo"].Default) - assert.Nil(t, root.Variables["foo"].Lookup) -} - -func TestRootMergeTargetOverridesWithVariableLookupClearsDefault(t *testing.T) { - root := &Root{ - Variables: map[string]*variable.Variable{ - "foo": { - Default: "bar", - }, - }, - Targets: map[string]*Target{ - "development": { - Variables: map[string]*variable.TargetVariable{ - "foo": { - Lookup: &variable.Lookup{ - Cluster: "my-cluster", - }, - }, - }, - }, - }, - } - - require.NoError(t, root.initializeDynamicValue()) - require.NoError(t, root.MergeTargetOverrides("development")) - assert.Nil(t, root.Variables["foo"].Default) - assert.Equal(t, "my-cluster", root.Variables["foo"].Lookup.Cluster) -} - -func TestRootMergeTargetOverridesWithShorthandVariableOverrideClearsLookup(t *testing.T) { - root, diags := LoadFromBytes("databricks.yml", []byte(` -variables: - my_variable: - lookup: - cluster: "The name of the cluster" - -targets: - my-target: - variables: - my_variable: "foobar" -`)) - require.NoError(t, diags.Error()) - - require.NoError(t, root.MergeTargetOverrides("my-target")) - assert.Equal(t, "foobar", root.Variables["my_variable"].Default) - assert.Nil(t, root.Variables["my_variable"].Lookup) -} - func TestIsFullVariableOverrideDef(t *testing.T) { testCases := []struct { value dyn.Value