From 33f25acb089dfce6661ec77ec3244d3851285505 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 9 Feb 2021 23:57:24 -0800 Subject: [PATCH] bake: allow variables to reference each other Signed-off-by: Tonis Tiigi --- bake/bake.go | 4 +- bake/hcl.go | 140 ++++++++++++++++++++++++++++++++++++++++----- bake/hcl_test.go | 144 ++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 245 insertions(+), 43 deletions(-) diff --git a/bake/bake.go b/bake/bake.go index 76dda4f3f53..ac7adcd6c9e 100644 --- a/bake/bake.go +++ b/bake/bake.go @@ -402,8 +402,8 @@ func (c Config) target(name string, visited map[string]struct{}, overrides map[s } type Variable struct { - Name string `json:"-" hcl:"name,label"` - Default string `json:"default,omitempty" hcl:"default,optional"` + Name string `json:"-" hcl:"name,label"` + Default *hcl.Attribute `json:"default,omitempty" hcl:"default,optional"` } type Group struct { diff --git a/bake/hcl.go b/bake/hcl.go index dfc94267939..de9d1840ba1 100644 --- a/bake/hcl.go +++ b/bake/hcl.go @@ -1,7 +1,10 @@ package bake import ( + "math" + "math/big" "os" + "strconv" "strings" "github.com/hashicorp/go-cty-funcs/cidr" @@ -17,6 +20,7 @@ import ( hcljson "github.com/hashicorp/hcl/v2/json" "github.com/moby/buildkit/solver/errdefs" "github.com/moby/buildkit/solver/pb" + "github.com/pkg/errors" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/function" "github.com/zclconf/go-cty/cty/function/stdlib" @@ -125,11 +129,14 @@ var ( } ) -// Used in the first pass of decoding instead of the Config struct to disallow -// interpolation while parsing variable blocks. type StaticConfig struct { Variables []*Variable `hcl:"variable,block"` Remain hcl.Body `hcl:",remain"` + + defaults map[string]*hcl.Attribute + env map[string]string + values map[string]cty.Value + progress map[string]struct{} } func mergeStaticConfig(scs []*StaticConfig) *StaticConfig { @@ -143,6 +150,118 @@ func mergeStaticConfig(scs []*StaticConfig) *StaticConfig { return sc } +func (sc *StaticConfig) Values(withEnv bool) (map[string]cty.Value, error) { + sc.defaults = map[string]*hcl.Attribute{} + for _, v := range sc.Variables { + sc.defaults[v.Name] = v.Default + } + + sc.env = map[string]string{} + if withEnv { + // Override default with values from environment. + for _, v := range os.Environ() { + parts := strings.SplitN(v, "=", 2) + name, value := parts[0], parts[1] + sc.env[name] = value + } + } + + sc.values = map[string]cty.Value{} + sc.progress = map[string]struct{}{} + + for k := range sc.defaults { + if _, err := sc.resolveValue(k); err != nil { + return nil, err + } + } + return sc.values, nil +} + +func (sc *StaticConfig) resolveValue(name string) (v *cty.Value, err error) { + if v, ok := sc.values[name]; ok { + return &v, nil + } + if _, ok := sc.progress[name]; ok { + return nil, errors.Errorf("variable cycle not allowed") + } + sc.progress[name] = struct{}{} + + defer func() { + if v != nil { + sc.values[name] = *v + } + }() + + def, ok := sc.defaults[name] + + if !ok { + return nil, errors.Errorf("undefined variable %q", name) + } + + if def == nil { + v := cty.StringVal(sc.env[name]) + return &v, nil + } + + ectx := &hcl.EvalContext{ + Variables: map[string]cty.Value{}, + Functions: stdlibFunctions, // user functions not possible atm + } + for _, v := range def.Expr.Variables() { + value, err := sc.resolveValue(v.RootName()) + if err != nil { + var diags hcl.Diagnostics + if !errors.As(err, &diags) { + return nil, err + } + r := v.SourceRange() + return nil, hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid expression", + Detail: err.Error(), + Subject: &r, + Context: &r, + }, + } + } + ectx.Variables[v.RootName()] = *value + } + + vv, diags := def.Expr.Value(ectx) + if diags.HasErrors() { + return nil, diags + } + + if envv, ok := sc.env[name]; ok { + if vv.Type().Equals(cty.Bool) { + b, err := strconv.ParseBool(envv) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse %s as bool", name) + } + v := cty.BoolVal(b) + return &v, nil + } else if vv.Type().Equals(cty.String) { + v := cty.StringVal(envv) + return &v, nil + } else if vv.Type().Equals(cty.Number) { + n, err := strconv.ParseFloat(envv, 64) + if err == nil && (math.IsNaN(n) || math.IsInf(n, 0)) { + err = errors.Errorf("invalid number value") + } + if err != nil { + return nil, errors.Wrapf(err, "failed to parse %s as number", name) + } + v := cty.NumberVal(big.NewFloat(n)) + return &v, nil + } else { + // TODO: support lists with csv values + return nil, errors.Errorf("unsupported type %s for variable %s", v.Type(), name) + } + } + return &vv, nil +} + func ParseHCLFile(dt []byte, fn string) (*hcl.File, *StaticConfig, error) { if strings.HasSuffix(fn, ".json") || strings.HasSuffix(fn, ".hcl") { return parseHCLFile(dt, fn) @@ -188,10 +307,10 @@ func parseHCLFile(dt []byte, fn string) (f *hcl.File, _ *StaticConfig, err error func ParseHCL(b hcl.Body, sc *StaticConfig) (_ *Config, err error) { - // Set all variables to their default value if defined. - variables := make(map[string]cty.Value) - for _, variable := range sc.Variables { - variables[variable.Name] = cty.StringVal(variable.Default) + // evaluate variables + variables, err := sc.Values(true) + if err != nil { + return nil, err } userFunctions, _, diags := userfunc.DecodeUserFunctions(b, "function", func() *hcl.EvalContext { @@ -204,15 +323,6 @@ func ParseHCL(b hcl.Body, sc *StaticConfig) (_ *Config, err error) { return nil, diags } - // Override default with values from environment. - for _, env := range os.Environ() { - parts := strings.SplitN(env, "=", 2) - name, value := parts[0], parts[1] - if _, ok := variables[name]; ok { - variables[name] = cty.StringVal(value) - } - } - functions := make(map[string]function.Function) for k, v := range stdlibFunctions { functions[k] = v diff --git a/bake/hcl_test.go b/bake/hcl_test.go index 2b7bdb57e52..1429f0256be 100644 --- a/bake/hcl_test.go +++ b/bake/hcl_test.go @@ -223,32 +223,6 @@ func TestHCLWithVariables(t *testing.T) { require.Equal(t, "456", c.Targets[0].Args["buildno"]) } -func TesstHCLWithIncorrectVariables(t *testing.T) { - dt := []byte(` - variable "DEFAULT_BUILD_NUMBER" { - default = "1" - } - - variable "BUILD_NUMBER" { - default = "${DEFAULT_BUILD_NUMBER}" - } - - group "default" { - targets = ["webapp"] - } - - target "webapp" { - args = { - buildno = "${BUILD_NUMBER}" - } - } - `) - - _, err := ParseFile(dt, "docker-bake.hcl") - require.Error(t, err) - require.Contains(t, err.Error(), "docker-bake.hcl:7,17-37: Variables not allowed; Variables may not be used here.") -} - func TestHCLWithVariablesInFunctions(t *testing.T) { dt := []byte(` variable "REPO" { @@ -323,3 +297,121 @@ func TestHCLMultiFileSharedVariables(t *testing.T) { require.Equal(t, "pre-def", c.Targets[0].Args["v1"]) require.Equal(t, "def-post", c.Targets[0].Args["v2"]) } + +func TestHCLVarsWithVars(t *testing.T) { + os.Unsetenv("FOO") + dt := []byte(` + variable "FOO" { + default = upper("${BASE}def") + } + variable "BAR" { + default = "-${FOO}-" + } + target "app" { + args = { + v1 = "pre-${BAR}" + } + } + `) + dt2 := []byte(` + variable "BASE" { + default = "abc" + } + target "app" { + args = { + v2 = "${FOO}-post" + } + } + `) + + c, err := parseFiles([]File{ + {Data: dt, Name: "c1.hcl"}, + {Data: dt2, Name: "c2.hcl"}, + }) + require.NoError(t, err) + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, c.Targets[0].Name, "app") + require.Equal(t, "pre--ABCDEF-", c.Targets[0].Args["v1"]) + require.Equal(t, "ABCDEF-post", c.Targets[0].Args["v2"]) + + os.Setenv("BASE", "new") + + c, err = parseFiles([]File{ + {Data: dt, Name: "c1.hcl"}, + {Data: dt2, Name: "c2.hcl"}, + }) + require.NoError(t, err) + + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, c.Targets[0].Name, "app") + require.Equal(t, "pre--NEWDEF-", c.Targets[0].Args["v1"]) + require.Equal(t, "NEWDEF-post", c.Targets[0].Args["v2"]) +} + +func TestHCLTypedVariables(t *testing.T) { + os.Unsetenv("FOO") + dt := []byte(` + variable "FOO" { + default = 3 + } + variable "IS_FOO" { + default = true + } + target "app" { + args = { + v1 = FOO > 5 ? "higher" : "lower" + v2 = IS_FOO ? "yes" : "no" + } + } + `) + + c, err := ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, c.Targets[0].Name, "app") + require.Equal(t, "lower", c.Targets[0].Args["v1"]) + require.Equal(t, "yes", c.Targets[0].Args["v2"]) + + os.Setenv("FOO", "5.1") + os.Setenv("IS_FOO", "0") + + c, err = ParseFile(dt, "docker-bake.hcl") + require.NoError(t, err) + + require.Equal(t, 1, len(c.Targets)) + require.Equal(t, c.Targets[0].Name, "app") + require.Equal(t, "higher", c.Targets[0].Args["v1"]) + require.Equal(t, "no", c.Targets[0].Args["v2"]) + + os.Setenv("FOO", "NaN") + _, err = ParseFile(dt, "docker-bake.hcl") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse FOO as number") + + os.Setenv("FOO", "0") + os.Setenv("IS_FOO", "maybe") + + _, err = ParseFile(dt, "docker-bake.hcl") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse IS_FOO as bool") +} + +func TestHCLVariableCycle(t *testing.T) { + dt := []byte(` + variable "FOO" { + default = BAR + } + variable "FOO2" { + default = FOO + } + variable "BAR" { + default = FOO + } + target "app" {} + `) + + _, err := ParseFile(dt, "docker-bake.hcl") + require.Error(t, err) + require.Contains(t, err.Error(), "variable cycle not allowed") +}