From 42910a5f8c3503de69a62403e60850893846970e Mon Sep 17 00:00:00 2001 From: Matthew Hooker Date: Wed, 5 Sep 2018 12:08:40 -0700 Subject: [PATCH 1/2] fix docker email fixer Fixing post-processors requires some smart parsing of the template. Let's turn that logic into a helper and use it everywhere. --- fix/fixer_docker_email.go | 10 ++++++---- fix/fixer_pp_manifest_filename.go | 19 +++---------------- fix/fixer_pp_vagrant_override.go | 20 +++----------------- fix/helpers.go | 25 +++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 37 deletions(-) create mode 100644 fix/helpers.go diff --git a/fix/fixer_docker_email.go b/fix/fixer_docker_email.go index d1402d3bd..363dfd345 100644 --- a/fix/fixer_docker_email.go +++ b/fix/fixer_docker_email.go @@ -7,8 +7,8 @@ type FixerDockerEmail struct{} func (FixerDockerEmail) Fix(input map[string]interface{}) (map[string]interface{}, error) { // Our template type we'll use for this fixer only type template struct { - Builders []map[string]interface{} - PostProcessors []map[string]interface{} `mapstructure:"post-processors"` + Builders []map[string]interface{} + PP `mapstructure:",squash"` } // Decode the input into our structure, if we can @@ -27,7 +27,9 @@ func (FixerDockerEmail) Fix(input map[string]interface{}) (map[string]interface{ } // Go through each post-processor and delete `docker_login` if present - for _, pp := range tpl.PostProcessors { + pps := tpl.postProcessors() + + for _, pp := range pps { _, ok := pp["login_email"] if !ok { continue @@ -36,7 +38,7 @@ func (FixerDockerEmail) Fix(input map[string]interface{}) (map[string]interface{ } input["builders"] = tpl.Builders - input["post-processors"] = tpl.PostProcessors + input["post-processors"] = pps return input, nil } diff --git a/fix/fixer_pp_manifest_filename.go b/fix/fixer_pp_manifest_filename.go index 50b3e5e16..c08083e62 100644 --- a/fix/fixer_pp_manifest_filename.go +++ b/fix/fixer_pp_manifest_filename.go @@ -11,7 +11,7 @@ func (FixerManifestFilename) Fix(input map[string]interface{}) (map[string]inter // Our template type we'll use for this fixer only type template struct { - PostProcessors []interface{} `mapstructure:"post-processors"` + PP `mapstructure:",squash"` } // Decode the input into our structure, if we can @@ -21,20 +21,7 @@ func (FixerManifestFilename) Fix(input map[string]interface{}) (map[string]inter } // Go through each post-processor and get out all the complex configs - pps := make([]map[string]interface{}, 0, len(tpl.PostProcessors)) - for _, rawPP := range tpl.PostProcessors { - switch pp := rawPP.(type) { - case string: - case map[string]interface{}: - pps = append(pps, pp) - case []interface{}: - for _, innerRawPP := range pp { - if innerPP, ok := innerRawPP.(map[string]interface{}); ok { - pps = append(pps, innerPP) - } - } - } - } + pps := tpl.postProcessors() for _, pp := range pps { ppTypeRaw, ok := pp["type"] @@ -60,7 +47,7 @@ func (FixerManifestFilename) Fix(input map[string]interface{}) (map[string]inter } - input["post-processors"] = tpl.PostProcessors + input["post-processors"] = pps return input, nil } diff --git a/fix/fixer_pp_vagrant_override.go b/fix/fixer_pp_vagrant_override.go index a0c530728..f04043ab0 100644 --- a/fix/fixer_pp_vagrant_override.go +++ b/fix/fixer_pp_vagrant_override.go @@ -12,7 +12,7 @@ type FixerVagrantPPOverride struct{} func (FixerVagrantPPOverride) Fix(input map[string]interface{}) (map[string]interface{}, error) { // Our template type we'll use for this fixer only type template struct { - PostProcessors []interface{} `mapstructure:"post-processors"` + PP `mapstructure:",squash"` } // Decode the input into our structure, if we can @@ -21,21 +21,7 @@ func (FixerVagrantPPOverride) Fix(input map[string]interface{}) (map[string]inte return nil, err } - // Go through each post-processor and get out all the complex configs - pps := make([]map[string]interface{}, 0, len(tpl.PostProcessors)) - for _, rawPP := range tpl.PostProcessors { - switch pp := rawPP.(type) { - case string: - case map[string]interface{}: - pps = append(pps, pp) - case []interface{}: - for _, innerRawPP := range pp { - if innerPP, ok := innerRawPP.(map[string]interface{}); ok { - pps = append(pps, innerPP) - } - } - } - } + pps := tpl.postProcessors() // Go through each post-processor and make the fix if necessary possible := []string{"aws", "digitalocean", "virtualbox", "vmware"} @@ -66,7 +52,7 @@ func (FixerVagrantPPOverride) Fix(input map[string]interface{}) (map[string]inte } } - input["post-processors"] = tpl.PostProcessors + input["post-processors"] = pps return input, nil } diff --git a/fix/helpers.go b/fix/helpers.go new file mode 100644 index 000000000..03b07573d --- /dev/null +++ b/fix/helpers.go @@ -0,0 +1,25 @@ +package fix + +// PP is a convenient way to interact with the post-processors within a fixer +type PP struct { + PostProcessors []interface{} `mapstructure:"post-processors"` +} + +// postProcessors converts the variable structure of the template to a list +func (pp *PP) postProcessors() []map[string]interface{} { + pps := make([]map[string]interface{}, 0, len(pp.PostProcessors)) + for _, rawPP := range pp.PostProcessors { + switch pp := rawPP.(type) { + case string: + case map[string]interface{}: + pps = append(pps, pp) + case []interface{}: + for _, innerRawPP := range pp { + if innerPP, ok := innerRawPP.(map[string]interface{}); ok { + pps = append(pps, innerPP) + } + } + } + } + return pps +} From 8a7d43dd44932515364a564f5555991441fa948a Mon Sep 17 00:00:00 2001 From: Matthew Hooker Date: Thu, 6 Sep 2018 11:55:11 -0700 Subject: [PATCH 2/2] bug fix and add test --- command/fix_test.go | 21 +++++++++++++++++++++ fix/fixer_docker_email.go | 8 ++++++-- fix/fixer_pp_manifest_filename.go | 7 +++++-- fix/fixer_pp_manifest_filename_test.go | 11 ++++------- fix/fixer_pp_vagrant_override.go | 12 +++++++----- fix/fixer_pp_vagrant_override_test.go | 11 ++++------- fix/helpers.go | 2 +- 7 files changed, 48 insertions(+), 24 deletions(-) diff --git a/command/fix_test.go b/command/fix_test.go index 1bc9db04d..966367c71 100644 --- a/command/fix_test.go +++ b/command/fix_test.go @@ -2,18 +2,39 @@ package command import ( "path/filepath" + "strings" "testing" + + "github.com/hashicorp/packer/packer" + "github.com/stretchr/testify/assert" ) func TestFix(t *testing.T) { + s := &strings.Builder{} + ui := &packer.BasicUi{ + Writer: s, + } c := &FixCommand{ Meta: testMeta(t), } + c.Ui = ui + args := []string{filepath.Join(testFixture("fix"), "template.json")} if code := c.Run(args); code != 0 { fatalCommand(t, c.Meta) } + expected := `{ + "builders": [ + { + "type": "dummy" + } + ], + "push": { + "name": "foo/bar" + } +}` + assert.Equal(t, expected, strings.TrimSpace(s.String())) } func TestFix_invalidTemplate(t *testing.T) { diff --git a/fix/fixer_docker_email.go b/fix/fixer_docker_email.go index 363dfd345..6db0cac60 100644 --- a/fix/fixer_docker_email.go +++ b/fix/fixer_docker_email.go @@ -5,6 +5,10 @@ import "github.com/mitchellh/mapstructure" type FixerDockerEmail struct{} func (FixerDockerEmail) Fix(input map[string]interface{}) (map[string]interface{}, error) { + if input["post-processors"] == nil { + return input, nil + } + // Our template type we'll use for this fixer only type template struct { Builders []map[string]interface{} @@ -27,7 +31,7 @@ func (FixerDockerEmail) Fix(input map[string]interface{}) (map[string]interface{ } // Go through each post-processor and delete `docker_login` if present - pps := tpl.postProcessors() + pps := tpl.ppList() for _, pp := range pps { _, ok := pp["login_email"] @@ -38,7 +42,7 @@ func (FixerDockerEmail) Fix(input map[string]interface{}) (map[string]interface{ } input["builders"] = tpl.Builders - input["post-processors"] = pps + input["post-processors"] = tpl.PostProcessors return input, nil } diff --git a/fix/fixer_pp_manifest_filename.go b/fix/fixer_pp_manifest_filename.go index c08083e62..fd5e78ba7 100644 --- a/fix/fixer_pp_manifest_filename.go +++ b/fix/fixer_pp_manifest_filename.go @@ -8,6 +8,9 @@ import ( type FixerManifestFilename struct{} func (FixerManifestFilename) Fix(input map[string]interface{}) (map[string]interface{}, error) { + if input["post-processors"] == nil { + return input, nil + } // Our template type we'll use for this fixer only type template struct { @@ -21,7 +24,7 @@ func (FixerManifestFilename) Fix(input map[string]interface{}) (map[string]inter } // Go through each post-processor and get out all the complex configs - pps := tpl.postProcessors() + pps := tpl.ppList() for _, pp := range pps { ppTypeRaw, ok := pp["type"] @@ -47,7 +50,7 @@ func (FixerManifestFilename) Fix(input map[string]interface{}) (map[string]inter } - input["post-processors"] = pps + input["post-processors"] = tpl.PostProcessors return input, nil } diff --git a/fix/fixer_pp_manifest_filename_test.go b/fix/fixer_pp_manifest_filename_test.go index 907d41260..ac7d14d6a 100644 --- a/fix/fixer_pp_manifest_filename_test.go +++ b/fix/fixer_pp_manifest_filename_test.go @@ -1,8 +1,9 @@ package fix import ( - "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func TestFixerManifestPPFilename_Impl(t *testing.T) { @@ -43,11 +44,7 @@ func TestFixerManifestPPFilename_Fix(t *testing.T) { } output, err := f.Fix(input) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.NoError(t, err) - if !reflect.DeepEqual(output, expected) { - t.Fatalf("unexpected: %#v\nexpected: %#v\n", output, expected) - } + assert.Equal(t, expected, output) } diff --git a/fix/fixer_pp_vagrant_override.go b/fix/fixer_pp_vagrant_override.go index f04043ab0..f3bd4693e 100644 --- a/fix/fixer_pp_vagrant_override.go +++ b/fix/fixer_pp_vagrant_override.go @@ -1,8 +1,6 @@ package fix -import ( - "github.com/mitchellh/mapstructure" -) +import "github.com/mitchellh/mapstructure" // FixerVagrantPPOverride is a Fixer that replaces the provider-specific // overrides for the Vagrant post-processor with the new style introduced @@ -10,6 +8,10 @@ import ( type FixerVagrantPPOverride struct{} func (FixerVagrantPPOverride) Fix(input map[string]interface{}) (map[string]interface{}, error) { + if input["post-processors"] == nil { + return input, nil + } + // Our template type we'll use for this fixer only type template struct { PP `mapstructure:",squash"` @@ -21,7 +23,7 @@ func (FixerVagrantPPOverride) Fix(input map[string]interface{}) (map[string]inte return nil, err } - pps := tpl.postProcessors() + pps := tpl.ppList() // Go through each post-processor and make the fix if necessary possible := []string{"aws", "digitalocean", "virtualbox", "vmware"} @@ -52,7 +54,7 @@ func (FixerVagrantPPOverride) Fix(input map[string]interface{}) (map[string]inte } } - input["post-processors"] = pps + input["post-processors"] = tpl.PostProcessors return input, nil } diff --git a/fix/fixer_pp_vagrant_override_test.go b/fix/fixer_pp_vagrant_override_test.go index ae344911e..f58099565 100644 --- a/fix/fixer_pp_vagrant_override_test.go +++ b/fix/fixer_pp_vagrant_override_test.go @@ -1,8 +1,9 @@ package fix import ( - "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func TestFixerVagrantPPOverride_Impl(t *testing.T) { @@ -69,11 +70,7 @@ func TestFixerVagrantPPOverride_Fix(t *testing.T) { } output, err := f.Fix(input) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.NoError(t, err) - if !reflect.DeepEqual(output, expected) { - t.Fatalf("unexpected: %#v\nexpected: %#v\n", output, expected) - } + assert.Equal(t, expected, output) } diff --git a/fix/helpers.go b/fix/helpers.go index 03b07573d..6866c12ea 100644 --- a/fix/helpers.go +++ b/fix/helpers.go @@ -6,7 +6,7 @@ type PP struct { } // postProcessors converts the variable structure of the template to a list -func (pp *PP) postProcessors() []map[string]interface{} { +func (pp *PP) ppList() []map[string]interface{} { pps := make([]map[string]interface{}, 0, len(pp.PostProcessors)) for _, rawPP := range pp.PostProcessors { switch pp := rawPP.(type) {