Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions bundle/config/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,11 @@ func (r *Root) MergeTargetOverrides(name string) error {
return err
}

root, err = normalizeVariableLookupOverrides(root, target)
Comment thread
andrewnester marked this conversation as resolved.
Outdated
if err != nil {
return err
}

// Merge fields that can be merged 1:1.
for _, f := range []string{
"bundle",
Expand Down Expand Up @@ -461,6 +466,37 @@ func validateVariableOverrides(root, target dyn.Value) (err error) {
return nil
}

// normalizeVariableLookupOverrides 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.
Comment thread
andrewnester marked this conversation as resolved.
Outdated
func normalizeVariableLookupOverrides(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.
Expand Down
6 changes: 2 additions & 4 deletions bundle/tests/variables/env_overrides/databricks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ variables:

d:
description: variable with lookup
lookup:
cluster: some-cluster
default: ""

e:
description: variable with lookup
lookup:
instance_pool: some-pool
default: "some-value"
Comment thread
andrewnester marked this conversation as resolved.

bundle:
name: test bundle
Expand Down
22 changes: 20 additions & 2 deletions bundle/tests/variables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -112,13 +115,28 @@ 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)

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)
}
Comment thread
andrewnester marked this conversation as resolved.

func TestVariableTargetOverrides(t *testing.T) {
Expand Down