Skip to content

Commit

Permalink
Resolve variable references inside variable lookup fields (#1368)
Browse files Browse the repository at this point in the history
## Changes
Allows for the syntax below

```
variables:
  service_principal_app_id:
    description: 'The app id of the service principal for running workflows as.'
    lookup:
      service_principal: "sp-${bundle.environment}"
```

Fixes #1259

## Tests
Added regression test
  • Loading branch information
andrewnester committed Apr 18, 2024
1 parent c3a7d17 commit 542156c
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 25 deletions.
61 changes: 61 additions & 0 deletions bundle/config/mutator/resolve_resource_references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,64 @@ func TestResolveServicePrincipal(t *testing.T) {
require.NoError(t, diags.Error())
require.Equal(t, "app-1234", *b.Config.Variables["my-sp"].Value)
}

func TestResolveVariableReferencesInVariableLookups(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)
clusterApi := m.GetMockClustersAPI()
clusterApi.EXPECT().GetByClusterName(mock.Anything, "cluster-bar-dev").Return(&compute.ClusterDetails{
ClusterId: "1234-5678-abcd",
}, nil)

diags := bundle.Apply(context.Background(), b, bundle.Seq(ResolveVariableReferencesInLookup(), ResolveResourceReferences()))
require.NoError(t, diags.Error())
require.Equal(t, "cluster-bar-dev", b.Config.Variables["lookup"].Lookup.Cluster)
require.Equal(t, "1234-5678-abcd", *b.Config.Variables["lookup"].Value)
}

func TestResolveLookupVariableReferencesInVariableLookups(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Variables: map[string]*variable.Variable{
"another_lookup": {
Lookup: &variable.Lookup{
Cluster: "cluster",
},
},
"lookup": {
Lookup: &variable.Lookup{
Cluster: "cluster-${var.another_lookup}",
},
},
},
},
}

m := mocks.NewMockWorkspaceClient(t)
b.SetWorkpaceClient(m.WorkspaceClient)

diags := bundle.Apply(context.Background(), b, bundle.Seq(ResolveVariableReferencesInLookup(), ResolveResourceReferences()))
require.ErrorContains(t, diags.Error(), "lookup variables cannot contain references to another lookup variables")
}
90 changes: 65 additions & 25 deletions bundle/config/mutator/resolve_variable_references.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package mutator

import (
"context"
"fmt"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/variable"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/dyn/convert"
Expand All @@ -13,10 +15,50 @@ import (

type resolveVariableReferences struct {
prefixes []string
pattern dyn.Pattern
lookupFn func(dyn.Value, dyn.Path) (dyn.Value, error)
}

func ResolveVariableReferences(prefixes ...string) bundle.Mutator {
return &resolveVariableReferences{prefixes: prefixes}
return &resolveVariableReferences{prefixes: prefixes, lookupFn: lookup}
}

func ResolveVariableReferencesInLookup() bundle.Mutator {
return &resolveVariableReferences{prefixes: []string{
"bundle",
"workspace",
"variables",
}, pattern: dyn.NewPattern(dyn.Key("variables"), dyn.AnyKey(), dyn.Key("lookup")), lookupFn: lookupForVariables}
}

func lookup(v dyn.Value, path dyn.Path) (dyn.Value, error) {
// Future opportunity: if we lookup this path in both the given root
// and the synthesized root, we know if it was explicitly set or implied to be empty.
// Then we can emit a warning if it was not explicitly set.
return dyn.GetByPath(v, path)
}

func lookupForVariables(v dyn.Value, path dyn.Path) (dyn.Value, error) {
if path[0].Key() != "variables" {
return lookup(v, path)
}

varV, err := dyn.GetByPath(v, path[:len(path)-1])
if err != nil {
return dyn.InvalidValue, err
}

var vv variable.Variable
err = convert.ToTyped(&vv, varV)
if err != nil {
return dyn.InvalidValue, err
}

if vv.Lookup != nil && vv.Lookup.String() != "" {
return dyn.InvalidValue, fmt.Errorf("lookup variables cannot contain references to another lookup variables")
}

return lookup(v, path)
}

func (*resolveVariableReferences) Name() string {
Expand Down Expand Up @@ -48,37 +90,35 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
//
// This is consistent with the behavior prior to using the dynamic value system.
//
// We can ignore the diagnostics return valuebecause we know that the dynamic value
// We can ignore the diagnostics return value because we know that the dynamic value
// has already been normalized when it was first loaded from the configuration file.
//
normalized, _ := convert.Normalize(b.Config, root, convert.IncludeMissingFields)
lookup := func(path dyn.Path) (dyn.Value, error) {
// Future opportunity: if we lookup this path in both the given root
// and the synthesized root, we know if it was explicitly set or implied to be empty.
// Then we can emit a warning if it was not explicitly set.
return dyn.GetByPath(normalized, path)
}

// Resolve variable references in all values.
root, err := dynvar.Resolve(root, func(path dyn.Path) (dyn.Value, error) {
// Rewrite the shorthand path ${var.foo} into ${variables.foo.value}.
if path.HasPrefix(varPath) && len(path) == 2 {
path = dyn.NewPath(
dyn.Key("variables"),
path[1],
dyn.Key("value"),
)
}

// Perform resolution only if the path starts with one of the specified prefixes.
for _, prefix := range prefixes {
if path.HasPrefix(prefix) {
return lookup(path)
// If the pattern is nil, we resolve references in the entire configuration.
root, err := dyn.MapByPattern(root, m.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
// Resolve variable references in all values.
return dynvar.Resolve(v, func(path dyn.Path) (dyn.Value, error) {
// Rewrite the shorthand path ${var.foo} into ${variables.foo.value}.
if path.HasPrefix(varPath) && len(path) == 2 {
path = dyn.NewPath(
dyn.Key("variables"),
path[1],
dyn.Key("value"),
)
}

// Perform resolution only if the path starts with one of the specified prefixes.
for _, prefix := range prefixes {
if path.HasPrefix(prefix) {
return m.lookupFn(normalized, path)
}
}
}

return dyn.InvalidValue, dynvar.ErrSkipResolution
return dyn.InvalidValue, dynvar.ErrSkipResolution
})
})

if err != nil {
return dyn.InvalidValue, err
}
Expand Down
1 change: 1 addition & 0 deletions bundle/phases/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func Initialize() bundle.Mutator {
mutator.ExpandWorkspaceRoot(),
mutator.DefineDefaultWorkspacePaths(),
mutator.SetVariables(),
mutator.ResolveVariableReferencesInLookup(),
mutator.ResolveResourceReferences(),
mutator.ResolveVariableReferences(
"bundle",
Expand Down

0 comments on commit 542156c

Please sign in to comment.