make it work and add tests

packer/core.go

@@ -3,6 +3,7 @@ package packer
 import (
 	"fmt"
 	"sort"
+	"strings"
 
 	multierror "github.com/hashicorp/go-multierror"
 	version "github.com/hashicorp/go-version"
@@ -300,31 +301,82 @@ func (c *Core) init() error {
 		c.variables = make(map[string]string)
 	}
 
-	// Go through the variables and interpolate the environment variables
+	// Go through the variables and interpolate the environment and
+	// user variables
+
 	ctx := c.Context()
 	ctx.EnableEnv = true
 	ctx.UserVariables = make(map[string]string)
-	for k, v := range c.Template.Variables {
-		// Ignore variables that are required
-		if v.Required {
-			continue
+	shouldRetry := true
+	tryCount := 0
+	changed := false
+	failedInterpolation := ""
+
+	// Why this giant loop?  User variables can be recursively defined. For
+	// example:
+	// "variables": {
+	//    	"foo":  "bar",
+	//	 	"baz":  "{{user `foo`}}baz",
+	// 		"bang": "bang{{user `baz`}}"
+	// },
+	// In this situation, we cannot guarantee that we've added "foo" to
+	// UserVariables before we try to interpolate "baz" the first time. We need
+	// to have the option to loop back over in order to add the properly
+	// interpolated "baz" to the UserVariables map.
+	// Likewise, we'd need to loop up to two times to properly add "bang",
+	// since that depends on "baz" being set, which depends on "foo" being set.
+
+	// We break out of the while loop either if all our variables have been
+	// interpolated or if after 100 loops we still haven't succeeded in
+	// interpolating them.  Please don't actually nest your variables in 100
+	// layers of other variables. Please.
+
+	for shouldRetry == true {
+		shouldRetry = false
+		for k, v := range c.Template.Variables {
+			// Ignore variables that are required
+			if v.Required {
+				continue
+			}
+
+			// Ignore variables that have a value already
+			if _, ok := c.variables[k]; ok {
+				continue
+			}
+
+			// Interpolate the default
+			def, err := interpolate.Render(v.Default, ctx)
+			if err != nil {
+				if strings.Contains(err.Error(), "error calling user") {
+					shouldRetry = true
+					tryCount++
+					failedInterpolation = fmt.Sprintf(`"%s": "%s"`, k, v.Default)
+					continue
+				} else {
+					return fmt.Errorf(
+						// unexpected interpolation error: abort the run
+						"error interpolating default value for '%s': %s",
+						k, err)
+				}
+			}
+
+			// We only get here if interpolation has succeeded, so something is
+			// different in this loop than in the last one.
+			changed = true
+			c.variables[k] = def
+			ctx.UserVariables = c.variables
+		}
+		if tryCount >= 100 {
+			break
 		}
 
-		// Ignore variables that have a value
-		if _, ok := c.variables[k]; ok {
-			continue
-		}
+	}
 
-		// Interpolate the default
-		def, err := interpolate.Render(v.Default, ctx)
-		if err != nil {
-			return fmt.Errorf(
-				"error interpolating default value for '%s': %s",
-				k, err)
-		}
-
-		c.variables[k] = def
-		ctx.UserVariables = c.variables
+	if (changed == false) && (shouldRetry == true) {
+		return fmt.Errorf("Failed to interpolate %s: Please make sure that "+
+			"the variable you're referencing has been defined; Packer treats "+
+			"all variables used to interpolate other user varaibles as "+
+			"required.", failedInterpolation)
 	}
 
 	for _, v := range c.Template.SensitiveVariables {

packer/core_test.go

@@ -523,6 +523,58 @@ func TestCoreValidate(t *testing.T) {
 	}
 }
 
+func TestCore_InterpolateUserVars(t *testing.T) {
+	cases := []struct {
+		File     string
+		Expected map[string]string
+		Err      bool
+	}{
+		{
+			"variables.json",
+			map[string]string{
+				"foo":  "bar",
+				"bar":  "bar",
+				"baz":  "barbaz",
+				"bang": "bangbarbaz",
+			},
+			false,
+		},
+		{
+			"variables2.json",
+			map[string]string{},
+			true,
+		},
+	}
+	for _, tc := range cases {
+		f, err := os.Open(fixtureDir(tc.File))
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		tpl, err := template.Parse(f)
+		f.Close()
+		if err != nil {
+			t.Fatalf("err: %s\n\n%s", tc.File, err)
+		}
+
+		ccf, err := NewCore(&CoreConfig{
+			Template: tpl,
+			Version:  "1.0.0",
+		})
+
+		if (err != nil) != tc.Err {
+			t.Fatalf("err: %s\n\n%s", tc.File, err)
+		}
+		if !tc.Err {
+			for k, v := range ccf.variables {
+				if tc.Expected[k] != v {
+					t.Fatalf("Expected %s but got %s", tc.Expected[k], v)
+				}
+			}
+		}
+	}
+}
+
 func TestSensitiveVars(t *testing.T) {
 	cases := []struct {
 		File          string

template/interpolate/funcs.go

@@ -163,7 +163,15 @@ func funcGenUser(ctx *Context) interface{} {
 			return "", errors.New("test")
 		}
 
-		return ctx.UserVariables[k], nil
+		val, ok := ctx.UserVariables[k]
+		if ctx.EnableEnv {
+			// error and retry if we're interpolating UserVariables. But if
+			// we're elsewhere in the template, just return the empty string.
+			if !ok {
+				return "", errors.New(fmt.Sprintf("variable %s not set", k))
+			}
+		}
+		return val, nil
 	}
 }