From fd4b01cf8595a6a2cc77c7617c71c1fb3296aff5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 31 Aug 2013 17:33:17 -0700 Subject: [PATCH] packer: required user variables [GH-374] --- CHANGELOG.md | 5 +++++ packer/build.go | 40 +++++++++++++++++++++++++++++++--------- packer/build_test.go | 18 ++++++++++++++---- packer/template.go | 40 +++++++++++++++++++++++++++++++++++----- packer/template_test.go | 35 ++++++++++++++++++++++++++++++++++- 5 files changed, 119 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c6d2891..05a27871e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## 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 diff --git a/packer/build.go b/packer/build.go index e324fdea7..d116f8684 100644 --- a/packer/build.go +++ b/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, } } diff --git a/packer/build_test.go b/packer/build_test.go index cbac0f208..ed05e8df7 100644 --- a/packer/build_test.go +++ b/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) diff --git a/packer/template.go b/packer/template.go index 4ac995f5e..7c3dd0529 100644 --- a/packer/template.go +++ b/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 diff --git a/packer/template_test.go b/packer/template_test.go index 2e3fbc672..883933367 100644 --- a/packer/template_test.go +++ b/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) {