Override variables with lookup value even if values has default value set

bundle/config/mutator/resolve_resource_references_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/config/variable"
+	"github.com/databricks/cli/libs/env"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
@@ -194,3 +195,37 @@ func TestResolveLookupVariableReferencesInVariableLookups(t *testing.T) {
 	diags := bundle.Apply(context.Background(), b, bundle.Seq(ResolveVariableReferencesInLookup(), ResolveResourceReferences()))
 	require.ErrorContains(t, diags.Error(), "lookup variables cannot contain references to another lookup variables")
 }
+
+func TestNoResolveLookupIfVariableSetWithEnvVariable(t *testing.T) {
+	s := func(s string) *string {
+		return &s
+	}
+
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Bundle: config.Bundle{
+				Target: "dev",
+			},
+			Variables: map[string]*variable.Variable{
+				"foo": {
+					Value: s("bar"),
+				},
+				"lookup": {
+					Lookup: &variable.Lookup{
+						Cluster: "cluster-${var.foo}-${bundle.target}",
+					},
+				},
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	b.SetWorkpaceClient(m.WorkspaceClient)
+
+	ctx := context.Background()
+	ctx = env.Set(ctx, "BUNDLE_VAR_lookup", "1234-5678-abcd")
+
+	diags := bundle.Apply(ctx, b, bundle.Seq(SetVariables(), ResolveVariableReferencesInLookup(), ResolveResourceReferences()))
+	require.NoError(t, diags.Error())
+	require.Equal(t, "1234-5678-abcd", *b.Config.Variables["lookup"].Value)
+}

bundle/config/root.go

@@ -321,6 +321,11 @@ func (r *Root) MergeTargetOverrides(name string) error {
 		return err
 	}
 
+	root, err = unsetDefaultForVariableLookupOverrides(root, target)
+	if err != nil {
+		return err
+	}
+
 	// Merge fields that can be merged 1:1.
 	for _, f := range []string{
 		"bundle",
@@ -461,6 +466,37 @@ func validateVariableOverrides(root, target dyn.Value) (err error) {
 	return nil
 }
 
+// unsetDefaultForVariableLookupOverrides makes sure that variables which are overriden in targets with lookup
+// do not have any default value set because otherwise the lookup will not be performed.
+func unsetDefaultForVariableLookupOverrides(root, target dyn.Value) (dyn.Value, error) {
+	r := root
+	// Collect variables from the root.
+	var rv map[string]*variable.Variable
+	err := convert.ToTyped(&rv, root.Get("variables"))
+	if err != nil {
+		return dyn.InvalidValue, fmt.Errorf("unable to collect variables from root: %w", err)
+	}
+
+	// Collect variables from the target.
+	var tv map[string]*variable.Variable
+	err = convert.ToTyped(&tv, target.Get("variables"))
+	if err != nil {
+		return dyn.InvalidValue, fmt.Errorf("unable to collect variables from target: %w", err)
+	}
+
+	// Check that all variables in the target exist in the root.
+	for k, v := range tv {
+		if rv[k].Default != nil && v.Lookup != nil {
+			r, err = dyn.SetByPath(r, dyn.NewPath(dyn.Key("variables"), dyn.Key(k), dyn.Key("default")), dyn.NilValue)
+			if err != nil {
+				return dyn.InvalidValue, err
+			}
+		}
+	}
+
+	return r, nil
+}
+
 // Best effort to get the location of configuration value at the specified path.
 // This function is useful to annotate error messages with the location, because
 // we don't want to fail with a different error message if we cannot retrieve the location.

bundle/tests/variables/env_overrides/databricks.yml

@@ -8,14 +8,16 @@ variables:
 
   d:
     description: variable with lookup
-    lookup:
-      cluster: some-cluster
+    default: ""
 
   e:
     description: variable with lookup
-    lookup:
-      instance_pool: some-pool
+    default: "some-value"
 
+  f:
+    description: variable with lookup
+    lookup:
+      cluster_policy: wrong-cluster-policy
 bundle:
   name: test bundle
 
@@ -49,4 +51,7 @@ targets:
       e:
         lookup:
           instance_pool: some-test-instance-pool
+      f:
+        lookup:
+          cluster_policy: some-test-cluster-policy
       b: prod-b

bundle/tests/variables_test.go

@@ -6,7 +6,10 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/databricks/databricks-sdk-go/service/compute"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -112,13 +115,34 @@ func TestVariablesWithoutDefinition(t *testing.T) {
 
 func TestVariablesWithTargetLookupOverrides(t *testing.T) {
 	b := load(t, "./variables/env_overrides")
+
+	mockWorkspaceClient := mocks.NewMockWorkspaceClient(t)
+	b.SetWorkpaceClient(mockWorkspaceClient.WorkspaceClient)
+	instancePoolApi := mockWorkspaceClient.GetMockInstancePoolsAPI()
+	instancePoolApi.EXPECT().GetByInstancePoolName(mock.Anything, "some-test-instance-pool").Return(&compute.InstancePoolAndStats{
+		InstancePoolId: "1234",
+	}, nil)
+
+	clustersApi := mockWorkspaceClient.GetMockClustersAPI()
+	clustersApi.EXPECT().GetByClusterName(mock.Anything, "some-test-cluster").Return(&compute.ClusterDetails{
+		ClusterId: "4321",
+	}, nil)
+
+	clusterPoliciesApi := mockWorkspaceClient.GetMockClusterPoliciesAPI()
+	clusterPoliciesApi.EXPECT().GetByName(mock.Anything, "some-test-cluster-policy").Return(&compute.Policy{
+		PolicyId: "9876",
+	}, nil)
+
 	diags := bundle.Apply(context.Background(), b, bundle.Seq(
 		mutator.SelectTarget("env-overrides-lookup"),
 		mutator.SetVariables(),
+		mutator.ResolveResourceReferences(),
 	))
+
 	require.NoError(t, diags.Error())
-	assert.Equal(t, "cluster: some-test-cluster", b.Config.Variables["d"].Lookup.String())
-	assert.Equal(t, "instance-pool: some-test-instance-pool", b.Config.Variables["e"].Lookup.String())
+	assert.Equal(t, "4321", *b.Config.Variables["d"].Value)
+	assert.Equal(t, "1234", *b.Config.Variables["e"].Value)
+	assert.Equal(t, "9876", *b.Config.Variables["f"].Value)
 }
 
 func TestVariableTargetOverrides(t *testing.T) {