diff --git a/bundle/config/mutator/set_variables.go b/bundle/config/mutator/set_variables.go index b3a9cf40..47ce2ad0 100644 --- a/bundle/config/mutator/set_variables.go +++ b/bundle/config/mutator/set_variables.go @@ -2,10 +2,12 @@ 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/env" ) @@ -21,52 +23,63 @@ func (m *setVariables) Name() string { return "SetVariables" } -func setVariable(ctx context.Context, v *variable.Variable, name string) diag.Diagnostics { +func setVariable(ctx context.Context, v dyn.Value, variable *variable.Variable, name string) (dyn.Value, error) { // case: variable already has value initialized, so skip - if v.HasValue() { - return nil + if variable.HasValue() { + return v, nil } // 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) + if variable.IsComplex() { + return dyn.InvalidValue, fmt.Errorf(`setting via environment variables (%s) is not supported for complex variable %s`, envVarName, name) } - err := v.Set(val) + v, err := dyn.Set(v, "value", dyn.V(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) + return dyn.InvalidValue, fmt.Errorf(`failed to assign value "%s" to variable %s from environment variable %s with error: %v`, val, name, envVarName, err) } - return nil + return v, nil } // case: Defined a variable for named lookup for a resource // It will be resolved later in ResolveResourceReferences mutator - if v.Lookup != nil { - return nil + if variable.Lookup != nil { + return v, nil } // case: Set the variable to its default value - if v.HasDefault() { - err := v.Set(v.Default) + if variable.HasDefault() { + vDefault, err := dyn.Get(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 dyn.InvalidValue, fmt.Errorf(`failed to get default value from config "%s" for variable %s with error: %v`, variable.Default, name, err) } - return nil + + v, err := dyn.Set(v, "value", vDefault) + if err != nil { + return dyn.InvalidValue, fmt.Errorf(`failed to assign default value from config "%s" to variable %s with error: %v`, variable.Default, name, err) + } + return v, nil } // We should have had a value to set for the variable at this point. - return diag.Errorf(`no value assigned to required variable %s. Assignment can be done through the "--var" flag or by setting the %s environment variable`, name, bundleVarPrefix+name) + return dyn.InvalidValue, fmt.Errorf(`no value assigned to required variable %s. Assignment can be done through the "--var" flag or by setting the %s environment variable`, name, bundleVarPrefix+name) + } func (m *setVariables) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - var diags diag.Diagnostics - for name, variable := range b.Config.Variables { - diags = diags.Extend(setVariable(ctx, variable, name)) - if diags.HasError() { - return diags - } - } - return diags + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.Map(v, "variables", dyn.Foreach(func(p dyn.Path, variable dyn.Value) (dyn.Value, error) { + name := p[1].Key() + v, ok := b.Config.Variables[name] + if !ok { + return dyn.InvalidValue, fmt.Errorf(`variable "%s" is not defined`, name) + } + + return setVariable(ctx, variable, v, name) + })) + }) + + return diag.FromErr(err) } diff --git a/bundle/config/mutator/set_variables_test.go b/bundle/config/mutator/set_variables_test.go index 65dedee9..d9719793 100644 --- a/bundle/config/mutator/set_variables_test.go +++ b/bundle/config/mutator/set_variables_test.go @@ -7,6 +7,8 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/variable" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,9 +22,14 @@ func TestSetVariableFromProcessEnvVar(t *testing.T) { // set value for variable as an environment variable t.Setenv("BUNDLE_VAR_foo", "process-env") + v, err := convert.FromTyped(variable, dyn.NilValue) + require.NoError(t, err) - diags := setVariable(context.Background(), &variable, "foo") - require.NoError(t, diags.Error()) + v, err = setVariable(context.Background(), v, &variable, "foo") + require.NoError(t, err) + + err = convert.ToTyped(&variable, v) + require.NoError(t, err) assert.Equal(t, variable.Value, "process-env") } @@ -33,8 +40,14 @@ func TestSetVariableUsingDefaultValue(t *testing.T) { Default: defaultVal, } - diags := setVariable(context.Background(), &variable, "foo") - require.NoError(t, diags.Error()) + v, err := convert.FromTyped(variable, dyn.NilValue) + require.NoError(t, err) + + v, err = setVariable(context.Background(), v, &variable, "foo") + require.NoError(t, err) + + err = convert.ToTyped(&variable, v) + require.NoError(t, err) assert.Equal(t, variable.Value, "default") } @@ -49,8 +62,14 @@ func TestSetVariableWhenAlreadyAValueIsAssigned(t *testing.T) { // since a value is already assigned to the variable, it would not be overridden // by the default value - diags := setVariable(context.Background(), &variable, "foo") - require.NoError(t, diags.Error()) + v, err := convert.FromTyped(variable, dyn.NilValue) + require.NoError(t, err) + + v, err = setVariable(context.Background(), v, &variable, "foo") + require.NoError(t, err) + + err = convert.ToTyped(&variable, v) + require.NoError(t, err) assert.Equal(t, variable.Value, "assigned-value") } @@ -68,8 +87,14 @@ func TestSetVariableEnvVarValueDoesNotOverridePresetValue(t *testing.T) { // since a value is already assigned to the variable, it would not be overridden // by the value from environment - diags := setVariable(context.Background(), &variable, "foo") - require.NoError(t, diags.Error()) + v, err := convert.FromTyped(variable, dyn.NilValue) + require.NoError(t, err) + + v, err = setVariable(context.Background(), v, &variable, "foo") + require.NoError(t, err) + + err = convert.ToTyped(&variable, v) + require.NoError(t, err) assert.Equal(t, variable.Value, "assigned-value") } @@ -79,8 +104,11 @@ func TestSetVariablesErrorsIfAValueCouldNotBeResolved(t *testing.T) { } // fails because we could not resolve a value for the variable - diags := setVariable(context.Background(), &variable, "foo") - assert.ErrorContains(t, diags.Error(), "no value assigned to required variable foo. Assignment can be done through the \"--var\" flag or by setting the BUNDLE_VAR_foo environment variable") + v, err := convert.FromTyped(variable, dyn.NilValue) + require.NoError(t, err) + + _, err = setVariable(context.Background(), v, &variable, "foo") + assert.ErrorContains(t, err, "no value assigned to required variable foo. Assignment can be done through the \"--var\" flag or by setting the BUNDLE_VAR_foo environment variable") } func TestSetVariablesMutator(t *testing.T) { @@ -126,6 +154,9 @@ func TestSetComplexVariablesViaEnvVariablesIsNotAllowed(t *testing.T) { // set value for variable as an environment variable t.Setenv("BUNDLE_VAR_foo", "process-env") - diags := setVariable(context.Background(), &variable, "foo") - assert.ErrorContains(t, diags.Error(), "setting via environment variables (BUNDLE_VAR_foo) is not supported for complex variable foo") + v, err := convert.FromTyped(variable, dyn.NilValue) + require.NoError(t, err) + + _, err = setVariable(context.Background(), v, &variable, "foo") + assert.ErrorContains(t, err, "setting via environment variables (BUNDLE_VAR_foo) is not supported for complex variable foo") } diff --git a/bundle/config/mutator/translate_paths_test.go b/bundle/config/mutator/translate_paths_test.go index 8476ee38..780a540d 100644 --- a/bundle/config/mutator/translate_paths_test.go +++ b/bundle/config/mutator/translate_paths_test.go @@ -11,7 +11,10 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/config/variable" "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go/service/compute" "github.com/databricks/databricks-sdk-go/service/jobs" @@ -708,3 +711,64 @@ func TestTranslatePathJobEnvironments(t *testing.T) { assert.Equal(t, "simplejson", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[2]) assert.Equal(t, "/Workspace/Users/foo@bar.com/test.whl", b.Config.Resources.Jobs["job"].JobSettings.Environments[0].Spec.Dependencies[3]) } + +func TestTranslatePathWithComplexVariables(t *testing.T) { + dir := t.TempDir() + b := &bundle.Bundle{ + RootPath: dir, + BundleRoot: vfs.MustNew(dir), + Config: config.Root{ + Variables: map[string]*variable.Variable{ + "cluster_libraries": { + Type: variable.VariableTypeComplex, + Default: [](map[string]string){ + { + "whl": "./local/whl.whl", + }, + }, + }, + }, + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "job": { + JobSettings: &jobs.JobSettings{ + Tasks: []jobs.Task{ + { + TaskKey: "test", + }, + }, + }, + }, + }, + }, + }, + } + + bundletest.SetLocation(b, "variables", filepath.Join(dir, "variables/variables.yml")) + bundletest.SetLocation(b, "resources.jobs", filepath.Join(dir, "job/resource.yml")) + + 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) { + p := dyn.MustPathFromString("resources.jobs.job.tasks[0]") + return dyn.SetByPath(v, p.Append(dyn.Key("libraries")), dyn.V("${var.cluster_libraries}")) + }) + return diag.FromErr(err) + }) + require.NoError(t, diags.Error()) + + diags = bundle.Apply(ctx, b, + bundle.Seq( + mutator.SetVariables(), + mutator.ResolveVariableReferences("variables"), + mutator.TranslatePaths(), + )) + require.NoError(t, diags.Error()) + + assert.Equal( + t, + filepath.Join("variables", "local", "whl.whl"), + b.Config.Resources.Jobs["job"].Tasks[0].Libraries[0].Whl, + ) +}