Merge pull request #539 from tonistiigi/vars-cross-refs

bake: allow variables to reference each other

bake/bake.go

@@ -403,7 +403,7 @@ 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"`
+	Default *hcl.Attribute `json:"default,omitempty" hcl:"default,optional"`
 }
 
 type Group struct {

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.
+	// evaluate variables
-	variables := make(map[string]cty.Value)
+	variables, err := sc.Values(true)
-	for _, variable := range sc.Variables {
+	if err != nil {
-		variables[variable.Name] = cty.StringVal(variable.Default)
+		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

bake/hcl_test.go

@@ -7,10 +7,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestParseHCL(t *testing.T) {
+func TestHCLBasic(t *testing.T) {
 	t.Parallel()
-
-	t.Run("Basic", func(t *testing.T) {
 	dt := []byte(`
 		group "default" {
 			targets = ["db", "webapp"]
@@ -66,9 +64,9 @@ func TestParseHCL(t *testing.T) {
 	require.Equal(t, c.Targets[3].Name, "webapp-plus")
 	require.Equal(t, 1, len(c.Targets[3].Args))
 	require.Equal(t, map[string]string{"IAMCROSS": "true"}, c.Targets[3].Args)
-	})
+}
 
-	t.Run("BasicInJSON", func(t *testing.T) {
+func TestHCLBasicInJSON(t *testing.T) {
 	dt := []byte(`
 		{
 			"group": {
@@ -126,9 +124,9 @@ func TestParseHCL(t *testing.T) {
 	require.Equal(t, c.Targets[3].Name, "webapp-plus")
 	require.Equal(t, 1, len(c.Targets[3].Args))
 	require.Equal(t, map[string]string{"IAMCROSS": "true"}, c.Targets[3].Args)
-	})
+}
 
-	t.Run("WithFunctions", func(t *testing.T) {
+func TestHCLWithFunctions(t *testing.T) {
 	dt := []byte(`
 		group "default" {
 			targets = ["webapp"]
@@ -151,9 +149,9 @@ func TestParseHCL(t *testing.T) {
 	require.Equal(t, 1, len(c.Targets))
 	require.Equal(t, c.Targets[0].Name, "webapp")
 	require.Equal(t, "124", c.Targets[0].Args["buildno"])
-	})
+}
 
-	t.Run("WithUserDefinedFunctions", func(t *testing.T) {
+func TestHCLWithUserDefinedFunctions(t *testing.T) {
 	dt := []byte(`
 		function "increment" {
 			params = [number]
@@ -181,9 +179,9 @@ func TestParseHCL(t *testing.T) {
 	require.Equal(t, 1, len(c.Targets))
 	require.Equal(t, c.Targets[0].Name, "webapp")
 	require.Equal(t, "124", c.Targets[0].Args["buildno"])
-	})
+}
 
-	t.Run("WithVariables", func(t *testing.T) {
+func TestHCLWithVariables(t *testing.T) {
 	dt := []byte(`
 		variable "BUILD_NUMBER" {
 			default = "123"
@@ -223,35 +221,9 @@ func TestParseHCL(t *testing.T) {
 	require.Equal(t, 1, len(c.Targets))
 	require.Equal(t, c.Targets[0].Name, "webapp")
 	require.Equal(t, "456", c.Targets[0].Args["buildno"])
-	})
+}
-
-	t.Run("WithIncorrectVariables", func(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.")
-	})
 
-	t.Run("WithVariablesInFunctions", func(t *testing.T) {
+func TestHCLWithVariablesInFunctions(t *testing.T) {
 	dt := []byte(`
 		variable "REPO" {
 			default = "user/repo"
@@ -281,9 +253,9 @@ func TestParseHCL(t *testing.T) {
 	require.Equal(t, 1, len(c.Targets))
 	require.Equal(t, c.Targets[0].Name, "webapp")
 	require.Equal(t, []string{"docker/buildx:v1"}, c.Targets[0].Tags)
-	})
+}
 
-	t.Run("MultiFileSharedVariables", func(t *testing.T) {
+func TestHCLMultiFileSharedVariables(t *testing.T) {
 	dt := []byte(`
 		variable "FOO" {
 			default = "abc"
@@ -324,5 +296,122 @@ func TestParseHCL(t *testing.T) {
 	require.Equal(t, c.Targets[0].Name, "app")
 	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")
 }