Updated the PR according to the review comments

* We now return an error when you set the script_path to
C:\Windows\Temp explaining this is currently not supported
* The fix in PR #1588 is converted to the updated setup in this PR
including the unit tests

Last thing to do is add a few tests for the WinRM communicator…

builtin/provisioners/remote-exec/resource_provisioner.go

@@ -181,12 +181,14 @@ func (p *ResourceProvisioner) runScripts(
 		go p.copyOutput(o, errR, errDoneCh)
 
 		err = retryFunc(comm.Timeout(), func() error {
-			if err := comm.UploadScript(comm.ScriptPath(), script); err != nil {
+			remotePath := comm.ScriptPath()
+
+			if err := comm.UploadScript(remotePath, script); err != nil {
 				return fmt.Errorf("Failed to upload script: %v", err)
 			}
 
 			cmd = &remote.Cmd{
-				Command: comm.ScriptPath(),
+				Command: remotePath,
 				Stdout:  outW,
 				Stderr:  errW,
 			}

communicator/ssh/communicator.go

@@ -8,9 +8,12 @@ import (
 	"io"
 	"io/ioutil"
 	"log"
+	"math/rand"
 	"net"
 	"os"
 	"path/filepath"
+	"strconv"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/terraform/communicator/remote"
@@ -142,7 +145,9 @@ func (c *Communicator) Timeout() time.Duration {
 
 // ScriptPath implementation of communicator.Communicator interface
 func (c *Communicator) ScriptPath() string {
-	return c.connInfo.ScriptPath
+	return strings.Replace(
+		c.connInfo.ScriptPath, "%RAND%",
+		strconv.FormatInt(int64(rand.Int31()), 10), -1)
 }
 
 // Start implementation of communicator.Communicator interface

communicator/ssh/communicator_test.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"fmt"
 	"net"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -167,3 +168,32 @@ func TestStart(t *testing.T) {
 		t.Fatalf("error executing remote command: %s", err)
 	}
 }
+
+func TestScriptPath(t *testing.T) {
+	cases := []struct {
+		Input   string
+		Pattern string
+	}{
+		{
+			"/tmp/script.sh",
+			`^/tmp/script\.sh$`,
+		},
+		{
+			"/tmp/script_%RAND%.sh",
+			`^/tmp/script_(\d+)\.sh$`,
+		},
+	}
+
+	for _, tc := range cases {
+		comm := &Communicator{connInfo: &connectionInfo{ScriptPath: tc.Input}}
+		output := comm.ScriptPath()
+
+		match, err := regexp.Match(tc.Pattern, []byte(output))
+		if err != nil {
+			t.Fatalf("bad: %s\n\nerr: %s", tc.Input, err)
+		}
+		if !match {
+			t.Fatalf("bad: %s\n\n%s", tc.Input, output)
+		}
+	}
+}

communicator/ssh/provisioner.go

@@ -25,7 +25,7 @@ const (
 
 	// DefaultScriptPath is used as the path to copy the file to
 	// for remote execution if not provided otherwise.
-	DefaultScriptPath = "/tmp/script_%RAND%.sh"
+	DefaultScriptPath = "/tmp/terraform_%RAND%.sh"
 
 	// DefaultTimeout is used if there is no timeout given
 	DefaultTimeout = 5 * time.Minute
@@ -61,6 +61,7 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
 	if err := dec.Decode(s.Ephemeral.ConnInfo); err != nil {
 		return nil, err
 	}
+
 	if connInfo.User == "" {
 		connInfo.User = DefaultUser
 	}
@@ -75,6 +76,7 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
 	} else {
 		connInfo.TimeoutVal = DefaultTimeout
 	}
+
 	return connInfo, nil
 }
 

communicator/winrm/communicator.go

@@ -4,6 +4,9 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"math/rand"
+	"strconv"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/terraform/communicator/remote"
@@ -113,7 +116,9 @@ func (c *Communicator) Timeout() time.Duration {
 
 // ScriptPath implementation of communicator.Communicator interface
 func (c *Communicator) ScriptPath() string {
-	return c.connInfo.ScriptPath
+	return strings.Replace(
+		c.connInfo.ScriptPath, "%RAND%",
+		strconv.FormatInt(int64(rand.Int31()), 10), -1)
 }
 
 // Start implementation of communicator.Communicator interface

communicator/winrm/communicator_test.go

@@ -1 +1,35 @@
 package winrm
+
+import (
+	"regexp"
+	"testing"
+)
+
+func TestScriptPath(t *testing.T) {
+	cases := []struct {
+		Input   string
+		Pattern string
+	}{
+		{
+			"/tmp/script.sh",
+			`^/tmp/script\.sh$`,
+		},
+		{
+			"/tmp/script_%RAND%.sh",
+			`^/tmp/script_(\d+)\.sh$`,
+		},
+	}
+
+	for _, tc := range cases {
+		comm := &Communicator{connInfo: &connectionInfo{ScriptPath: tc.Input}}
+		output := comm.ScriptPath()
+
+		match, err := regexp.Match(tc.Pattern, []byte(output))
+		if err != nil {
+			t.Fatalf("bad: %s\n\nerr: %s", tc.Input, err)
+		}
+		if !match {
+			t.Fatalf("bad: %s\n\n%s", tc.Input, output)
+		}
+	}
+}

communicator/winrm/provisioner.go

@@ -3,6 +3,7 @@ package winrm
 import (
 	"fmt"
 	"log"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -19,7 +20,7 @@ const (
 
 	// DefaultScriptPath is used as the path to copy the file to
 	// for remote execution if not provided otherwise.
-	DefaultScriptPath = "C:/Temp/script.cmd"
+	DefaultScriptPath = "C:/Temp/terraform_%RAND%.cmd"
 
 	// DefaultTimeout is used if there is no timeout given
 	DefaultTimeout = 5 * time.Minute
@@ -56,19 +57,25 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
 	if err := dec.Decode(s.Ephemeral.ConnInfo); err != nil {
 		return nil, err
 	}
+
+	// Check on script paths which point to the default Windows TEMP folder because files
+	// which are put in there very early in the boot process could get cleaned/deleted
+	// before you had the change to execute them.
+	//
+	// TODO (SvH) Needs some more debugging to fully understand the exact sequence of events
+	// causing this...
+	if strings.HasPrefix(filepath.ToSlash(connInfo.ScriptPath), "C:/Windows/Temp") {
+		return nil, fmt.Errorf(
+			`Using the C:\Windows\Temp folder is not supported. Please use a different 'script_path'.`)
+	}
+
 	if connInfo.User == "" {
 		connInfo.User = DefaultUser
 	}
 	if connInfo.Port == 0 {
 		connInfo.Port = DefaultPort
 	}
-	// We also check on script paths which point to the default Windows TEMP folder because
-	// files which are put in there very early in the boot process could get cleaned/deleted
-	// before you had the change to execute them.
-	//
-	// TODO (SvH) Needs some more debugging to fully understand the exact sequence of events
-	// causing this...
-	if connInfo.ScriptPath == "" || strings.HasPrefix(connInfo.ScriptPath, "C:/Windows/Temp") {
+	if connInfo.ScriptPath == "" {
 		connInfo.ScriptPath = DefaultScriptPath
 	}
 	if connInfo.Timeout != "" {
@@ -76,6 +83,7 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
 	} else {
 		connInfo.TimeoutVal = DefaultTimeout
 	}
+
 	return connInfo, nil
 }