From 3b4ef72e47d2dc8b46a96458c6b8c562c1899b4c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 14 Jul 2013 09:28:56 +0900 Subject: [PATCH] Unused keys are invalid in templates [GH-104] --- CHANGELOG.md | 2 ++ builder/amazonebs/builder.go | 31 ++++++++++++++++++++---- builder/amazonebs/builder_test.go | 12 ++++++++++ builder/digitalocean/builder.go | 34 ++++++++++++++++++++++----- builder/digitalocean/builder_test.go | 12 ++++++++++ builder/virtualbox/builder.go | 30 +++++++++++++++++++---- builder/virtualbox/builder_test.go | 12 ++++++++++ builder/vmware/builder.go | 32 +++++++++++++++++++++---- builder/vmware/builder_test.go | 12 ++++++++++ provisioner/file/provisioner.go | 30 +++++++++++++++++++++-- provisioner/file/provisioner_test.go | 12 ++++++++++ provisioner/shell/provisioner.go | 31 +++++++++++++++++++++--- provisioner/shell/provisioner_test.go | 12 ++++++++++ 13 files changed, 238 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b0a5790..37c5a1418 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ FEATURES: IMPROVEMENTS: +* everything: invalid keys in configuration are now considered validation + errors. [GH-104] * amazon-ebs: Verify the source AMI is EBS-backed before launching. [GH-169] * vmware: error if shutdown command has non-zero exit status. diff --git a/builder/amazonebs/builder.go b/builder/amazonebs/builder.go index 8a0c0c71f..48446bbab 100644 --- a/builder/amazonebs/builder.go +++ b/builder/amazonebs/builder.go @@ -16,6 +16,8 @@ import ( "github.com/mitchellh/packer/packer" "log" "os" + "sort" + "strings" "text/template" "time" ) @@ -50,15 +52,38 @@ type Builder struct { } func (b *Builder) Prepare(raws ...interface{}) error { - var err error + var md mapstructure.Metadata + decoderConfig := &mapstructure.DecoderConfig{ + Metadata: &md, + Result: &b.config, + } + + decoder, err := mapstructure.NewDecoder(decoderConfig) + if err != nil { + return err + } for _, raw := range raws { - err := mapstructure.Decode(raw, &b.config) + err := decoder.Decode(raw) if err != nil { return err } } + // 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)) + } + } + } + if b.config.AccessKey == "" { b.config.AccessKey = os.Getenv("AWS_ACCESS_KEY_ID") } @@ -84,8 +109,6 @@ func (b *Builder) Prepare(raws ...interface{}) error { } // Accumulate any errors - errs := make([]error, 0) - if b.config.AccessKey == "" { errs = append(errs, errors.New("An access_key must be specified")) } diff --git a/builder/amazonebs/builder_test.go b/builder/amazonebs/builder_test.go index d0f0d3f0c..c03f908bb 100644 --- a/builder/amazonebs/builder_test.go +++ b/builder/amazonebs/builder_test.go @@ -148,6 +148,18 @@ func TestBuilderPrepare_InstanceType(t *testing.T) { } } +func TestBuilderPrepare_InvalidKey(t *testing.T) { + var b Builder + config := testConfig() + + // Add a random key + config["i_should_not_be_valid"] = true + err := b.Prepare(config) + if err == nil { + t.Fatal("should have error") + } +} + func TestBuilderPrepare_Region(t *testing.T) { var b Builder config := testConfig() diff --git a/builder/digitalocean/builder.go b/builder/digitalocean/builder.go index 65ec8282e..1f0ebfae7 100644 --- a/builder/digitalocean/builder.go +++ b/builder/digitalocean/builder.go @@ -13,7 +13,9 @@ import ( "github.com/mitchellh/packer/packer" "log" "os" + "sort" "strconv" + "strings" "text/template" "time" ) @@ -56,15 +58,39 @@ type Builder struct { } func (b *Builder) Prepare(raws ...interface{}) error { + var md mapstructure.Metadata + decoderConfig := &mapstructure.DecoderConfig{ + Metadata: &md, + Result: &b.config, + } + + decoder, err := mapstructure.NewDecoder(decoderConfig) + if err != nil { + return err + } + for _, raw := range raws { - err := mapstructure.Decode(raw, &b.config) + err := decoder.Decode(raw) if err != nil { return err } } + // 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)) + } + } + } + // Optional configuration with defaults - // if b.config.APIKey == "" { // Default to environment variable for api_key, if it exists b.config.APIKey = os.Getenv("DIGITALOCEAN_API_KEY") @@ -123,11 +149,7 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.RawStateTimeout = "6m" } - // A list of errors on the configuration - errs := make([]error, 0) - // Required configurations that will display errors if not set - // if b.config.ClientID == "" { errs = append(errs, errors.New("a client_id must be specified")) } diff --git a/builder/digitalocean/builder_test.go b/builder/digitalocean/builder_test.go index 6fe9217dc..59cfc0556 100644 --- a/builder/digitalocean/builder_test.go +++ b/builder/digitalocean/builder_test.go @@ -106,6 +106,18 @@ func TestBuilderPrepare_ClientID(t *testing.T) { } } +func TestBuilderPrepare_InvalidKey(t *testing.T) { + var b Builder + config := testConfig() + + // Add a random key + config["i_should_not_be_valid"] = true + err := b.Prepare(config) + if err == nil { + t.Fatal("should have error") + } +} + func TestBuilderPrepare_RegionID(t *testing.T) { var b Builder config := testConfig() diff --git a/builder/virtualbox/builder.go b/builder/virtualbox/builder.go index 05737f244..21cb4296d 100644 --- a/builder/virtualbox/builder.go +++ b/builder/virtualbox/builder.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" "path/filepath" + "sort" "strings" "time" ) @@ -62,15 +63,38 @@ type config struct { } func (b *Builder) Prepare(raws ...interface{}) error { - var err error + var md mapstructure.Metadata + decoderConfig := &mapstructure.DecoderConfig{ + Metadata: &md, + Result: &b.config, + } + + decoder, err := mapstructure.NewDecoder(decoderConfig) + if err != nil { + return err + } for _, raw := range raws { - err := mapstructure.Decode(raw, &b.config) + err := decoder.Decode(raw) if err != nil { return err } } + // 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)) + } + } + } + if b.config.DiskSize == 0 { b.config.DiskSize = 40000 } @@ -127,8 +151,6 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.VMName = fmt.Sprintf("packer-%s", b.config.PackerBuildName) } - errs := make([]error, 0) - if b.config.HTTPPortMin > b.config.HTTPPortMax { errs = append(errs, errors.New("http_port_min must be less than http_port_max")) } diff --git a/builder/virtualbox/builder_test.go b/builder/virtualbox/builder_test.go index 46f9c8997..7a08ebf04 100644 --- a/builder/virtualbox/builder_test.go +++ b/builder/virtualbox/builder_test.go @@ -271,6 +271,18 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { } } +func TestBuilderPrepare_InvalidKey(t *testing.T) { + var b Builder + config := testConfig() + + // Add a random key + config["i_should_not_be_valid"] = true + err := b.Prepare(config) + if err == nil { + t.Fatal("should have error") + } +} + func TestBuilderPrepare_ISOMD5(t *testing.T) { var b Builder config := testConfig() diff --git a/builder/vmware/builder.go b/builder/vmware/builder.go index 91cc8e0db..6cd7575df 100644 --- a/builder/vmware/builder.go +++ b/builder/vmware/builder.go @@ -12,6 +12,7 @@ import ( "net/url" "os" "path/filepath" + "sort" "strings" "text/template" "time" @@ -63,13 +64,38 @@ type config struct { } func (b *Builder) Prepare(raws ...interface{}) error { + var md mapstructure.Metadata + decoderConfig := &mapstructure.DecoderConfig{ + Metadata: &md, + Result: &b.config, + } + + decoder, err := mapstructure.NewDecoder(decoderConfig) + if err != nil { + return err + } + for _, raw := range raws { - err := mapstructure.Decode(raw, &b.config) + err := decoder.Decode(raw) if err != nil { return err } } + // 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)) + } + } + } + if b.config.DiskName == "" { b.config.DiskName = "disk" } @@ -122,10 +148,6 @@ func (b *Builder) Prepare(raws ...interface{}) error { b.config.ToolsUploadPath = "{{ .Flavor }}.iso" } - // Accumulate any errors - var err error - errs := make([]error, 0) - if b.config.HTTPPortMin > b.config.HTTPPortMax { errs = append(errs, errors.New("http_port_min must be less than http_port_max")) } diff --git a/builder/vmware/builder_test.go b/builder/vmware/builder_test.go index 0cf8a42b1..9b35249d1 100644 --- a/builder/vmware/builder_test.go +++ b/builder/vmware/builder_test.go @@ -163,6 +163,18 @@ func TestBuilderPrepare_HTTPPort(t *testing.T) { } } +func TestBuilderPrepare_InvalidKey(t *testing.T) { + var b Builder + config := testConfig() + + // Add a random key + config["i_should_not_be_valid"] = true + err := b.Prepare(config) + if err == nil { + t.Fatal("should have error") + } +} + func TestBuilderPrepare_ISOMD5(t *testing.T) { var b Builder config := testConfig() diff --git a/provisioner/file/provisioner.go b/provisioner/file/provisioner.go index 2e7c17190..1a9a3589b 100644 --- a/provisioner/file/provisioner.go +++ b/provisioner/file/provisioner.go @@ -6,6 +6,8 @@ import ( "github.com/mitchellh/mapstructure" "github.com/mitchellh/packer/packer" "os" + "sort" + "strings" ) type config struct { @@ -21,13 +23,37 @@ type Provisioner struct { } func (p *Provisioner) Prepare(raws ...interface{}) error { + var md mapstructure.Metadata + decoderConfig := &mapstructure.DecoderConfig{ + Metadata: &md, + Result: &p.config, + } + + decoder, err := mapstructure.NewDecoder(decoderConfig) + if err != nil { + return err + } + for _, raw := range raws { - if err := mapstructure.Decode(raw, &p.config); err != nil { + err := decoder.Decode(raw) + if err != nil { return err } } - errs := []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)) + } + } + } if _, err := os.Stat(p.config.Source); err != nil { errs = append(errs, diff --git a/provisioner/file/provisioner_test.go b/provisioner/file/provisioner_test.go index 1cae2395a..9c57ac452 100644 --- a/provisioner/file/provisioner_test.go +++ b/provisioner/file/provisioner_test.go @@ -23,6 +23,18 @@ func TestProvisioner_Impl(t *testing.T) { } } +func TestProvisionerPrepare_InvalidKey(t *testing.T) { + var p Provisioner + config := testConfig() + + // Add a random key + config["i_should_not_be_valid"] = true + err := p.Prepare(config) + if err == nil { + t.Fatal("should have error") + } +} + func TestProvisionerPrepare_InvalidSource(t *testing.T) { var p Provisioner config := testConfig() diff --git a/provisioner/shell/provisioner.go b/provisioner/shell/provisioner.go index 3d7571c0d..5b0dd65b0 100644 --- a/provisioner/shell/provisioner.go +++ b/provisioner/shell/provisioner.go @@ -14,6 +14,7 @@ import ( "io/ioutil" "log" "os" + "sort" "strings" "text/template" ) @@ -58,12 +59,38 @@ type ExecuteCommandTemplate struct { } func (p *Provisioner) Prepare(raws ...interface{}) error { + var md mapstructure.Metadata + decoderConfig := &mapstructure.DecoderConfig{ + Metadata: &md, + Result: &p.config, + } + + decoder, err := mapstructure.NewDecoder(decoderConfig) + if err != nil { + return err + } + for _, raw := range raws { - if err := mapstructure.Decode(raw, &p.config); err != nil { + err := decoder.Decode(raw) + if err != nil { return err } } + // 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)) + } + } + } + if p.config.ExecuteCommand == "" { p.config.ExecuteCommand = "chmod +x {{.Path}}; {{.Vars}} {{.Path}}" } @@ -88,8 +115,6 @@ func (p *Provisioner) Prepare(raws ...interface{}) error { p.config.Vars = make([]string, 0) } - errs := make([]error, 0) - if p.config.Script != "" && len(p.config.Scripts) > 0 { errs = append(errs, errors.New("Only one of script or scripts can be specified.")) } diff --git a/provisioner/shell/provisioner_test.go b/provisioner/shell/provisioner_test.go index 32d3084a5..ab5308bd2 100644 --- a/provisioner/shell/provisioner_test.go +++ b/provisioner/shell/provisioner_test.go @@ -62,6 +62,18 @@ func TestProvisionerPrepare_InlineShebang(t *testing.T) { } } +func TestProvisionerPrepare_InvalidKey(t *testing.T) { + var p Provisioner + config := testConfig() + + // Add a random key + config["i_should_not_be_valid"] = true + err := p.Prepare(config) + if err == nil { + t.Fatal("should have error") + } +} + func TestProvisionerPrepare_Script(t *testing.T) { config := testConfig() delete(config, "inline")