Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for complex variables #1467

Merged
merged 16 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
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
32 changes: 11 additions & 21 deletions bundle/config/mutator/resolve_resource_references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestResolveClusterReference(t *testing.T) {
},
},
"some-variable": {
Value: &justString,
Value: justString,
},
},
},
Expand All @@ -53,8 +53,8 @@ func TestResolveClusterReference(t *testing.T) {

diags := bundle.Apply(context.Background(), b, ResolveResourceReferences())
require.NoError(t, diags.Error())
require.Equal(t, "1234-5678-abcd", *b.Config.Variables["my-cluster-id-1"].Value)
require.Equal(t, "9876-5432-xywz", *b.Config.Variables["my-cluster-id-2"].Value)
require.Equal(t, "1234-5678-abcd", b.Config.Variables["my-cluster-id-1"].Value)
require.Equal(t, "9876-5432-xywz", b.Config.Variables["my-cluster-id-2"].Value)
}

func TestResolveNonExistentClusterReference(t *testing.T) {
Expand All @@ -69,7 +69,7 @@ func TestResolveNonExistentClusterReference(t *testing.T) {
},
},
"some-variable": {
Value: &justString,
Value: justString,
},
},
},
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestNoLookupIfVariableIsSet(t *testing.T) {

diags := bundle.Apply(context.Background(), b, ResolveResourceReferences())
require.NoError(t, diags.Error())
require.Equal(t, "random value", *b.Config.Variables["my-cluster-id"].Value)
require.Equal(t, "random value", b.Config.Variables["my-cluster-id"].Value)
}

func TestResolveServicePrincipal(t *testing.T) {
Expand All @@ -132,22 +132,19 @@ func TestResolveServicePrincipal(t *testing.T) {

diags := bundle.Apply(context.Background(), b, ResolveResourceReferences())
require.NoError(t, diags.Error())
require.Equal(t, "app-1234", *b.Config.Variables["my-sp"].Value)
require.Equal(t, "app-1234", b.Config.Variables["my-sp"].Value)
}

func TestResolveVariableReferencesInVariableLookups(t *testing.T) {
s := func(s string) *string {
return &s
}

s := "bar"
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Target: "dev",
},
Variables: map[string]*variable.Variable{
"foo": {
Value: s("bar"),
Value: s,
},
"lookup": {
Lookup: &variable.Lookup{
Expand All @@ -168,7 +165,7 @@ func TestResolveVariableReferencesInVariableLookups(t *testing.T) {
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)
require.Equal(t, "1234-5678-abcd", b.Config.Variables["lookup"].Value)
}

func TestResolveLookupVariableReferencesInVariableLookups(t *testing.T) {
Expand Down Expand Up @@ -197,22 +194,15 @@ func TestResolveLookupVariableReferencesInVariableLookups(t *testing.T) {
}

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}",
Cluster: "cluster-${bundle.target}",
},
},
},
Expand All @@ -227,5 +217,5 @@ func TestNoResolveLookupIfVariableSetWithEnvVariable(t *testing.T) {

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)
require.Equal(t, "1234-5678-abcd", b.Config.Variables["lookup"].Value)
}
55 changes: 53 additions & 2 deletions bundle/config/mutator/resolve_variable_references.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type resolveVariableReferences struct {
prefixes []string
pattern dyn.Pattern
lookupFn func(dyn.Value, dyn.Path) (dyn.Value, error)
skipFn func(dyn.Value) bool
}

func ResolveVariableReferences(prefixes ...string) bundle.Mutator {
Expand All @@ -31,13 +32,53 @@ func ResolveVariableReferencesInLookup() bundle.Mutator {
}, pattern: dyn.NewPattern(dyn.Key("variables"), dyn.AnyKey(), dyn.Key("lookup")), lookupFn: lookupForVariables}
}

func ResolveVariableReferencesInComplexVariables() bundle.Mutator {
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
return &resolveVariableReferences{prefixes: []string{
"bundle",
"workspace",
"variables",
},
pattern: dyn.NewPattern(dyn.Key("variables"), dyn.AnyKey(), dyn.Key("value")),
lookupFn: lookupForComplexVariables,
skipFn: skipResolvingInNonComplexVariables,
}
}

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 lookupForComplexVariables(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.Type == variable.VariableTypeComplex {
return dyn.InvalidValue, fmt.Errorf("complex variables cannot contain references to another complex variables")
}

return lookup(v, path)
}

func skipResolvingInNonComplexVariables(v dyn.Value) bool {
_, ok := v.AsMap()
return !ok
}

func lookupForVariables(v dyn.Value, path dyn.Path) (dyn.Value, error) {
if path[0].Key() != "variables" {
return lookup(v, path)
Expand Down Expand Up @@ -100,17 +141,27 @@ func (m *resolveVariableReferences) Apply(ctx context.Context, b *bundle.Bundle)
// 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(
if path.HasPrefix(varPath) {
newPath := dyn.NewPath(
dyn.Key("variables"),
path[1],
dyn.Key("value"),
)

if len(path) > 2 {
newPath = newPath.Append(path[2:]...)
}

path = newPath
}

// Perform resolution only if the path starts with one of the specified prefixes.
for _, prefix := range prefixes {
if path.HasPrefix(prefix) {
// Skip resolution if there is a skip function and it returns true.
if m.skipFn != nil && m.skipFn(v) {
return dyn.InvalidValue, dynvar.ErrSkipResolution
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved outside of dynvar.Resolve.

return m.lookupFn(normalized, path)
}
}
Expand Down
121 changes: 116 additions & 5 deletions bundle/config/mutator/resolve_variable_references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ func TestResolveVariableReferences(t *testing.T) {
}

func TestResolveVariableReferencesToBundleVariables(t *testing.T) {
s := func(s string) *string {
return &s
}

b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Expand All @@ -57,7 +53,7 @@ func TestResolveVariableReferencesToBundleVariables(t *testing.T) {
},
Variables: map[string]*variable.Variable{
"foo": {
Value: s("bar"),
Value: "bar",
},
},
},
Expand Down Expand Up @@ -195,3 +191,118 @@ func TestResolveVariableReferencesForPrimitiveNonStringFields(t *testing.T) {
assert.Equal(t, 2, b.Config.Resources.Jobs["job1"].JobSettings.Tasks[0].NewCluster.Autoscale.MaxWorkers)
assert.Equal(t, 0.5, b.Config.Resources.Jobs["job1"].JobSettings.Tasks[0].NewCluster.AzureAttributes.SpotBidMaxPrice)
}

func TestResolveComplexVariable(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Name: "example",
},
Variables: map[string]*variable.Variable{
"cluster": {
Value: map[string]any{
"node_type_id": "Standard_DS3_v2",
"num_workers": 2,
},
},
},

Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
JobSettings: &jobs.JobSettings{
JobClusters: []jobs.JobCluster{
{
NewCluster: compute.ClusterSpec{
NodeTypeId: "random",
},
},
},
},
},
},
},
},
}

ctx := context.Background()

// Assign the variables to the dynamic configuration.
diags := bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
var p dyn.Path
var err error

// Set the notification settings.
p = dyn.MustPathFromString("resources.jobs.job1.job_clusters[0]")
v, err = dyn.SetByPath(v, p.Append(dyn.Key("new_cluster")), dyn.V("${var.cluster}"))
require.NoError(t, err)

return v, nil
})
return diag.FromErr(err)
})
require.NoError(t, diags.Error())

diags = bundle.Apply(ctx, b, ResolveVariableReferences("bundle", "workspace", "variables"))
require.NoError(t, diags.Error())
require.Equal(t, "Standard_DS3_v2", b.Config.Resources.Jobs["job1"].JobSettings.JobClusters[0].NewCluster.NodeTypeId)
require.Equal(t, 2, b.Config.Resources.Jobs["job1"].JobSettings.JobClusters[0].NewCluster.NumWorkers)
}

func TestResolveComplexVariableReferencesToFields(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Bundle: config.Bundle{
Name: "example",
},
Variables: map[string]*variable.Variable{
"cluster": {
Value: map[string]any{
"node_type_id": "Standard_DS3_v2",
"num_workers": 2,
},
},
},

Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job1": {
JobSettings: &jobs.JobSettings{
JobClusters: []jobs.JobCluster{
{
NewCluster: compute.ClusterSpec{
NodeTypeId: "random",
},
},
},
},
},
},
},
},
}

ctx := context.Background()

// Assign the variables to the dynamic configuration.
diags := bundle.ApplyFunc(ctx, b, func(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
var p dyn.Path
var err error

// Set the notification settings.
p = dyn.MustPathFromString("resources.jobs.job1.job_clusters[0].new_cluster")
v, err = dyn.SetByPath(v, p.Append(dyn.Key("node_type_id")), dyn.V("${var.cluster.node_type_id}"))
require.NoError(t, err)

return v, nil
})
return diag.FromErr(err)
})
require.NoError(t, diags.Error())

diags = bundle.Apply(ctx, b, ResolveVariableReferences("bundle", "workspace", "variables"))
require.NoError(t, diags.Error())
require.Equal(t, "Standard_DS3_v2", b.Config.Resources.Jobs["job1"].JobSettings.JobClusters[0].NewCluster.NodeTypeId)
}
8 changes: 6 additions & 2 deletions bundle/config/mutator/set_variables.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ func setVariable(ctx context.Context, v *variable.Variable, name string) diag.Di
// case: read and set variable value from process environment
envVarName := bundleVarPrefix + name
if val, ok := env.Lookup(ctx, envVarName); ok {
if v.IsComplex() {
return diag.Errorf(`setting via environment variables (%s) is not supported for complex variable %s`, envVarName, name)
}

err := v.Set(val)
if err != nil {
return diag.Errorf(`failed to assign value "%s" to variable %s from environment variable %s with error: %v`, val, name, envVarName, err)
Expand All @@ -45,9 +49,9 @@ func setVariable(ctx context.Context, v *variable.Variable, name string) diag.Di

// case: Set the variable to its default value
if v.HasDefault() {
err := v.Set(*v.Default)
err := v.Set(v.Default)
if err != nil {
return diag.Errorf(`failed to assign default value from config "%s" to variable %s with error: %v`, *v.Default, name, err)
return diag.Errorf(`failed to assign default value from config "%s" to variable %s with error: %v`, v.Default, name, err)
}
return nil
}
Expand Down
Loading
Loading