From e251adb37e9ddbfaa92d7b27901d732cbbb6b399 Mon Sep 17 00:00:00 2001 From: Don Kuntz Date: Fri, 22 Feb 2019 10:18:37 -0600 Subject: [PATCH 1/5] Make the amazon-private-ip fixer errors more visible At present, when using the packer fix command on a template that has "ssh_private_ip" set to anything but a boolean value, the fixer will fail, and appear to fail silently, simply returning a non-zero status code without any message. To determine what happened, users have to know to set PACKER_LOG=1 to make the log message visible. So far as I can tell, this is the only instance of log.Fatalf being called, and based on the surrounding code the better solution would be to return an error, which will then be visible to users of packer fix without having to look in the logs. --- fix/fixer_amazon_private_ip.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fix/fixer_amazon_private_ip.go b/fix/fixer_amazon_private_ip.go index 7b0189ca5..bf1575644 100644 --- a/fix/fixer_amazon_private_ip.go +++ b/fix/fixer_amazon_private_ip.go @@ -1,7 +1,7 @@ package fix import ( - "log" + "fmt" "strconv" "strings" @@ -53,7 +53,7 @@ func (FixerAmazonPrivateIP) Fix(input map[string]interface{}) (map[string]interf var err error privateIP, err = strconv.ParseBool(privateIPi.(string)) if err != nil { - log.Fatalf("Wrong type for ssh_private_ip") + return nil, fmt.Errorf("ssh_private_ip is not a boolean") continue } } From e4faa98b6fac503e3a64c297f8a9faf069df21e3 Mon Sep 17 00:00:00 2001 From: Don Kuntz Date: Fri, 22 Feb 2019 10:38:27 -0600 Subject: [PATCH 2/5] cleanup: remove continue statement after a return statement, because it's unreachable --- fix/fixer_amazon_private_ip.go | 1 - 1 file changed, 1 deletion(-) diff --git a/fix/fixer_amazon_private_ip.go b/fix/fixer_amazon_private_ip.go index bf1575644..bff7eff74 100644 --- a/fix/fixer_amazon_private_ip.go +++ b/fix/fixer_amazon_private_ip.go @@ -54,7 +54,6 @@ func (FixerAmazonPrivateIP) Fix(input map[string]interface{}) (map[string]interf privateIP, err = strconv.ParseBool(privateIPi.(string)) if err != nil { return nil, fmt.Errorf("ssh_private_ip is not a boolean") - continue } } From 04de86d211f5b68fc6d1d238e2efd92ca7f389d9 Mon Sep 17 00:00:00 2001 From: Don Kuntz Date: Mon, 25 Feb 2019 14:19:13 -0600 Subject: [PATCH 3/5] Add test for non-boolean values in ssh_private_ip for FixerAmazonPrivateIP --- fix/fixer_amazon_private_ip_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/fix/fixer_amazon_private_ip_test.go b/fix/fixer_amazon_private_ip_test.go index 71961f2f8..96f925b97 100644 --- a/fix/fixer_amazon_private_ip_test.go +++ b/fix/fixer_amazon_private_ip_test.go @@ -75,3 +75,19 @@ func TestFixerAmazonPrivateIP(t *testing.T) { } } } + +func TestFixerAmazonPrivateIPNonBoolean(t *testing.T) { + var f FixerAmazonPrivateIP + + input := map[string]interface{}{ + "builders": []map[string]interface{}{map[string]interface{}{ + "type": "amazon-ebs", + "ssh_private_ip": "not-a-boolean-value", + }}, + } + + _, err := f.Fix(input) + if err == nil { + t.Fatal("should have errored") + } +} From b0589c964312e13f7a834a34f217c03a985aa1dd Mon Sep 17 00:00:00 2001 From: Don Kuntz Date: Mon, 25 Feb 2019 14:20:14 -0600 Subject: [PATCH 4/5] Append underlying error to output when FixerAmazonPrivateIP cannot parse the value of ssh_private_ip --- fix/fixer_amazon_private_ip.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fix/fixer_amazon_private_ip.go b/fix/fixer_amazon_private_ip.go index bff7eff74..5fd11d60a 100644 --- a/fix/fixer_amazon_private_ip.go +++ b/fix/fixer_amazon_private_ip.go @@ -53,7 +53,7 @@ func (FixerAmazonPrivateIP) Fix(input map[string]interface{}) (map[string]interf var err error privateIP, err = strconv.ParseBool(privateIPi.(string)) if err != nil { - return nil, fmt.Errorf("ssh_private_ip is not a boolean") + return nil, fmt.Errorf("ssh_private_ip is not a boolean, %s", err) } } From 279fb7a63282028d08cfbf11dd42cb67cd5ad9e6 Mon Sep 17 00:00:00 2001 From: Don Kuntz Date: Mon, 25 Feb 2019 14:41:26 -0600 Subject: [PATCH 5/5] gofix --- fix/fixer_amazon_private_ip_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fix/fixer_amazon_private_ip_test.go b/fix/fixer_amazon_private_ip_test.go index 96f925b97..55b2104a6 100644 --- a/fix/fixer_amazon_private_ip_test.go +++ b/fix/fixer_amazon_private_ip_test.go @@ -80,7 +80,7 @@ func TestFixerAmazonPrivateIPNonBoolean(t *testing.T) { var f FixerAmazonPrivateIP input := map[string]interface{}{ - "builders": []map[string]interface{}{map[string]interface{}{ + "builders": []map[string]interface{}{{ "type": "amazon-ebs", "ssh_private_ip": "not-a-boolean-value", }},