Fix variable overrides in targets for non-string variables (#1397)

Before variable overrides that were not string in a target would not
work. This PR fixes that. Tested manually and via a unit test.
This commit is contained in:
shreyas-goenka 2024-04-25 16:51:10 +05:30 committed by GitHub
parent 6fd581d173
commit e652333103
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 101 additions and 7 deletions

View File

@ -408,15 +408,19 @@ func rewriteShorthands(v dyn.Value) (dyn.Value, error) {
// For each variable, normalize its contents if it is a single string. // For each variable, normalize its contents if it is a single string.
return dyn.Map(target, "variables", dyn.Foreach(func(_ dyn.Path, variable dyn.Value) (dyn.Value, error) { return dyn.Map(target, "variables", dyn.Foreach(func(_ dyn.Path, variable dyn.Value) (dyn.Value, error) {
if variable.Kind() != dyn.KindString { switch variable.Kind() {
case dyn.KindString, dyn.KindBool, dyn.KindFloat, dyn.KindInt:
// Rewrite the variable to a map with a single key called "default".
// This conforms to the variable type. Normalization back to the typed
// configuration will convert this to a string if necessary.
return dyn.NewValue(map[string]dyn.Value{
"default": variable,
}, variable.Location()), nil
default:
return variable, nil return variable, nil
} }
// Rewrite the variable to a map with a single key called "default".
// This conforms to the variable type.
return dyn.NewValue(map[string]dyn.Value{
"default": variable,
}, variable.Location()), nil
})) }))
})) }))
} }

View File

@ -0,0 +1,41 @@
bundle:
name: foobar
resources:
pipelines:
my_pipeline:
name: ${var.foo}
continuous: ${var.baz}
clusters:
- num_workers: ${var.bar}
variables:
foo:
default: "a_string"
description: "A string variable"
bar:
default: 42
description: "An integer variable"
baz:
default: true
description: "A boolean variable"
targets:
use-default-variable-values:
override-string-variable:
variables:
foo: "overridden_string"
override-int-variable:
variables:
bar: 43
override-both-bool-and-string-variables:
variables:
foo: "overridden_string"
baz: false

View File

@ -120,3 +120,52 @@ func TestVariablesWithTargetLookupOverrides(t *testing.T) {
assert.Equal(t, "cluster: some-test-cluster", b.Config.Variables["d"].Lookup.String()) 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, "instance-pool: some-test-instance-pool", b.Config.Variables["e"].Lookup.String())
} }
func TestVariableTargetOverrides(t *testing.T) {
var tcases = []struct {
targetName string
pipelineName string
pipelineContinuous bool
pipelineNumWorkers int
}{
{
"use-default-variable-values",
"a_string",
true,
42,
},
{
"override-string-variable",
"overridden_string",
true,
42,
},
{
"override-int-variable",
"a_string",
true,
43,
},
{
"override-both-bool-and-string-variables",
"overridden_string",
false,
42,
},
}
for _, tcase := range tcases {
t.Run(tcase.targetName, func(t *testing.T) {
b := loadTarget(t, "./variables/variable_overrides_in_target", tcase.targetName)
diags := bundle.Apply(context.Background(), b, bundle.Seq(
mutator.SetVariables(),
mutator.ResolveVariableReferences("variables")),
)
require.NoError(t, diags.Error())
assert.Equal(t, tcase.pipelineName, b.Config.Resources.Pipelines["my_pipeline"].Name)
assert.Equal(t, tcase.pipelineContinuous, b.Config.Resources.Pipelines["my_pipeline"].Continuous)
assert.Equal(t, tcase.pipelineNumWorkers, b.Config.Resources.Pipelines["my_pipeline"].Clusters[0].NumWorkers)
})
}
}