diff --git a/packer/build.go b/packer/build.go index 982dce40b..987e4d339 100644 --- a/packer/build.go +++ b/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 diff --git a/packer/build_test.go b/packer/build_test.go index fce6cf382..5a073d39c 100644 --- a/packer/build_test.go +++ b/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) diff --git a/packer/template.go b/packer/template.go index 346e5a5a0..ac04bad4d 100644 --- a/packer/template.go +++ b/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{ diff --git a/packer/template_test.go b/packer/template_test.go index 800c982d0..e71b35d96 100644 --- a/packer/template_test.go +++ b/packer/template_test.go @@ -50,7 +50,7 @@ func TestParseTemplateFile_basic(t *testing.T) { tf.Write([]byte(data)) tf.Close() - result, err := ParseTemplateFile(tf.Name()) + result, err := ParseTemplateFile(tf.Name(), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -83,7 +83,7 @@ func TestParseTemplateFile_stdin(t *testing.T) { defer func() { os.Stdin = oldStdin }() os.Stdin = tf - result, err := ParseTemplateFile("-") + result, err := ParseTemplateFile("-", nil) if err != nil { t.Fatalf("err: %s", err) } @@ -100,7 +100,7 @@ func TestParseTemplate_Basic(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -120,7 +120,7 @@ func TestParseTemplate_Description(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -141,7 +141,7 @@ func TestParseTemplate_Invalid(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("shold have error") } @@ -160,7 +160,7 @@ func TestParseTemplate_InvalidKeys(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -176,7 +176,7 @@ func TestParseTemplate_BuilderWithoutType(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -191,7 +191,7 @@ func TestParseTemplate_BuilderWithNonStringType(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -208,7 +208,7 @@ func TestParseTemplate_BuilderWithoutName(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -240,7 +240,7 @@ func TestParseTemplate_BuilderWithName(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -289,7 +289,7 @@ func TestParseTemplate_BuilderWithConflictingName(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -307,7 +307,7 @@ func TestParseTemplate_Hooks(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -342,7 +342,7 @@ func TestParseTemplate_PostProcessors(t *testing.T) { } ` - tpl, err := ParseTemplate([]byte(data)) + tpl, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("error parsing: %s", err) } @@ -392,7 +392,7 @@ func TestParseTemplate_ProvisionerWithoutType(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("err should not be nil") } @@ -409,7 +409,7 @@ func TestParseTemplate_ProvisionerWithNonStringType(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -428,7 +428,7 @@ func TestParseTemplate_Provisioners(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatal("err: %s", err) } @@ -460,7 +460,7 @@ func TestParseTemplate_ProvisionerPauseBefore(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatal("err: %s", err) } @@ -491,7 +491,9 @@ func TestParseTemplate_Variables(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), map[string]string{ + "bar": "bar", + }) if err != nil { t.Fatalf("err: %s", err) } @@ -503,18 +505,25 @@ func TestParseTemplate_Variables(t *testing.T) { 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["foo"].HasValue { + t.Fatal("foo should not have value") + } if result.Variables["bar"].Default != "" { t.Fatal("default should be empty") } - if !result.Variables["bar"].Required { t.Fatal("bar should be required") } + if !result.Variables["bar"].HasValue { + t.Fatal("bar should have value") + } + if result.Variables["bar"].Value != "bar" { + t.Fatal("bad value") + } if result.Variables["baz"].Default != "27" { t.Fatal("default should be empty") @@ -525,6 +534,61 @@ func TestParseTemplate_Variables(t *testing.T) { } } +func TestParseTemplate_variablesSet(t *testing.T) { + data := ` + { + "variables": { + "foo": "bar" + }, + + "builders": [ + { + "name": "test1", + "type": "test-builder" + } + ] + } + ` + + template, err := ParseTemplate([]byte(data), map[string]string{ + "foo": "value", + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + if len(template.Variables) != 1 { + t.Fatalf("bad vars: %#v", template.Variables) + } + if template.Variables["foo"].Value != "value" { + t.Fatalf("bad: %#v", template.Variables["foo"]) + } +} + +func TestParseTemplate_variablesSetUnknown(t *testing.T) { + data := ` + { + "variables": { + "foo": "bar" + }, + + "builders": [ + { + "name": "test1", + "type": "test-builder" + } + ] + } + ` + + _, err := ParseTemplate([]byte(data), map[string]string{ + "what": "value", + }) + if err == nil { + t.Fatal("should error") + } +} + func TestParseTemplate_variablesBadDefault(t *testing.T) { data := ` { @@ -536,7 +600,7 @@ func TestParseTemplate_variablesBadDefault(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -558,7 +622,7 @@ func TestTemplate_BuildNames(t *testing.T) { } ` - result, err := ParseTemplate([]byte(data)) + result, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -582,7 +646,7 @@ func TestTemplate_BuildUnknown(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("bad: %s", err) } @@ -608,7 +672,7 @@ func TestTemplate_BuildUnknownBuilder(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -642,7 +706,7 @@ func TestTemplate_Build_NilBuilderFunc(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -679,7 +743,7 @@ func TestTemplate_Build_NilProvisionerFunc(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -714,7 +778,7 @@ func TestTemplate_Build_NilProvisionerFunc_WithNoProvisioners(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -754,7 +818,7 @@ func TestTemplate_Build(t *testing.T) { "type": "test-builder", } - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -851,7 +915,7 @@ func TestTemplateBuild_exceptOnlyPP(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -881,7 +945,7 @@ func TestTemplateBuild_exceptOnlyProv(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -910,7 +974,7 @@ func TestTemplateBuild_exceptPPInvalid(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -939,7 +1003,7 @@ func TestTemplateBuild_exceptPP(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -990,7 +1054,7 @@ func TestTemplateBuild_exceptProvInvalid(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -1019,7 +1083,7 @@ func TestTemplateBuild_exceptProv(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -1070,7 +1134,7 @@ func TestTemplateBuild_onlyPPInvalid(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -1099,7 +1163,7 @@ func TestTemplateBuild_onlyPP(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -1150,7 +1214,7 @@ func TestTemplateBuild_onlyProvInvalid(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -1179,7 +1243,7 @@ func TestTemplateBuild_onlyProv(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -1229,7 +1293,7 @@ func TestTemplate_Build_ProvisionerOverride(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -1305,7 +1369,7 @@ func TestTemplate_Build_ProvisionerOverrideBad(t *testing.T) { } ` - _, err := ParseTemplate([]byte(data)) + _, err := ParseTemplate([]byte(data), nil) if err == nil { t.Fatal("should have error") } @@ -1330,7 +1394,7 @@ func TestTemplateBuild_ProvisionerPauseBefore(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -1391,7 +1455,7 @@ func TestTemplateBuild_variables(t *testing.T) { } ` - template, err := ParseTemplate([]byte(data)) + template, err := ParseTemplate([]byte(data), nil) if err != nil { t.Fatalf("err: %s", err) } @@ -1406,7 +1470,35 @@ func TestTemplateBuild_variables(t *testing.T) { t.Fatalf("couldn't convert!") } - if len(coreBuild.variables) != 1 { + expected := map[string]string{"foo": "bar"} + if !reflect.DeepEqual(coreBuild.variables, expected) { t.Fatalf("bad vars: %#v", coreBuild.variables) } } + +func TestTemplateBuild_variablesRequiredNotSet(t *testing.T) { + data := ` + { + "variables": { + "foo": null + }, + + "builders": [ + { + "name": "test1", + "type": "test-builder" + } + ] + } + ` + + template, err := ParseTemplate([]byte(data), map[string]string{}) + if err != nil { + t.Fatalf("err: %s", err) + } + + _, err = template.Build("test1", testComponentFinder()) + if err == nil { + t.Fatal("should error") + } +}