diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index 9bd4ac519..fecd85502 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -14,8 +14,6 @@ import ( "github.com/mitchellh/packer/builder/common" "github.com/mitchellh/packer/packer" "log" - "sort" - "strings" "text/template" "time" ) @@ -58,18 +56,7 @@ func (b *Builder) Prepare(raws ...interface{}) error { } // Accumulate any errors - errs := make([]error, 0) - - // Unused keys are errors - if len(md.Unused) > 0 { - sort.Strings(md.Unused) - for _, unused := range md.Unused { - if unused != "type" && !strings.HasPrefix(unused, "packer_") { - errs = append( - errs, fmt.Errorf("Unknown configuration key: %s", unused)) - } - } - } + errs := common.CheckUnusedConfig(md) if b.config.SSHPort == 0 { b.config.SSHPort = 22 @@ -81,39 +68,47 @@ func (b *Builder) Prepare(raws ...interface{}) error { // Accumulate any errors if b.config.SourceAmi == "" { - errs = append(errs, errors.New("A source_ami must be specified")) + errs = packer.MultiErrorAppend( + errs, errors.New("A source_ami must be specified")) } if b.config.InstanceType == "" { - errs = append(errs, errors.New("An instance_type must be specified")) + errs = packer.MultiErrorAppend( + errs, errors.New("An instance_type must be specified")) } if b.config.Region == "" { - errs = append(errs, errors.New("A region must be specified")) + errs = packer.MultiErrorAppend( + errs, errors.New("A region must be specified")) } else if _, ok := aws.Regions[b.config.Region]; !ok { - errs = append(errs, fmt.Errorf("Unknown region: %s", b.config.Region)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Unknown region: %s", b.config.Region)) } if b.config.SSHUsername == "" { - errs = append(errs, errors.New("An ssh_username must be specified")) + errs = packer.MultiErrorAppend( + errs, errors.New("An ssh_username must be specified")) } b.config.sshTimeout, err = time.ParseDuration(b.config.RawSSHTimeout) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err)) } if b.config.AMIName == "" { - errs = append(errs, errors.New("ami_name must be specified")) + errs = packer.MultiErrorAppend( + errs, errors.New("ami_name must be specified")) } else { _, err = template.New("ami").Parse(b.config.AMIName) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing ami_name: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing ami_name: %s", err)) } } - if len(errs) > 0 { - return &packer.MultiError{errs} + if errs != nil && len(errs.Errors) > 0 { + return errs } log.Printf("Config: %+v", b.config) diff --git a/builder/common/config.go b/builder/common/config.go index 88351a35e..c69031779 100644 --- a/builder/common/config.go +++ b/builder/common/config.go @@ -10,7 +10,7 @@ import ( // CheckUnusedConfig is a helper that makes sure that the there are no // unused configuration keys, properly ignoring keys that don't matter. -func CheckUnusedConfig(md *mapstructure.Metadata) error { +func CheckUnusedConfig(md *mapstructure.Metadata) *packer.MultiError { errs := make([]error, 0) if md.Unused != nil && len(md.Unused) > 0 { diff --git a/builder/digitalocean/builder.go b/builder/digitalocean/builder.go index 839e88c65..ae79f7c24 100644 --- a/builder/digitalocean/builder.go +++ b/builder/digitalocean/builder.go @@ -12,9 +12,7 @@ import ( "github.com/mitchellh/packer/packer" "log" "os" - "sort" "strconv" - "strings" "text/template" "time" ) @@ -66,18 +64,7 @@ func (b *Builder) Prepare(raws ...interface{}) error { } // Accumulate any errors - errs := make([]error, 0) - - // Unused keys are errors - if len(md.Unused) > 0 { - sort.Strings(md.Unused) - for _, unused := range md.Unused { - if unused != "type" && !strings.HasPrefix(unused, "packer_") { - errs = append( - errs, fmt.Errorf("Unknown configuration key: %s", unused)) - } - } - } + errs := common.CheckUnusedConfig(md) // Optional configuration with defaults if b.config.APIKey == "" { @@ -140,28 +127,33 @@ func (b *Builder) Prepare(raws ...interface{}) error { // Required configurations that will display errors if not set if b.config.ClientID == "" { - errs = append(errs, errors.New("a client_id must be specified")) + errs = packer.MultiErrorAppend( + errs, errors.New("a client_id must be specified")) } if b.config.APIKey == "" { - errs = append(errs, errors.New("an api_key must be specified")) + errs = packer.MultiErrorAppend( + errs, errors.New("an api_key must be specified")) } sshTimeout, err := time.ParseDuration(b.config.RawSSHTimeout) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing ssh_timeout: %s", err)) } b.config.sshTimeout = sshTimeout eventDelay, err := time.ParseDuration(b.config.RawEventDelay) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing event_delay: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing event_delay: %s", err)) } b.config.eventDelay = eventDelay stateTimeout, err := time.ParseDuration(b.config.RawStateTimeout) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing state_timeout: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing state_timeout: %s", err)) } b.config.stateTimeout = stateTimeout @@ -172,14 +164,15 @@ func (b *Builder) Prepare(raws ...interface{}) error { } t, err := template.New("snapshot").Parse(b.config.RawSnapshotName) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing snapshot_name: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing snapshot_name: %s", err)) } else { t.Execute(snapNameBuf, tData) b.config.SnapshotName = snapNameBuf.String() } - if len(errs) > 0 { - return &packer.MultiError{errs} + if errs != nil && len(errs.Errors) > 0 { + return errs } log.Printf("Config: %+v", b.config) diff --git a/builder/virtualbox/builder.go b/builder/virtualbox/builder.go index 32503f612..5754a8b8f 100644 --- a/builder/virtualbox/builder.go +++ b/builder/virtualbox/builder.go @@ -11,7 +11,6 @@ import ( "os" "os/exec" "path/filepath" - "sort" "strings" "time" ) @@ -70,18 +69,7 @@ func (b *Builder) Prepare(raws ...interface{}) error { } // Accumulate any errors - errs := make([]error, 0) - - // Unused keys are errors - if len(md.Unused) > 0 { - sort.Strings(md.Unused) - for _, unused := range md.Unused { - if unused != "type" && !strings.HasPrefix(unused, "packer_") { - errs = append( - errs, fmt.Errorf("Unknown configuration key: %s", unused)) - } - } - } + errs := common.CheckUnusedConfig(md) if b.config.DiskSize == 0 { b.config.DiskSize = 40000 @@ -140,32 +128,37 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if b.config.HTTPPortMin > b.config.HTTPPortMax { - errs = append(errs, errors.New("http_port_min must be less than http_port_max")) + errs = packer.MultiErrorAppend( + errs, errors.New("http_port_min must be less than http_port_max")) } if b.config.ISOChecksum == "" { - errs = append(errs, errors.New("Due to large file sizes, an iso_checksum is required")) + errs = packer.MultiErrorAppend( + errs, errors.New("Due to large file sizes, an iso_checksum is required")) } else { b.config.ISOChecksum = strings.ToLower(b.config.ISOChecksum) } if b.config.ISOChecksumType == "" { - errs = append(errs, errors.New("The iso_checksum_type must be specified.")) + errs = packer.MultiErrorAppend( + errs, errors.New("The iso_checksum_type must be specified.")) } else { b.config.ISOChecksumType = strings.ToLower(b.config.ISOChecksumType) if h := common.HashForType(b.config.ISOChecksumType); h == nil { - errs = append( + errs = packer.MultiErrorAppend( errs, fmt.Errorf("Unsupported checksum type: %s", b.config.ISOChecksumType)) } } if b.config.ISOUrl == "" { - errs = append(errs, errors.New("An iso_url must be specified.")) + errs = packer.MultiErrorAppend( + errs, errors.New("An iso_url must be specified.")) } else { url, err := url.Parse(b.config.ISOUrl) if err != nil { - errs = append(errs, fmt.Errorf("iso_url is not a valid URL: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("iso_url is not a valid URL: %s", err)) } else { if url.Scheme == "" { url.Scheme = "file" @@ -173,7 +166,8 @@ func (b *Builder) Prepare(raws ...interface{}) error { if url.Scheme == "file" { if _, err := os.Stat(url.Path); err != nil { - errs = append(errs, fmt.Errorf("iso_url points to bad file: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("iso_url points to bad file: %s", err)) } } else { supportedSchemes := []string{"file", "http", "https"} @@ -188,12 +182,13 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if !found { - errs = append(errs, fmt.Errorf("Unsupported URL scheme in iso_url: %s", scheme)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Unsupported URL scheme in iso_url: %s", scheme)) } } } - if len(errs) == 0 { + if errs == nil || len(errs.Errors) == 0 { // Put the URL back together since we may have modified it b.config.ISOUrl = url.String() } @@ -206,7 +201,8 @@ func (b *Builder) Prepare(raws ...interface{}) error { if b.config.GuestAdditionsURL != "" { url, err := url.Parse(b.config.GuestAdditionsURL) if err != nil { - errs = append(errs, fmt.Errorf("guest_additions_url is not a valid URL: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("guest_additions_url is not a valid URL: %s", err)) } else { if url.Scheme == "" { url.Scheme = "file" @@ -214,7 +210,8 @@ func (b *Builder) Prepare(raws ...interface{}) error { if url.Scheme == "file" { if _, err := os.Stat(url.Path); err != nil { - errs = append(errs, fmt.Errorf("guest_additions_url points to bad file: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("guest_additions_url points to bad file: %s", err)) } } else { supportedSchemes := []string{"file", "http", "https"} @@ -229,12 +226,13 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if !found { - errs = append(errs, fmt.Errorf("Unsupported URL scheme in guest_additions_url: %s", scheme)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Unsupported URL scheme in guest_additions_url: %s", scheme)) } } } - if len(errs) == 0 { + if errs == nil || len(errs.Errors) == 0 { // Put the URL back together since we may have modified it b.config.GuestAdditionsURL = url.String() } @@ -242,7 +240,7 @@ func (b *Builder) Prepare(raws ...interface{}) error { if !b.config.PackerForce { if _, err := os.Stat(b.config.OutputDir); err == nil { - errs = append( + errs = packer.MultiErrorAppend( errs, errors.New("Output directory already exists. It must not exist.")) } @@ -250,7 +248,8 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.bootWait, err = time.ParseDuration(b.config.RawBootWait) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing boot_wait: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing boot_wait: %s", err)) } if b.config.RawShutdownTimeout == "" { @@ -263,29 +262,34 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.shutdownTimeout, err = time.ParseDuration(b.config.RawShutdownTimeout) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err)) } if b.config.SSHHostPortMin > b.config.SSHHostPortMax { - errs = append(errs, errors.New("ssh_host_port_min must be less than ssh_host_port_max")) + errs = packer.MultiErrorAppend( + errs, errors.New("ssh_host_port_min must be less than ssh_host_port_max")) } if b.config.SSHUser == "" { - errs = append(errs, errors.New("An ssh_username must be specified.")) + errs = packer.MultiErrorAppend( + errs, errors.New("An ssh_username must be specified.")) } b.config.sshWaitTimeout, err = time.ParseDuration(b.config.RawSSHWaitTimeout) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing ssh_wait_timeout: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing ssh_wait_timeout: %s", err)) } b.driver, err = b.newDriver() if err != nil { - errs = append(errs, fmt.Errorf("Failed creating VirtualBox driver: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed creating VirtualBox driver: %s", err)) } - if len(errs) > 0 { - return &packer.MultiError{errs} + if errs != nil && len(errs.Errors) > 0 { + return errs } return nil diff --git a/builder/vmware/builder.go b/builder/vmware/builder.go index 59a87acd1..e43e2cd62 100644 --- a/builder/vmware/builder.go +++ b/builder/vmware/builder.go @@ -11,7 +11,6 @@ import ( "net/url" "os" "path/filepath" - "sort" "strings" "text/template" "time" @@ -71,18 +70,7 @@ func (b *Builder) Prepare(raws ...interface{}) error { } // Accumulate any errors - errs := make([]error, 0) - - // Unused keys are errors - if len(md.Unused) > 0 { - sort.Strings(md.Unused) - for _, unused := range md.Unused { - if unused != "type" && !strings.HasPrefix(unused, "packer_") { - errs = append( - errs, fmt.Errorf("Unknown configuration key: %s", unused)) - } - } - } + errs := common.CheckUnusedConfig(md) if b.config.DiskName == "" { b.config.DiskName = "disk" @@ -137,32 +125,37 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if b.config.HTTPPortMin > b.config.HTTPPortMax { - errs = append(errs, errors.New("http_port_min must be less than http_port_max")) + errs = packer.MultiErrorAppend( + errs, errors.New("http_port_min must be less than http_port_max")) } if b.config.ISOChecksum == "" { - errs = append(errs, errors.New("Due to large file sizes, an iso_checksum is required")) + errs = packer.MultiErrorAppend( + errs, errors.New("Due to large file sizes, an iso_checksum is required")) } else { b.config.ISOChecksum = strings.ToLower(b.config.ISOChecksum) } if b.config.ISOChecksumType == "" { - errs = append(errs, errors.New("The iso_checksum_type must be specified.")) + errs = packer.MultiErrorAppend( + errs, errors.New("The iso_checksum_type must be specified.")) } else { b.config.ISOChecksumType = strings.ToLower(b.config.ISOChecksumType) if h := common.HashForType(b.config.ISOChecksumType); h == nil { - errs = append( + errs = packer.MultiErrorAppend( errs, fmt.Errorf("Unsupported checksum type: %s", b.config.ISOChecksumType)) } } if b.config.ISOUrl == "" { - errs = append(errs, errors.New("An iso_url must be specified.")) + errs = packer.MultiErrorAppend( + errs, errors.New("An iso_url must be specified.")) } else { url, err := url.Parse(b.config.ISOUrl) if err != nil { - errs = append(errs, fmt.Errorf("iso_url is not a valid URL: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("iso_url is not a valid URL: %s", err)) } else { if url.Scheme == "" { url.Scheme = "file" @@ -170,7 +163,8 @@ func (b *Builder) Prepare(raws ...interface{}) error { if url.Scheme == "file" { if _, err := os.Stat(url.Path); err != nil { - errs = append(errs, fmt.Errorf("iso_url points to bad file: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("iso_url points to bad file: %s", err)) } } else { supportedSchemes := []string{"file", "http", "https"} @@ -185,12 +179,13 @@ func (b *Builder) Prepare(raws ...interface{}) error { } if !found { - errs = append(errs, fmt.Errorf("Unsupported URL scheme in iso_url: %s", scheme)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Unsupported URL scheme in iso_url: %s", scheme)) } } } - if len(errs) == 0 { + if errs == nil || len(errs.Errors) == 0 { // Put the URL back together since we may have modified it b.config.ISOUrl = url.String() } @@ -198,20 +193,22 @@ func (b *Builder) Prepare(raws ...interface{}) error { if !b.config.PackerForce { if _, err := os.Stat(b.config.OutputDir); err == nil { - errs = append( + errs = packer.MultiErrorAppend( errs, errors.New("Output directory already exists. It must not exist.")) } } if b.config.SSHUser == "" { - errs = append(errs, errors.New("An ssh_username must be specified.")) + errs = packer.MultiErrorAppend( + errs, errors.New("An ssh_username must be specified.")) } if b.config.RawBootWait != "" { b.config.bootWait, err = time.ParseDuration(b.config.RawBootWait) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing boot_wait: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing boot_wait: %s", err)) } } @@ -221,7 +218,8 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.shutdownTimeout, err = time.ParseDuration(b.config.RawShutdownTimeout) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing shutdown_timeout: %s", err)) } if b.config.RawSSHWaitTimeout == "" { @@ -230,24 +228,28 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.sshWaitTimeout, err = time.ParseDuration(b.config.RawSSHWaitTimeout) if err != nil { - errs = append(errs, fmt.Errorf("Failed parsing ssh_wait_timeout: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed parsing ssh_wait_timeout: %s", err)) } if _, err := template.New("path").Parse(b.config.ToolsUploadPath); err != nil { - errs = append(errs, fmt.Errorf("tools_upload_path invalid: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("tools_upload_path invalid: %s", err)) } if b.config.VNCPortMin > b.config.VNCPortMax { - errs = append(errs, fmt.Errorf("vnc_port_min must be less than vnc_port_max")) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("vnc_port_min must be less than vnc_port_max")) } b.driver, err = NewDriver() if err != nil { - errs = append(errs, fmt.Errorf("Failed creating VMware driver: %s", err)) + errs = packer.MultiErrorAppend( + errs, fmt.Errorf("Failed creating VMware driver: %s", err)) } - if len(errs) > 0 { - return &packer.MultiError{errs} + if errs != nil && len(errs.Errors) > 0 { + return errs } return nil diff --git a/packer/multi_error.go b/packer/multi_error.go index e743a7504..9c143057f 100644 --- a/packer/multi_error.go +++ b/packer/multi_error.go @@ -27,8 +27,20 @@ func (e *MultiError) Error() string { // onto a MultiError in order to create a larger multi-error. If the // original error is not a MultiError, it will be turned into one. func MultiErrorAppend(err error, errs ...error) *MultiError { + if err == nil { + err = new(MultiError) + } + switch err := err.(type) { case *MultiError: + if err == nil { + err = new(MultiError) + } + + if err.Errors == nil { + err.Errors = make([]error, 0, len(errs)) + } + err.Errors = append(err.Errors, errs...) return err default: diff --git a/packer/multi_error_test.go b/packer/multi_error_test.go index 60ec94d7e..be28f25c7 100644 --- a/packer/multi_error_test.go +++ b/packer/multi_error_test.go @@ -40,6 +40,12 @@ func TestMultiErrorAppend_MultiError(t *testing.T) { if len(result.Errors) != 2 { t.Fatalf("wrong len: %d", len(result.Errors)) } + + original = &MultiError{} + result = MultiErrorAppend(original, errors.New("bar")) + if len(result.Errors) != 1 { + t.Fatalf("wrong len: %d", len(result.Errors)) + } } func TestMultiErrorAppend_NonMultiError(t *testing.T) {