packer: template now handles user var logic

packer/build.go

@@ -40,7 +40,7 @@ type Build interface {
 	// Prepare configures the various components of this build and reports
 	// any errors in doing so (such as syntax errors, validation errors, etc.).
 	// It also reports any warnings.
-	Prepare(v map[string]string) ([]string, error)
+	Prepare() ([]string, error)
 
 	// Run runs the actual builder, returning an artifact implementation
 	// of what is built. If anything goes wrong, an error is returned.
@@ -78,7 +78,7 @@ type coreBuild struct {
 	hooks          map[string][]Hook
 	postProcessors [][]coreBuildPostProcessor
 	provisioners   []coreBuildProvisioner
-	variables      map[string]coreBuildVariable
+	variables      map[string]string
 
 	debug         bool
 	force         bool
@@ -102,12 +102,6 @@ type coreBuildProvisioner struct {
 	config      []interface{}
 }
 
-// A user-variable that is part of a single build.
-type coreBuildVariable struct {
-	Default  string
-	Required bool
-}
-
 // Returns the name of the build.
 func (b *coreBuild) Name() string {
 	return b.name
@@ -116,7 +110,7 @@ func (b *coreBuild) Name() string {
 // Prepare prepares the build by doing some initialization for the builder
 // and any hooks. This _must_ be called prior to Run. The parameter is the
 // overrides for the variables within the template (if any).
-func (b *coreBuild) Prepare(userVars map[string]string) (warn []string, err error) {
+func (b *coreBuild) Prepare() (warn []string, err error) {
 	b.l.Lock()
 	defer b.l.Unlock()
 
@@ -126,46 +120,12 @@ func (b *coreBuild) Prepare(userVars map[string]string) (warn []string, err erro
 
 	b.prepareCalled = true
 
-	// Compile the variables
-	varErrs := make([]error, 0)
-	variables := make(map[string]string)
-	for k, v := range b.variables {
-		variables[k] = v.Default
-
-		if v.Required {
-			if _, ok := userVars[k]; !ok {
-				varErrs = append(varErrs,
-					fmt.Errorf("Required user variable '%s' not set", k))
-			}
-		}
-	}
-
-	if userVars != nil {
-		for k, v := range userVars {
-			if _, ok := variables[k]; !ok {
-				varErrs = append(
-					varErrs, fmt.Errorf("Unknown user variable: %s", k))
-				continue
-			}
-
-			variables[k] = v
-		}
-	}
-
-	// If there were any problem with variables, return an error right
-	// away because we can't be certain anything else will actually work.
-	if len(varErrs) > 0 {
-		return nil, &MultiError{
-			Errors: varErrs,
-		}
-	}
-
 	packerConfig := map[string]interface{}{
 		BuildNameConfigKey:     b.name,
 		BuilderTypeConfigKey:   b.builderType,
 		DebugConfigKey:         b.debug,
 		ForceConfigKey:         b.force,
-		UserVariablesConfigKey: variables,
+		UserVariablesConfigKey: b.variables,
 	}
 
 	// Prepare the builder

packer/build_test.go

@@ -22,7 +22,7 @@ func testBuild() *coreBuild {
 				coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp"}, "testPP", make(map[string]interface{}), true},
 			},
 		},
-		variables: make(map[string]coreBuildVariable),
+		variables: make(map[string]string),
 	}
 }
 
@@ -48,7 +48,7 @@ func TestBuild_Prepare(t *testing.T) {
 	build := testBuild()
 	builder := build.builder.(*MockBuilder)
 
-	build.Prepare(nil)
+	build.Prepare()
 	if !builder.PrepareCalled {
 		t.Fatal("should be called")
 	}
@@ -77,7 +77,7 @@ func TestBuild_Prepare(t *testing.T) {
 
 func TestBuild_Prepare_Twice(t *testing.T) {
 	build := testBuild()
-	warn, err := build.Prepare(nil)
+	warn, err := build.Prepare()
 	if len(warn) > 0 {
 		t.Fatalf("bad: %#v", warn)
 	}
@@ -96,7 +96,7 @@ func TestBuild_Prepare_Twice(t *testing.T) {
 		}
 	}()
 
-	build.Prepare(nil)
+	build.Prepare()
 }
 
 func TestBuildPrepare_BuilderWarniings(t *testing.T) {
@@ -106,7 +106,7 @@ func TestBuildPrepare_BuilderWarniings(t *testing.T) {
 	builder := build.builder.(*MockBuilder)
 	builder.PrepareWarnings = expected
 
-	warn, err := build.Prepare(nil)
+	warn, err := build.Prepare()
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -123,7 +123,7 @@ func TestBuild_Prepare_Debug(t *testing.T) {
 	builder := build.builder.(*MockBuilder)
 
 	build.SetDebug(true)
-	build.Prepare(nil)
+	build.Prepare()
 	if !builder.PrepareCalled {
 		t.Fatalf("should be called")
 	}
@@ -148,10 +148,10 @@ func TestBuildPrepare_variables_default(t *testing.T) {
 	}
 
 	build := testBuild()
-	build.variables["foo"] = coreBuildVariable{Default: "bar"}
+	build.variables["foo"] = "bar"
 	builder := build.builder.(*MockBuilder)
 
-	warn, err := build.Prepare(nil)
+	warn, err := build.Prepare()
 	if len(warn) > 0 {
 		t.Fatalf("bad: %#v", warn)
 	}
@@ -168,76 +168,12 @@ func TestBuildPrepare_variables_default(t *testing.T) {
 	}
 }
 
-func TestBuildPrepare_variables_nonexist(t *testing.T) {
-	build := testBuild()
-	build.variables["foo"] = coreBuildVariable{Default: "bar"}
-
-	warn, err := build.Prepare(map[string]string{"bar": "baz"})
-	if len(warn) > 0 {
-		t.Fatalf("bad: %#v", warn)
-	}
-	if err == nil {
-		t.Fatal("should have had error")
-	}
-}
-
-func TestBuildPrepare_variables_override(t *testing.T) {
-	packerConfig := testDefaultPackerConfig()
-	packerConfig[UserVariablesConfigKey] = map[string]string{
-		"foo": "baz",
-	}
-
-	build := testBuild()
-	build.variables["foo"] = coreBuildVariable{Default: "bar"}
-	builder := build.builder.(*MockBuilder)
-
-	warn, err := build.Prepare(map[string]string{"foo": "baz"})
-	if len(warn) > 0 {
-		t.Fatalf("bad: %#v", warn)
-	}
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if !builder.PrepareCalled {
-		t.Fatal("prepare should be called")
-	}
-
-	if !reflect.DeepEqual(builder.PrepareConfig[1], packerConfig) {
-		t.Fatalf("prepare bad: %#v", builder.PrepareConfig[1])
-	}
-}
-
-func TestBuildPrepare_variablesRequired(t *testing.T) {
-	build := testBuild()
-	build.variables["foo"] = coreBuildVariable{Required: true}
-
-	warn, err := build.Prepare(map[string]string{})
-	if len(warn) > 0 {
-		t.Fatalf("bad: %#v", warn)
-	}
-	if err == nil {
-		t.Fatal("should have had error")
-	}
-
-	// Test with setting the value
-	build = testBuild()
-	build.variables["foo"] = coreBuildVariable{Required: true}
-	warn, err = build.Prepare(map[string]string{"foo": ""})
-	if len(warn) > 0 {
-		t.Fatalf("bad: %#v", warn)
-	}
-	if err != nil {
-		t.Fatalf("should not have error: %s", err)
-	}
-}
-
 func TestBuild_Run(t *testing.T) {
 	cache := &TestCache{}
 	ui := testUi()
 
 	build := testBuild()
-	build.Prepare(nil)
+	build.Prepare()
 	artifacts, err := build.Run(ui, cache)
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -287,7 +223,7 @@ func TestBuild_Run_Artifacts(t *testing.T) {
 	build := testBuild()
 	build.postProcessors = [][]coreBuildPostProcessor{}
 
-	build.Prepare(nil)
+	build.Prepare()
 	artifacts, err := build.Run(ui, cache)
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -312,7 +248,7 @@ func TestBuild_Run_Artifacts(t *testing.T) {
 		},
 	}
 
-	build.Prepare(nil)
+	build.Prepare()
 	artifacts, err = build.Run(ui, cache)
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -340,7 +276,7 @@ func TestBuild_Run_Artifacts(t *testing.T) {
 		},
 	}
 
-	build.Prepare(nil)
+	build.Prepare()
 	artifacts, err = build.Run(ui, cache)
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -370,7 +306,7 @@ func TestBuild_Run_Artifacts(t *testing.T) {
 		},
 	}
 
-	build.Prepare(nil)
+	build.Prepare()
 	artifacts, err = build.Run(ui, cache)
 	if err != nil {
 		t.Fatalf("err: %s", err)
@@ -397,7 +333,7 @@ func TestBuild_Run_Artifacts(t *testing.T) {
 		},
 	}
 
-	build.Prepare(nil)
+	build.Prepare()
 	artifacts, err = build.Run(ui, cache)
 	if err != nil {
 		t.Fatalf("err: %s", err)

packer/template.go

@@ -75,8 +75,10 @@ type RawProvisionerConfig struct {
 
 // RawVariable represents a variable configuration within a template.
 type RawVariable struct {
-	Default  string
-	Required bool
+	Default  string // The default value for this variable
+	Required bool   // If the variable is required or not
+	Value    string // The set value for this variable
+	HasValue bool   // True if the value was set
 }
 
 // ParseTemplate takes a byte slice and parses a Template from it, returning
@@ -84,7 +86,9 @@ type RawVariable struct {
 // could potentially be a MultiError, representing multiple errors. Knowing
 // and checking for this can be useful, if you wish to format it in a certain
 // way.
-func ParseTemplate(data []byte) (t *Template, err error) {
+//
+// The second parameter, vars, are the values for a set of user variables.
+func ParseTemplate(data []byte, vars map[string]string) (t *Template, err error) {
 	var rawTplInterface interface{}
 	err = jsonutil.Unmarshal(data, &rawTplInterface)
 	if err != nil {
@@ -152,6 +156,13 @@ func ParseTemplate(data []byte) (t *Template, err error) {
 			continue
 		}
 
+		// Set the value of this variable if we have it
+		if val, ok := vars[k]; ok {
+			variable.HasValue = true
+			variable.Value = val
+			delete(vars, k)
+		}
+
 		t.Variables[k] = variable
 	}
 
@@ -313,6 +324,11 @@ func ParseTemplate(data []byte) (t *Template, err error) {
 		errors = append(errors, fmt.Errorf("No builders are defined in the template."))
 	}
 
+	// Verify that all the variable sets were for real variables.
+	for k, _ := range vars {
+		errors = append(errors, fmt.Errorf("Unknown user variables: %s", k))
+	}
+
 	// If there were errors, we put it into a MultiError and return
 	if len(errors) > 0 {
 		err = &MultiError{errors}
@@ -325,7 +341,7 @@ func ParseTemplate(data []byte) (t *Template, err error) {
 
 // ParseTemplateFile takes the given template file and parses it into
 // a single template.
-func ParseTemplateFile(path string) (*Template, error) {
+func ParseTemplateFile(path string, vars map[string]string) (*Template, error) {
 	var data []byte
 
 	if path == "-" {
@@ -345,7 +361,7 @@ func ParseTemplateFile(path string) (*Template, error) {
 		}
 	}
 
-	return ParseTemplate(data)
+	return ParseTemplate(data, vars)
 }
 
 func parsePostProcessor(i int, rawV interface{}) (result []map[string]interface{}, errors []error) {
@@ -527,12 +543,24 @@ func (t *Template) Build(name string, components *ComponentFinder) (b Build, err
 	}
 
 	// Prepare the variables
-	variables := make(map[string]coreBuildVariable)
+	var varErrors []error
+	variables := make(map[string]string)
 	for k, v := range t.Variables {
-		variables[k] = coreBuildVariable{
-			Default:  v.Default,
-			Required: v.Required,
+		if v.Required && !v.HasValue {
+			varErrors = append(varErrors,
+				fmt.Errorf("Required user variable '%s' not set", k))
 		}
+
+		var val string = v.Default
+		if v.HasValue {
+			val = v.Value
+		}
+
+		variables[k] = val
+	}
+
+	if len(varErrors) > 0 {
+		return nil, &MultiError{varErrors}
 	}
 
 	b = &coreBuild{