From d2047d714e564b24ed2c6096dcec4a0aadfead0f Mon Sep 17 00:00:00 2001 From: Peter McAtominey Date: Fri, 20 Jan 2017 14:04:43 +0000 Subject: [PATCH] provisioner/remote-exec: fail on first inline script with bad exit code (#11155) The provisioner collected all inline commands into a single script which meant only the exit code of the last command was actually checked for an error. --- .../remote-exec/resource_provisioner.go | 32 ++++++++++--------- .../remote-exec/resource_provisioner_test.go | 29 ++++++++++------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go index a199b95c4..46d4fd1ac 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner.go +++ b/builtin/provisioners/remote-exec/resource_provisioner.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "log" "os" - "strings" "time" "github.com/hashicorp/terraform/communicator" @@ -63,32 +62,30 @@ func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string return } -// generateScript takes the configuration and creates a script to be executed -// from the inline configs -func (p *ResourceProvisioner) generateScript(c *terraform.ResourceConfig) (string, error) { - var lines []string +// generateScripts takes the configuration and creates a script from each inline config +func (p *ResourceProvisioner) generateScripts(c *terraform.ResourceConfig) ([]string, error) { + var scripts []string command, ok := c.Config["inline"] if ok { switch cmd := command.(type) { case string: - lines = append(lines, cmd) + scripts = append(scripts, cmd) case []string: - lines = append(lines, cmd...) + scripts = append(scripts, cmd...) case []interface{}: for _, l := range cmd { lStr, ok := l.(string) if ok { - lines = append(lines, lStr) + scripts = append(scripts, lStr) } else { - return "", fmt.Errorf("Unsupported 'inline' type! Must be string, or list of strings.") + return nil, fmt.Errorf("Unsupported 'inline' type! Must be string, or list of strings.") } } default: - return "", fmt.Errorf("Unsupported 'inline' type! Must be string, or list of strings.") + return nil, fmt.Errorf("Unsupported 'inline' type! Must be string, or list of strings.") } } - lines = append(lines, "") - return strings.Join(lines, "\n"), nil + return scripts, nil } // collectScripts is used to collect all the scripts we need @@ -97,12 +94,17 @@ func (p *ResourceProvisioner) collectScripts(c *terraform.ResourceConfig) ([]io. // Check if inline _, ok := c.Config["inline"] if ok { - script, err := p.generateScript(c) + scripts, err := p.generateScripts(c) if err != nil { return nil, err } - rc := ioutil.NopCloser(bytes.NewReader([]byte(script))) - return []io.ReadCloser{rc}, nil + + r := []io.ReadCloser{} + for _, script := range scripts { + r = append(r, ioutil.NopCloser(bytes.NewReader([]byte(script)))) + } + + return r, nil } // Collect scripts diff --git a/builtin/provisioners/remote-exec/resource_provisioner_test.go b/builtin/provisioners/remote-exec/resource_provisioner_test.go index a10520fb8..a581301e3 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner_test.go +++ b/builtin/provisioners/remote-exec/resource_provisioner_test.go @@ -3,8 +3,11 @@ package remoteexec import ( "bytes" "io" + "strings" "testing" + "reflect" + "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/terraform" ) @@ -46,7 +49,9 @@ wget http://foobar exit 0 ` -func TestResourceProvider_generateScript(t *testing.T) { +var expectedInlineScriptsOut = strings.Split(expectedScriptOut, "\n") + +func TestResourceProvider_generateScripts(t *testing.T) { p := new(ResourceProvisioner) conf := testConfig(t, map[string]interface{}{ "inline": []interface{}{ @@ -55,12 +60,12 @@ func TestResourceProvider_generateScript(t *testing.T) { "exit 0", }, }) - out, err := p.generateScript(conf) + out, err := p.generateScripts(conf) if err != nil { t.Fatalf("err: %v", err) } - if out != expectedScriptOut { + if reflect.DeepEqual(out, expectedInlineScriptsOut) { t.Fatalf("bad: %v", out) } } @@ -80,18 +85,20 @@ func TestResourceProvider_CollectScripts_inline(t *testing.T) { t.Fatalf("err: %v", err) } - if len(scripts) != 1 { + if len(scripts) != 3 { t.Fatalf("bad: %v", scripts) } - var out bytes.Buffer - _, err = io.Copy(&out, scripts[0]) - if err != nil { - t.Fatalf("err: %v", err) - } + for i, script := range scripts { + var out bytes.Buffer + _, err = io.Copy(&out, script) + if err != nil { + t.Fatalf("err: %v", err) + } - if out.String() != expectedScriptOut { - t.Fatalf("bad: %v", out.String()) + if out.String() != expectedInlineScriptsOut[i] { + t.Fatalf("bad: %v", out.String()) + } } }