packer: required user variables [GH-374]

CHANGELOG.md

 ## 0.3.6 (unreleased)
 
+FEATURES:
+
+* User variables can now be specified as "required", meaning the user
+  MUST specify a value. Just set the default value to "null". [GH-374]
+
 IMPROVEMENTS:
 
 * core: Much improved interrupt handling. For example, interrupts now

packer/build.go

@@ -77,7 +77,7 @@ type coreBuild struct {
 	hooks          map[string][]Hook
 	postProcessors [][]coreBuildPostProcessor
 	provisioners   []coreBuildProvisioner
-	variables      map[string]string
+	variables      map[string]coreBuildVariable
 
 	debug         bool
 	force         bool
@@ -101,6 +101,12 @@ 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
@@ -122,25 +128,41 @@ func (b *coreBuild) Prepare(userVars map[string]string) (err error) {
 	// Compile the variables
 	variables := make(map[string]string)
 	for k, v := range b.variables {
-		variables[k] = v
+		if !v.Required {
+			variables[k] = v.Default
+		}
 	}
 
+	varErrs := make([]error, 0)
 	if userVars != nil {
-		errs := make([]error, 0)
 		for k, v := range userVars {
 			if _, ok := variables[k]; !ok {
-				errs = append(
-					errs, fmt.Errorf("Unknown user variable: %s", k))
+				varErrs = append(
+					varErrs, fmt.Errorf("Unknown user variable: %s", k))
 				continue
 			}
 
 			variables[k] = v
 		}
+	}
 
-		if len(errs) > 0 {
-			return &MultiError{
-				Errors: errs,
-			}
+	// Verify all required variables have been set.
+	for k, v := range b.variables {
+		if !v.Required {
+			continue
+		}
+
+		if _, ok := variables[k]; !ok {
+			varErrs = append(
+				varErrs, fmt.Errorf("Required user variable '%s' not set", k))
+		}
+	}
+
+	// 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 &MultiError{
+			Errors: varErrs,
 		}
 	}
 

packer/build_test.go

@@ -23,7 +23,7 @@ func testBuild() *coreBuild {
 				coreBuildPostProcessor{&TestPostProcessor{artifactId: "pp"}, "testPP", make(map[string]interface{}), true},
 			},
 		},
-		variables: make(map[string]string),
+		variables: make(map[string]coreBuildVariable),
 	}
 }
 
@@ -116,7 +116,7 @@ func TestBuildPrepare_variables_default(t *testing.T) {
 	}
 
 	build := testBuild()
-	build.variables["foo"] = "bar"
+	build.variables["foo"] = coreBuildVariable{Default: "bar"}
 	builder := build.builder.(*TestBuilder)
 
 	err := build.Prepare(nil)
@@ -135,7 +135,7 @@ func TestBuildPrepare_variables_default(t *testing.T) {
 
 func TestBuildPrepare_variables_nonexist(t *testing.T) {
 	build := testBuild()
-	build.variables["foo"] = "bar"
+	build.variables["foo"] = coreBuildVariable{Default: "bar"}
 
 	err := build.Prepare(map[string]string{"bar": "baz"})
 	if err == nil {
@@ -150,7 +150,7 @@ func TestBuildPrepare_variables_override(t *testing.T) {
 	}
 
 	build := testBuild()
-	build.variables["foo"] = "bar"
+	build.variables["foo"] = coreBuildVariable{Default: "bar"}
 	builder := build.builder.(*TestBuilder)
 
 	err := build.Prepare(map[string]string{"foo": "baz"})
@@ -167,6 +167,16 @@ func TestBuildPrepare_variables_override(t *testing.T) {
 	}
 }
 
+func TestBuildPrepare_variablesRequired(t *testing.T) {
+	build := testBuild()
+	build.variables["foo"] = coreBuildVariable{Required: true}
+
+	err := build.Prepare(map[string]string{})
+	if err == nil {
+		t.Fatal("should have had error")
+	}
+}
+
 func TestBuild_Run(t *testing.T) {
 	assert := asserts.NewTestingAsserts(t, true)
 

packer/template.go

@@ -16,7 +16,7 @@ import (
 // "interface{}" pointers since we actually don't know what their contents
 // are until we read the "type" field.
 type rawTemplate struct {
-	Variables      map[string]string
+	Variables      map[string]interface{}
 	Builders       []map[string]interface{}
 	Hooks          map[string][]string
 	Provisioners   []map[string]interface{}
@@ -26,7 +26,7 @@ type rawTemplate struct {
 // The Template struct represents a parsed template, parsed into the most
 // completed form it can be without additional processing by the caller.
 type Template struct {
-	Variables      map[string]string
+	Variables      map[string]RawVariable
 	Builders       map[string]RawBuilderConfig
 	Hooks          map[string][]string
 	PostProcessors [][]RawPostProcessorConfig
@@ -63,6 +63,12 @@ type RawProvisionerConfig struct {
 	RawConfig interface{}
 }
 
+// RawVariable represents a variable configuration within a template.
+type RawVariable struct {
+	Default  string
+	Required bool
+}
+
 // ParseTemplate takes a byte slice and parses a Template from it, returning
 // the template and possibly errors while loading the template. The error
 // could potentially be a MultiError, representing multiple errors. Knowing
@@ -105,7 +111,7 @@ func ParseTemplate(data []byte) (t *Template, err error) {
 	}
 
 	t = &Template{}
-	t.Variables = make(map[string]string)
+	t.Variables = make(map[string]RawVariable)
 	t.Builders = make(map[string]RawBuilderConfig)
 	t.Hooks = rawTpl.Hooks
 	t.PostProcessors = make([][]RawPostProcessorConfig, len(rawTpl.PostProcessors))
@@ -113,7 +119,22 @@ func ParseTemplate(data []byte) (t *Template, err error) {
 
 	// Gather all the variables
 	for k, v := range rawTpl.Variables {
-		t.Variables[k] = v
+		var variable RawVariable
+		variable.Default = ""
+		variable.Required = v == nil
+
+		if v != nil {
+			def, ok := v.(string)
+			if !ok {
+				errors = append(errors,
+					fmt.Errorf("variable '%s': default value must be string or null", k))
+				continue
+			}
+
+			variable.Default = def
+		}
+
+		t.Variables[k] = variable
 	}
 
 	// Gather all the builders
@@ -428,6 +449,15 @@ func (t *Template) Build(name string, components *ComponentFinder) (b Build, err
 		provisioners = append(provisioners, coreProv)
 	}
 
+	// Prepare the variables
+	variables := make(map[string]coreBuildVariable)
+	for k, v := range t.Variables {
+		variables[k] = coreBuildVariable{
+			Default:  v.Default,
+			Required: v.Required,
+		}
+	}
+
 	b = &coreBuild{
 		name:           name,
 		builder:        builder,
@@ -436,7 +466,7 @@ func (t *Template) Build(name string, components *ComponentFinder) (b Build, err
 		hooks:          hooks,
 		postProcessors: postProcessors,
 		provisioners:   provisioners,
-		variables:      t.Variables,
+		variables:      variables,
 	}
 
 	return

packer/template_test.go

@@ -364,7 +364,7 @@ func TestParseTemplate_Variables(t *testing.T) {
 	{
 		"variables": {
 			"foo": "bar",
-			"bar": ""
+			"bar": null
 		},
 
 		"builders": [{"type": "something"}]
@@ -379,6 +379,39 @@ func TestParseTemplate_Variables(t *testing.T) {
 	if result.Variables == nil || len(result.Variables) != 2 {
 		t.Fatalf("bad vars: %#v", result.Variables)
 	}
+
+	if result.Variables["foo"].Default != "bar" {
+		t.Fatal("foo default is not right")
+	}
+
+	if result.Variables["foo"].Required {
+		t.Fatal("foo should not be required")
+	}
+
+	if result.Variables["bar"].Default != "" {
+		t.Fatal("default should be empty")
+	}
+
+	if !result.Variables["bar"].Required {
+		t.Fatal("bar should be required")
+	}
+}
+
+func TestParseTemplate_variablesBadDefault(t *testing.T) {
+	data := `
+	{
+		"variables": {
+			"foo": 7,
+		},
+
+		"builders": [{"type": "something"}]
+	}
+	`
+
+	_, err := ParseTemplate([]byte(data))
+	if err == nil {
+		t.Fatal("should have error")
+	}
 }
 
 func TestTemplate_BuildNames(t *testing.T) {