From c9e9e374bb45371b54a882d9594ca5943a1e18e6 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 9 Apr 2015 21:58:00 +0200 Subject: [PATCH 01/10] Adding some abstractions for the communicators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed as preperation for adding WinRM support. There is still one error in the tests which needs another look, but other than that it seems like were now ready to start working on the WinRM part… --- .../provisioners/file/resource_provisioner.go | 34 +-- .../remote-exec/resource_provisioner.go | 82 ++---- communicator/communicator.go | 52 ++++ communicator/communicator_test.go | 24 ++ communicator/remote/command.go | 67 +++++ communicator/remote/command_test.go | 1 + {helper => communicator}/ssh/communicator.go | 269 +++++++++--------- .../ssh/communicator_test.go | 78 +++-- {helper => communicator}/ssh/password.go | 0 {helper => communicator}/ssh/password_test.go | 0 {helper => communicator}/ssh/provisioner.go | 101 +++---- communicator/ssh/provisioner_test.go | 50 ++++ helper/ssh/provisioner_test.go | 97 ------- plugin/client_test.go | 2 +- 14 files changed, 442 insertions(+), 415 deletions(-) create mode 100644 communicator/communicator.go create mode 100644 communicator/communicator_test.go create mode 100644 communicator/remote/command.go create mode 100644 communicator/remote/command_test.go rename {helper => communicator}/ssh/communicator.go (74%) rename {helper => communicator}/ssh/communicator_test.go (79%) rename {helper => communicator}/ssh/password.go (100%) rename {helper => communicator}/ssh/password_test.go (100%) rename {helper => communicator}/ssh/provisioner.go (60%) create mode 100644 communicator/ssh/provisioner_test.go delete mode 100644 helper/ssh/provisioner_test.go diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go index 3b7b790250c6..1817693a8642 100644 --- a/builtin/provisioners/file/resource_provisioner.go +++ b/builtin/provisioners/file/resource_provisioner.go @@ -6,25 +6,22 @@ import ( "os" "time" + "github.com/hashicorp/terraform/communicator" "github.com/hashicorp/terraform/helper/config" - helper "github.com/hashicorp/terraform/helper/ssh" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/go-homedir" ) +// ResourceProvisioner represents a file provisioner type ResourceProvisioner struct{} +// Apply executes the file provisioner func (p *ResourceProvisioner) Apply( o terraform.UIOutput, s *terraform.InstanceState, c *terraform.ResourceConfig) error { - // Ensure the connection type is SSH - if err := helper.VerifySSH(s); err != nil { - return err - } - - // Get the SSH configuration - conf, err := helper.ParseSSHConfig(s) + // Get a new communicator + comm, err := communicator.New(s) if err != nil { return err } @@ -46,9 +43,10 @@ func (p *ResourceProvisioner) Apply( if !ok { return fmt.Errorf("Unsupported 'destination' type! Must be string.") } - return p.copyFiles(conf, src, dst) + return p.copyFiles(comm, src, dst) } +// Validate checks if the required arguments are configured func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) { v := &config.Validator{ Required: []string{ @@ -60,24 +58,16 @@ func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string } // copyFiles is used to copy the files from a source to a destination -func (p *ResourceProvisioner) copyFiles(conf *helper.SSHConfig, src, dst string) error { - // Get the SSH client config - config, err := helper.PrepareConfig(conf) - if err != nil { - return err - } - defer config.CleanupConfig() - - // Wait and retry until we establish the SSH connection - var comm *helper.SSHCommunicator - err = retryFunc(conf.TimeoutVal, func() error { - host := fmt.Sprintf("%s:%d", conf.Host, conf.Port) - comm, err = helper.New(host, config) +func (p *ResourceProvisioner) copyFiles(comm communicator.Communicator, src, dst string) error { + // Wait and retry until we establish the connection + err := retryFunc(comm.Timeout(), func() error { + err := comm.Connect(nil) return err }) if err != nil { return err } + defer comm.Disconnect() info, err := os.Stat(src) if err != nil { diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go index a4c169b2274f..b3e3d1cfd21e 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner.go +++ b/builtin/provisioners/remote-exec/resource_provisioner.go @@ -10,29 +10,22 @@ import ( "strings" "time" - helper "github.com/hashicorp/terraform/helper/ssh" + "github.com/hashicorp/terraform/communicator" + "github.com/hashicorp/terraform/communicator/remote" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/go-linereader" ) -const ( - // DefaultShebang is added at the top of the script file - DefaultShebang = "#!/bin/sh" -) - +// ResourceProvisioner represents a remote exec provisioner type ResourceProvisioner struct{} +// Apply executes the remote exec provisioner func (p *ResourceProvisioner) Apply( o terraform.UIOutput, s *terraform.InstanceState, c *terraform.ResourceConfig) error { - // Ensure the connection type is SSH - if err := helper.VerifySSH(s); err != nil { - return err - } - - // Get the SSH configuration - conf, err := helper.ParseSSHConfig(s) + // Get a new communicator + comm, err := communicator.New(s) if err != nil { return err } @@ -47,12 +40,13 @@ func (p *ResourceProvisioner) Apply( } // Copy and execute each script - if err := p.runScripts(o, conf, scripts); err != nil { + if err := p.runScripts(o, comm, scripts); err != nil { return err } return nil } +// Validate checks if the required arguments are configured func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) { num := 0 for name := range c.Raw { @@ -76,7 +70,7 @@ func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string // generateScript takes the configuration and creates a script to be executed // from the inline configs func (p *ResourceProvisioner) generateScript(c *terraform.ResourceConfig) (string, error) { - lines := []string{DefaultShebang} + var lines []string command, ok := c.Config["inline"] if ok { switch cmd := command.(type) { @@ -165,46 +159,20 @@ func (p *ResourceProvisioner) collectScripts(c *terraform.ResourceConfig) ([]io. // runScripts is used to copy and execute a set of scripts func (p *ResourceProvisioner) runScripts( o terraform.UIOutput, - conf *helper.SSHConfig, + comm communicator.Communicator, scripts []io.ReadCloser) error { - // Get the SSH client config - config, err := helper.PrepareConfig(conf) - if err != nil { - return err - } - defer config.CleanupConfig() - - o.Output(fmt.Sprintf( - "Connecting to remote host via SSH...\n"+ - " Host: %s\n"+ - " User: %s\n"+ - " Password: %v\n"+ - " Private key: %v"+ - " SSH Agent: %v", - conf.Host, conf.User, - conf.Password != "", - conf.KeyFile != "", - conf.Agent, - )) - - // Wait and retry until we establish the SSH connection - var comm *helper.SSHCommunicator - err = retryFunc(conf.TimeoutVal, func() error { - host := fmt.Sprintf("%s:%d", conf.Host, conf.Port) - comm, err = helper.New(host, config) - if err != nil { - o.Output(fmt.Sprintf("Connection error, will retry: %s", err)) - } - + // Wait and retry until we establish the connection + err := retryFunc(comm.Timeout(), func() error { + err := comm.Connect(o) return err }) if err != nil { return err } + defer comm.Disconnect() - o.Output("Connected! Executing scripts...") for _, script := range scripts { - var cmd *helper.RemoteCmd + var cmd *remote.Cmd outR, outW := io.Pipe() errR, errW := io.Pipe() outDoneCh := make(chan struct{}) @@ -212,30 +180,20 @@ func (p *ResourceProvisioner) runScripts( go p.copyOutput(o, outR, outDoneCh) go p.copyOutput(o, errR, errDoneCh) - err := retryFunc(conf.TimeoutVal, func() error { - remotePath := conf.RemotePath() - - if err := comm.Upload(remotePath, script); err != nil { + err = retryFunc(comm.Timeout(), func() error { + if err := comm.UploadScript(comm.ScriptPath(), script); err != nil { return fmt.Errorf("Failed to upload script: %v", err) } - cmd = &helper.RemoteCmd{ - Command: fmt.Sprintf("chmod 0777 %s", remotePath), - } - if err := comm.Start(cmd); err != nil { - return fmt.Errorf( - "Error chmodding script file to 0777 in remote "+ - "machine: %s", err) - } - cmd.Wait() - cmd = &helper.RemoteCmd{ - Command: remotePath, + cmd = &remote.Cmd{ + Command: comm.ScriptPath(), Stdout: outW, Stderr: errW, } if err := comm.Start(cmd); err != nil { return fmt.Errorf("Error starting script: %v", err) } + return nil }) if err == nil { diff --git a/communicator/communicator.go b/communicator/communicator.go new file mode 100644 index 000000000000..096ecbcb28a0 --- /dev/null +++ b/communicator/communicator.go @@ -0,0 +1,52 @@ +package communicator + +import ( + "fmt" + "io" + "time" + + "github.com/hashicorp/terraform/communicator/remote" + "github.com/hashicorp/terraform/communicator/ssh" + "github.com/hashicorp/terraform/terraform" +) + +// Communicator is an interface that must be implemented by all communicators +// used for any of the provisioners +type Communicator interface { + // Connect is used to setup the connection + Connect(terraform.UIOutput) error + + // Disconnect is used to terminate the connection + Disconnect() error + + // Timeout returns the configured connection timeout + Timeout() time.Duration + + // ScriptPath returns the configured script path + ScriptPath() string + + // Start executes a remote command in a new session + Start(*remote.Cmd) error + + // Upload is used to upload a single file + Upload(string, io.Reader) error + + // UploadScript is used to upload a file as a executable script + UploadScript(string, io.Reader) error + + // UploadDir is used to upload a directory + UploadDir(string, string, []string) error +} + +// New returns a configured Communicator or an error if the connection type is not supported +func New(s *terraform.InstanceState) (Communicator, error) { + connType := s.Ephemeral.ConnInfo["type"] + switch connType { + case "ssh", "": // The default connection type is ssh, so if connType is empty use ssh + return ssh.New(s) + //case "winrm": + // return winrm.New() + default: + return nil, fmt.Errorf("Connection type '%s' not supported", connType) + } +} diff --git a/communicator/communicator_test.go b/communicator/communicator_test.go new file mode 100644 index 000000000000..d2bdd0aafbe9 --- /dev/null +++ b/communicator/communicator_test.go @@ -0,0 +1,24 @@ +package communicator + +import ( + "testing" + + "github.com/hashicorp/terraform/terraform" +) + +func TestCommunicator_new(t *testing.T) { + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "telnet", + }, + }, + } + if _, err := New(r); err == nil { + t.Fatalf("expected error with telnet") + } + r.Ephemeral.ConnInfo["type"] = "ssh" + if _, err := New(r); err != nil { + t.Fatalf("err: %v", err) + } +} diff --git a/communicator/remote/command.go b/communicator/remote/command.go new file mode 100644 index 000000000000..ae16dfa882a3 --- /dev/null +++ b/communicator/remote/command.go @@ -0,0 +1,67 @@ +package remote + +import ( + "io" + "sync" +) + +// Cmd represents a remote command being prepared or run. +type Cmd struct { + // Command is the command to run remotely. This is executed as if + // it were a shell command, so you are expected to do any shell escaping + // necessary. + Command string + + // Stdin specifies the process's standard input. If Stdin is + // nil, the process reads from an empty bytes.Buffer. + Stdin io.Reader + + // Stdout and Stderr represent the process's standard output and + // error. + // + // If either is nil, it will be set to ioutil.Discard. + Stdout io.Writer + Stderr io.Writer + + // This will be set to true when the remote command has exited. It + // shouldn't be set manually by the user, but there is no harm in + // doing so. + Exited bool + + // Once Exited is true, this will contain the exit code of the process. + ExitStatus int + + // Internal fields + exitCh chan struct{} + + // This thing is a mutex, lock when making modifications concurrently + sync.Mutex +} + +// SetExited is a helper for setting that this process is exited. This +// should be called by communicators who are running a remote command in +// order to set that the command is done. +func (r *Cmd) SetExited(status int) { + r.Lock() + defer r.Unlock() + + if r.exitCh == nil { + r.exitCh = make(chan struct{}) + } + + r.Exited = true + r.ExitStatus = status + close(r.exitCh) +} + +// Wait waits for the remote command to complete. +func (r *Cmd) Wait() { + // Make sure our condition variable is initialized. + r.Lock() + if r.exitCh == nil { + r.exitCh = make(chan struct{}) + } + r.Unlock() + + <-r.exitCh +} diff --git a/communicator/remote/command_test.go b/communicator/remote/command_test.go new file mode 100644 index 000000000000..fbe5b64eba1d --- /dev/null +++ b/communicator/remote/command_test.go @@ -0,0 +1 @@ +package remote diff --git a/helper/ssh/communicator.go b/communicator/ssh/communicator.go similarity index 74% rename from helper/ssh/communicator.go rename to communicator/ssh/communicator.go index f908de97dfa3..cf949c8a3f0d 100644 --- a/helper/ssh/communicator.go +++ b/communicator/ssh/communicator.go @@ -11,84 +11,30 @@ import ( "net" "os" "path/filepath" - "sync" "time" + "github.com/hashicorp/terraform/communicator/remote" + "github.com/hashicorp/terraform/terraform" "golang.org/x/crypto/ssh" ) -// RemoteCmd represents a remote command being prepared or run. -type RemoteCmd struct { - // Command is the command to run remotely. This is executed as if - // it were a shell command, so you are expected to do any shell escaping - // necessary. - Command string - - // Stdin specifies the process's standard input. If Stdin is - // nil, the process reads from an empty bytes.Buffer. - Stdin io.Reader - - // Stdout and Stderr represent the process's standard output and - // error. - // - // If either is nil, it will be set to ioutil.Discard. - Stdout io.Writer - Stderr io.Writer - - // This will be set to true when the remote command has exited. It - // shouldn't be set manually by the user, but there is no harm in - // doing so. - Exited bool - - // Once Exited is true, this will contain the exit code of the process. - ExitStatus int - - // Internal fields - exitCh chan struct{} - - // This thing is a mutex, lock when making modifications concurrently - sync.Mutex -} - -// SetExited is a helper for setting that this process is exited. This -// should be called by communicators who are running a remote command in -// order to set that the command is done. -func (r *RemoteCmd) SetExited(status int) { - r.Lock() - defer r.Unlock() - - if r.exitCh == nil { - r.exitCh = make(chan struct{}) - } - - r.Exited = true - r.ExitStatus = status - close(r.exitCh) -} - -// Wait waits for the remote command to complete. -func (r *RemoteCmd) Wait() { - // Make sure our condition variable is initialized. - r.Lock() - if r.exitCh == nil { - r.exitCh = make(chan struct{}) - } - r.Unlock() - - <-r.exitCh -} +const ( + // DefaultShebang is added at the top of a SSH script file + DefaultShebang = "#!/bin/sh\n" +) -type SSHCommunicator struct { - client *ssh.Client - config *Config - conn net.Conn - address string +type communicator struct { + connInfo *ConnectionInfo + config *SSHConfig + client *ssh.Client + conn net.Conn + address string } -// Config is the structure used to configure the SSH communicator. -type Config struct { +// SSHConfig is the structure used to configure the SSH communicator. +type SSHConfig struct { // The configuration of the Go SSH connection - SSHConfig *ssh.ClientConfig + Config *ssh.ClientConfig // Connection returns a new connection. The current connection // in use will be closed as part of the Close method, or in the @@ -103,27 +49,105 @@ type Config struct { SSHAgentConn net.Conn } -// New creates a new packer.Communicator implementation over SSH. This takes +// New creates a new communicator implementation over SSH. This takes // an already existing TCP connection and SSH configuration. -func New(address string, config *Config) (result *SSHCommunicator, err error) { - // Establish an initial connection and connect - result = &SSHCommunicator{ - config: config, - address: address, +func New(s *terraform.InstanceState) (*communicator, error) { + connInfo, err := ParseConnectionInfo(s) + if err != nil { + return nil, err + } + + comm := &communicator{connInfo: connInfo} + + return comm, nil +} + +// Connect implementation of communicator.Communicator interface +func (c *communicator) Connect(o terraform.UIOutput) (err error) { + if c.conn != nil { + c.conn.Close() + } + + // Set the conn and client to nil since we'll recreate it + c.conn = nil + c.client = nil + + c.config, err = PrepareSSHConfig(c.connInfo) + if err != nil { + return err + } + + if o != nil { + o.Output(fmt.Sprintf( + "Connecting to remote host via SSH...\n"+ + " Host: %s\n"+ + " User: %s\n"+ + " Password: %v\n"+ + " Private key: %v"+ + " SSH Agent: %v", + c.connInfo.Host, c.connInfo.User, + c.connInfo.Password != "", + c.connInfo.KeyFile != "", + c.connInfo.Agent, + )) + } + + log.Printf("connecting to TCP connection for SSH") + c.conn, err = c.config.Connection() + if err != nil { + // Explicitly set this to the REAL nil. Connection() can return + // a nil implementation of net.Conn which will make the + // "if c.conn == nil" check fail above. Read here for more information + // on this psychotic language feature: + // + // http://golang.org/doc/faq#nil_error + c.conn = nil + + log.Printf("connection error: %s", err) + return err + } + + log.Printf("handshaking with SSH") + host := fmt.Sprintf("%s:%d", c.connInfo.Host, c.connInfo.Port) + sshConn, sshChan, req, err := ssh.NewClientConn(c.conn, host, c.config.Config) + if err != nil { + log.Printf("handshake error: %s", err) + return err + } + + c.client = ssh.NewClient(sshConn, sshChan, req) + + if o != nil { + o.Output("Connected!") } - if err = result.reconnect(); err != nil { - result = nil - return + return err +} + +// Disconnect implementation of communicator.Communicator interface +func (c *communicator) Disconnect() error { + if c.config.SSHAgentConn != nil { + return c.config.SSHAgentConn.Close() } - return + return nil } -func (c *SSHCommunicator) Start(cmd *RemoteCmd) (err error) { +// Timeout implementation of communicator.Communicator interface +func (c *communicator) Timeout() time.Duration { + return c.connInfo.TimeoutVal +} + +// Timeout implementation of communicator.Communicator interface +func (c *communicator) ScriptPath() string { + return c.connInfo.ScriptPath +} + +// Start implementation of communicator.Communicator interface +func (c *communicator) Start(cmd *remote.Cmd) error { session, err := c.newSession() if err != nil { - return + return err } // Setup our session @@ -139,15 +163,15 @@ func (c *SSHCommunicator) Start(cmd *RemoteCmd) (err error) { ssh.TTY_OP_OSPEED: 14400, // output speed = 14.4kbaud } - if err = session.RequestPty("xterm", 80, 40, termModes); err != nil { - return + if err := session.RequestPty("xterm", 80, 40, termModes); err != nil { + return err } } log.Printf("starting remote command: %s", cmd.Command) err = session.Start(cmd.Command + "\n") if err != nil { - return + return err } // Start a goroutine to wait for the session to end and set the @@ -168,10 +192,11 @@ func (c *SSHCommunicator) Start(cmd *RemoteCmd) (err error) { cmd.SetExited(exitStatus) }() - return + return nil } -func (c *SSHCommunicator) Upload(path string, input io.Reader) error { +// Upload implementation of communicator.Communicator interface +func (c *communicator) Upload(path string, input io.Reader) error { // The target directory and file for talking the SCP protocol targetDir := filepath.Dir(path) targetFile := filepath.Base(path) @@ -188,7 +213,30 @@ func (c *SSHCommunicator) Upload(path string, input io.Reader) error { return c.scpSession("scp -vt "+targetDir, scpFunc) } -func (c *SSHCommunicator) UploadDir(dst string, src string, excl []string) error { +// UploadScript implementation of communicator.Communicator interface +func (c *communicator) UploadScript(path string, input io.Reader) error { + script := bytes.NewBufferString(DefaultShebang) + script.ReadFrom(input) + + if err := c.Upload(path, script); err != nil { + return err + } + + cmd := &remote.Cmd{ + Command: fmt.Sprintf("chmod 0777 %s", c.connInfo.ScriptPath), + } + if err := c.Start(cmd); err != nil { + return fmt.Errorf( + "Error chmodding script file to 0777 in remote "+ + "machine: %s", err) + } + cmd.Wait() + + return nil +} + +// UploadDir implementation of communicator.Communicator interface +func (c *communicator) UploadDir(dst string, src string, excl []string) error { log.Printf("Upload dir '%s' to '%s'", src, dst) scpFunc := func(w io.Writer, r *bufio.Reader) error { uploadEntries := func() error { @@ -217,11 +265,12 @@ func (c *SSHCommunicator) UploadDir(dst string, src string, excl []string) error return c.scpSession("scp -rvt "+dst, scpFunc) } -func (c *SSHCommunicator) Download(string, io.Writer) error { +// Download implementation of communicator.Communicator interface +func (c *communicator) Download(string, io.Writer) error { panic("not implemented yet") } -func (c *SSHCommunicator) newSession() (session *ssh.Session, err error) { +func (c *communicator) newSession() (session *ssh.Session, err error) { log.Println("opening new ssh session") if c.client == nil { err = errors.New("client not available") @@ -231,7 +280,7 @@ func (c *SSHCommunicator) newSession() (session *ssh.Session, err error) { if err != nil { log.Printf("ssh session open error: '%s', attempting reconnect", err) - if err := c.reconnect(); err != nil { + if err := c.Connect(nil); err != nil { return nil, err } @@ -241,43 +290,7 @@ func (c *SSHCommunicator) newSession() (session *ssh.Session, err error) { return session, nil } -func (c *SSHCommunicator) reconnect() (err error) { - if c.conn != nil { - c.conn.Close() - } - - // Set the conn and client to nil since we'll recreate it - c.conn = nil - c.client = nil - - log.Printf("reconnecting to TCP connection for SSH") - c.conn, err = c.config.Connection() - if err != nil { - // Explicitly set this to the REAL nil. Connection() can return - // a nil implementation of net.Conn which will make the - // "if c.conn == nil" check fail above. Read here for more information - // on this psychotic language feature: - // - // http://golang.org/doc/faq#nil_error - c.conn = nil - - log.Printf("reconnection error: %s", err) - return - } - - log.Printf("handshaking with SSH") - sshConn, sshChan, req, err := ssh.NewClientConn(c.conn, c.address, c.config.SSHConfig) - if err != nil { - log.Printf("handshake error: %s", err) - } - if sshConn != nil { - c.client = ssh.NewClient(sshConn, sshChan, req) - } - - return -} - -func (c *SSHCommunicator) scpSession(scpCommand string, f func(io.Writer, *bufio.Reader) error) error { +func (c *communicator) scpSession(scpCommand string, f func(io.Writer, *bufio.Reader) error) error { session, err := c.newSession() if err != nil { return err @@ -382,7 +395,7 @@ func checkSCPStatus(r *bufio.Reader) error { func scpUploadFile(dst string, src io.Reader, w io.Writer, r *bufio.Reader) error { // Create a temporary file where we can copy the contents of the src // so that we can determine the length, since SCP is length-prefixed. - tf, err := ioutil.TempFile("", "packer-upload") + tf, err := ioutil.TempFile("", "terraform-upload") if err != nil { return fmt.Errorf("Error creating temporary file for upload: %s", err) } diff --git a/helper/ssh/communicator_test.go b/communicator/ssh/communicator_test.go similarity index 79% rename from helper/ssh/communicator_test.go rename to communicator/ssh/communicator_test.go index b71321701070..e23ad9625b91 100644 --- a/helper/ssh/communicator_test.go +++ b/communicator/ssh/communicator_test.go @@ -6,8 +6,11 @@ import ( "bytes" "fmt" "net" + "strings" "testing" + "github.com/hashicorp/terraform/communicator/remote" + "github.com/hashicorp/terraform/terraform" "golang.org/x/crypto/ssh" ) @@ -105,67 +108,62 @@ func newMockLineServer(t *testing.T) string { } func TestNew_Invalid(t *testing.T) { - clientConfig := &ssh.ClientConfig{ - User: "user", - Auth: []ssh.AuthMethod{ - ssh.Password("i-am-invalid"), - }, - } - address := newMockLineServer(t) - conn := func() (net.Conn, error) { - conn, err := net.Dial("tcp", address) - if err != nil { - t.Errorf("Unable to accept incoming connection: %v", err) - } - return conn, err + parts := strings.Split(address, ":") + + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "ssh", + "user": "user", + "password": "i-am-invalid", + "host": parts[0], + "port": parts[1], + "timeout": "30s", + }, + }, } - config := &Config{ - Connection: conn, - SSHConfig: clientConfig, + c, err := New(r) + if err != nil { + t.Fatalf("error creating communicator: %s", err) } - _, err := New(address, config) + err = c.Connect(nil) if err == nil { t.Fatal("should have had an error connecting") } } func TestStart(t *testing.T) { - clientConfig := &ssh.ClientConfig{ - User: "user", - Auth: []ssh.AuthMethod{ - ssh.Password("pass"), - }, - } - address := newMockLineServer(t) - conn := func() (net.Conn, error) { - conn, err := net.Dial("tcp", address) - if err != nil { - t.Fatalf("unable to dial to remote side: %s", err) - } - return conn, err - } - - config := &Config{ - Connection: conn, - SSHConfig: clientConfig, + parts := strings.Split(address, ":") + + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "ssh", + "user": "user", + "password": "pass", + "host": parts[0], + "port": parts[1], + "timeout": "30s", + }, + }, } - client, err := New(address, config) + c, err := New(r) if err != nil { - t.Fatalf("error connecting to SSH: %s", err) + t.Fatalf("error creating communicator: %s", err) } - var cmd RemoteCmd + var cmd remote.Cmd stdout := new(bytes.Buffer) cmd.Command = "echo foo" cmd.Stdout = stdout - err = client.Start(&cmd) + err = c.Start(&cmd) if err != nil { - t.Fatalf("error executing command: %s", err) + t.Fatalf("error executing remote command: %s", err) } } diff --git a/helper/ssh/password.go b/communicator/ssh/password.go similarity index 100% rename from helper/ssh/password.go rename to communicator/ssh/password.go diff --git a/helper/ssh/password_test.go b/communicator/ssh/password_test.go similarity index 100% rename from helper/ssh/password_test.go rename to communicator/ssh/password_test.go diff --git a/helper/ssh/provisioner.go b/communicator/ssh/provisioner.go similarity index 60% rename from helper/ssh/provisioner.go rename to communicator/ssh/provisioner.go index 69468de7dbaf..228eb81b8fe1 100644 --- a/helper/ssh/provisioner.go +++ b/communicator/ssh/provisioner.go @@ -5,11 +5,8 @@ import ( "fmt" "io/ioutil" "log" - "math/rand" "net" "os" - "strconv" - "strings" "time" "github.com/hashicorp/terraform/terraform" @@ -34,10 +31,10 @@ const ( DefaultTimeout = 5 * time.Minute ) -// SSHConfig is decoded from the ConnInfo of the resource. These -// are the only keys we look at. If a KeyFile is given, that is used -// instead of a password. -type SSHConfig struct { +// ConnectionInfo is decoded from the ConnInfo of the resource. These are the +// only keys we look at. If a KeyFile is given, that is used instead +// of a password. +type ConnectionInfo struct { User string Password string KeyFile string `mapstructure:"key_file"` @@ -49,31 +46,13 @@ type SSHConfig struct { TimeoutVal time.Duration `mapstructure:"-"` } -func (c *SSHConfig) RemotePath() string { - return strings.Replace( - c.ScriptPath, "%RAND%", - strconv.FormatInt(int64(rand.Int31()), 10), -1) -} - -// VerifySSH is used to verify the ConnInfo is usable by remote-exec -func VerifySSH(s *terraform.InstanceState) error { - connType := s.Ephemeral.ConnInfo["type"] - switch connType { - case "": - case "ssh": - default: - return fmt.Errorf("Connection type '%s' not supported", connType) - } - return nil -} - -// ParseSSHConfig is used to convert the ConnInfo of the InstanceState into -// a SSHConfig struct -func ParseSSHConfig(s *terraform.InstanceState) (*SSHConfig, error) { - sshConf := &SSHConfig{} +// ParseConnectionInfo is used to convert the ConnInfo of the InstanceState into +// a ConnectionInfo struct +func ParseConnectionInfo(s *terraform.InstanceState) (*ConnectionInfo, error) { + connInfo := &ConnectionInfo{} decConf := &mapstructure.DecoderConfig{ WeaklyTypedInput: true, - Result: sshConf, + Result: connInfo, } dec, err := mapstructure.NewDecoder(decConf) if err != nil { @@ -82,21 +61,21 @@ func ParseSSHConfig(s *terraform.InstanceState) (*SSHConfig, error) { if err := dec.Decode(s.Ephemeral.ConnInfo); err != nil { return nil, err } - if sshConf.User == "" { - sshConf.User = DefaultUser + if connInfo.User == "" { + connInfo.User = DefaultUser } - if sshConf.Port == 0 { - sshConf.Port = DefaultPort + if connInfo.Port == 0 { + connInfo.Port = DefaultPort } - if sshConf.ScriptPath == "" { - sshConf.ScriptPath = DefaultScriptPath + if connInfo.ScriptPath == "" { + connInfo.ScriptPath = DefaultScriptPath } - if sshConf.Timeout != "" { - sshConf.TimeoutVal = safeDuration(sshConf.Timeout, DefaultTimeout) + if connInfo.Timeout != "" { + connInfo.TimeoutVal = safeDuration(connInfo.Timeout, DefaultTimeout) } else { - sshConf.TimeoutVal = DefaultTimeout + connInfo.TimeoutVal = DefaultTimeout } - return sshConf, nil + return connInfo, nil } // safeDuration returns either the parsed duration or a default value @@ -109,16 +88,16 @@ func safeDuration(dur string, defaultDur time.Duration) time.Duration { return d } -// PrepareConfig is used to turn the *SSHConfig provided into a -// usable *Config for client initialization. -func PrepareConfig(conf *SSHConfig) (*Config, error) { +// PrepareSSHConfig is used to turn the *ConnectionInfo provided into a +// usable *SSHConfig for client initialization. +func PrepareSSHConfig(connInfo *ConnectionInfo) (*SSHConfig, error) { var conn net.Conn var err error sshConf := &ssh.ClientConfig{ - User: conf.User, + User: connInfo.User, } - if conf.Agent { + if connInfo.Agent { sshAuthSock := os.Getenv("SSH_AUTH_SOCK") if sshAuthSock == "" { @@ -138,14 +117,14 @@ func PrepareConfig(conf *SSHConfig) (*Config, error) { sshConf.Auth = append(sshConf.Auth, ssh.PublicKeys(signers...)) } - if conf.KeyFile != "" { - fullPath, err := homedir.Expand(conf.KeyFile) + if connInfo.KeyFile != "" { + fullPath, err := homedir.Expand(connInfo.KeyFile) if err != nil { return nil, fmt.Errorf("Failed to expand home directory: %v", err) } key, err := ioutil.ReadFile(fullPath) if err != nil { - return nil, fmt.Errorf("Failed to read key file '%s': %v", conf.KeyFile, err) + return nil, fmt.Errorf("Failed to read key file '%s': %v", connInfo.KeyFile, err) } // We parse the private key on our own first so that we can @@ -153,40 +132,32 @@ func PrepareConfig(conf *SSHConfig) (*Config, error) { block, _ := pem.Decode(key) if block == nil { return nil, fmt.Errorf( - "Failed to read key '%s': no key found", conf.KeyFile) + "Failed to read key '%s': no key found", connInfo.KeyFile) } if block.Headers["Proc-Type"] == "4,ENCRYPTED" { return nil, fmt.Errorf( "Failed to read key '%s': password protected keys are\n"+ - "not supported. Please decrypt the key prior to use.", conf.KeyFile) + "not supported. Please decrypt the key prior to use.", connInfo.KeyFile) } signer, err := ssh.ParsePrivateKey(key) if err != nil { - return nil, fmt.Errorf("Failed to parse key file '%s': %v", conf.KeyFile, err) + return nil, fmt.Errorf("Failed to parse key file '%s': %v", connInfo.KeyFile, err) } sshConf.Auth = append(sshConf.Auth, ssh.PublicKeys(signer)) } - if conf.Password != "" { + if connInfo.Password != "" { sshConf.Auth = append(sshConf.Auth, - ssh.Password(conf.Password)) + ssh.Password(connInfo.Password)) sshConf.Auth = append(sshConf.Auth, - ssh.KeyboardInteractive(PasswordKeyboardInteractive(conf.Password))) + ssh.KeyboardInteractive(PasswordKeyboardInteractive(connInfo.Password))) } - host := fmt.Sprintf("%s:%d", conf.Host, conf.Port) - config := &Config{ - SSHConfig: sshConf, + host := fmt.Sprintf("%s:%d", connInfo.Host, connInfo.Port) + config := &SSHConfig{ + Config: sshConf, Connection: ConnectFunc("tcp", host), SSHAgentConn: conn, } return config, nil } - -func (c *Config) CleanupConfig() error { - if c.SSHAgentConn != nil { - return c.SSHAgentConn.Close() - } - - return nil -} diff --git a/communicator/ssh/provisioner_test.go b/communicator/ssh/provisioner_test.go new file mode 100644 index 000000000000..150524c65754 --- /dev/null +++ b/communicator/ssh/provisioner_test.go @@ -0,0 +1,50 @@ +package ssh + +import ( + "testing" + + "github.com/hashicorp/terraform/terraform" +) + +func TestProvisioner_connInfo(t *testing.T) { + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "ssh", + "user": "root", + "password": "supersecret", + "key_file": "/my/key/file.pem", + "host": "127.0.0.1", + "port": "22", + "timeout": "30s", + }, + }, + } + + conf, err := ParseConnectionInfo(r) + if err != nil { + t.Fatalf("err: %v", err) + } + + if conf.User != "root" { + t.Fatalf("bad: %v", conf) + } + if conf.Password != "supersecret" { + t.Fatalf("bad: %v", conf) + } + if conf.KeyFile != "/my/key/file.pem" { + t.Fatalf("bad: %v", conf) + } + if conf.Host != "127.0.0.1" { + t.Fatalf("bad: %v", conf) + } + if conf.Port != 22 { + t.Fatalf("bad: %v", conf) + } + if conf.Timeout != "30s" { + t.Fatalf("bad: %v", conf) + } + if conf.ScriptPath != DefaultScriptPath { + t.Fatalf("bad: %v", conf) + } +} diff --git a/helper/ssh/provisioner_test.go b/helper/ssh/provisioner_test.go deleted file mode 100644 index 4559a4f20e00..000000000000 --- a/helper/ssh/provisioner_test.go +++ /dev/null @@ -1,97 +0,0 @@ -package ssh - -import ( - "regexp" - "testing" - - "github.com/hashicorp/terraform/terraform" -) - -func TestSSHConfig_RemotePath(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 { - config := &SSHConfig{ScriptPath: tc.Input} - output := config.RemotePath() - - 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) - } - } -} - -func TestResourceProvider_verifySSH(t *testing.T) { - r := &terraform.InstanceState{ - Ephemeral: terraform.EphemeralState{ - ConnInfo: map[string]string{ - "type": "telnet", - }, - }, - } - if err := VerifySSH(r); err == nil { - t.Fatalf("expected error with telnet") - } - r.Ephemeral.ConnInfo["type"] = "ssh" - if err := VerifySSH(r); err != nil { - t.Fatalf("err: %v", err) - } -} - -func TestResourceProvider_sshConfig(t *testing.T) { - r := &terraform.InstanceState{ - Ephemeral: terraform.EphemeralState{ - ConnInfo: map[string]string{ - "type": "ssh", - "user": "root", - "password": "supersecret", - "key_file": "/my/key/file.pem", - "host": "127.0.0.1", - "port": "22", - "timeout": "30s", - }, - }, - } - - conf, err := ParseSSHConfig(r) - if err != nil { - t.Fatalf("err: %v", err) - } - - if conf.User != "root" { - t.Fatalf("bad: %v", conf) - } - if conf.Password != "supersecret" { - t.Fatalf("bad: %v", conf) - } - if conf.KeyFile != "/my/key/file.pem" { - t.Fatalf("bad: %v", conf) - } - if conf.Host != "127.0.0.1" { - t.Fatalf("bad: %v", conf) - } - if conf.Port != 22 { - t.Fatalf("bad: %v", conf) - } - if conf.Timeout != "30s" { - t.Fatalf("bad: %v", conf) - } - if conf.ScriptPath != DefaultScriptPath { - t.Fatalf("bad: %v", conf) - } -} diff --git a/plugin/client_test.go b/plugin/client_test.go index d558b4912ebd..68b995c13972 100644 --- a/plugin/client_test.go +++ b/plugin/client_test.go @@ -99,7 +99,7 @@ func TestClient_Stderr(t *testing.T) { func TestClient_Stdin(t *testing.T) { // Overwrite stdin for this test with a temporary file - tf, err := ioutil.TempFile("", "packer") + tf, err := ioutil.TempFile("", "terraform") if err != nil { t.Fatalf("err: %s", err) } From b1c6a3f63f97cfb174b894e332f26e5f6225ef56 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 10 Apr 2015 00:50:48 +0200 Subject: [PATCH 02/10] Few small fixes to make the last tests also pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reason why the shebang is removed from these tests, is because the shebang is only needed for SSH/Linux connections. So in the new setup the shebang line is added in the SSH communicator instead of in the resource provisioner itself… --- .../remote-exec/resource_provisioner_test.go | 23 +++++++++---------- .../remote-exec/test-fixtures/script1.sh | 1 - 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/builtin/provisioners/remote-exec/resource_provisioner_test.go b/builtin/provisioners/remote-exec/resource_provisioner_test.go index 47a723947419..a10520fb816c 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner_test.go +++ b/builtin/provisioners/remote-exec/resource_provisioner_test.go @@ -41,6 +41,11 @@ func TestResourceProvider_Validate_bad(t *testing.T) { } } +var expectedScriptOut = `cd /tmp +wget http://foobar +exit 0 +` + func TestResourceProvider_generateScript(t *testing.T) { p := new(ResourceProvisioner) conf := testConfig(t, map[string]interface{}{ @@ -60,12 +65,6 @@ func TestResourceProvider_generateScript(t *testing.T) { } } -var expectedScriptOut = `#!/bin/sh -cd /tmp -wget http://foobar -exit 0 -` - func TestResourceProvider_CollectScripts_inline(t *testing.T) { p := new(ResourceProvisioner) conf := testConfig(t, map[string]interface{}{ @@ -91,8 +90,8 @@ func TestResourceProvider_CollectScripts_inline(t *testing.T) { t.Fatalf("err: %v", err) } - if string(out.Bytes()) != expectedScriptOut { - t.Fatalf("bad: %v", out.Bytes()) + if out.String() != expectedScriptOut { + t.Fatalf("bad: %v", out.String()) } } @@ -117,8 +116,8 @@ func TestResourceProvider_CollectScripts_script(t *testing.T) { t.Fatalf("err: %v", err) } - if string(out.Bytes()) != expectedScriptOut { - t.Fatalf("bad: %v", out.Bytes()) + if out.String() != expectedScriptOut { + t.Fatalf("bad: %v", out.String()) } } @@ -148,8 +147,8 @@ func TestResourceProvider_CollectScripts_scripts(t *testing.T) { t.Fatalf("err: %v", err) } - if string(out.Bytes()) != expectedScriptOut { - t.Fatalf("bad: %v", out.Bytes()) + if out.String() != expectedScriptOut { + t.Fatalf("bad: %v", out.String()) } } } diff --git a/builtin/provisioners/remote-exec/test-fixtures/script1.sh b/builtin/provisioners/remote-exec/test-fixtures/script1.sh index cd22f3e63003..81b3d5af86a2 100755 --- a/builtin/provisioners/remote-exec/test-fixtures/script1.sh +++ b/builtin/provisioners/remote-exec/test-fixtures/script1.sh @@ -1,4 +1,3 @@ -#!/bin/sh cd /tmp wget http://foobar exit 0 From 4a29c714e5d70dda6d8c413844e691d1702cb27e Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 10 Apr 2015 20:34:46 +0200 Subject: [PATCH 03/10] Adding support for WinRM --- .../provisioners/file/resource_provisioner.go | 2 +- communicator/communicator.go | 9 +- communicator/communicator_test.go | 6 + communicator/ssh/communicator.go | 89 ++++----- communicator/ssh/provisioner.go | 24 +-- communicator/ssh/provisioner_test.go | 2 +- communicator/winrm/communicator.go | 186 ++++++++++++++++++ communicator/winrm/communicator_test.go | 1 + communicator/winrm/provisioner.go | 109 ++++++++++ communicator/winrm/provisioner_test.go | 103 ++++++++++ 10 files changed, 467 insertions(+), 64 deletions(-) create mode 100644 communicator/winrm/communicator.go create mode 100644 communicator/winrm/communicator_test.go create mode 100644 communicator/winrm/provisioner.go create mode 100644 communicator/winrm/provisioner_test.go diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go index 1817693a8642..9484d3f18f4f 100644 --- a/builtin/provisioners/file/resource_provisioner.go +++ b/builtin/provisioners/file/resource_provisioner.go @@ -76,7 +76,7 @@ func (p *ResourceProvisioner) copyFiles(comm communicator.Communicator, src, dst // If we're uploading a directory, short circuit and do that if info.IsDir() { - if err := comm.UploadDir(dst, src, nil); err != nil { + if err := comm.UploadDir(dst, src); err != nil { return fmt.Errorf("Upload failed: %v", err) } return nil diff --git a/communicator/communicator.go b/communicator/communicator.go index 096ecbcb28a0..5fa2631a49ff 100644 --- a/communicator/communicator.go +++ b/communicator/communicator.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/communicator/remote" "github.com/hashicorp/terraform/communicator/ssh" + "github.com/hashicorp/terraform/communicator/winrm" "github.com/hashicorp/terraform/terraform" ) @@ -35,7 +36,7 @@ type Communicator interface { UploadScript(string, io.Reader) error // UploadDir is used to upload a directory - UploadDir(string, string, []string) error + UploadDir(string, string) error } // New returns a configured Communicator or an error if the connection type is not supported @@ -44,9 +45,9 @@ func New(s *terraform.InstanceState) (Communicator, error) { switch connType { case "ssh", "": // The default connection type is ssh, so if connType is empty use ssh return ssh.New(s) - //case "winrm": - // return winrm.New() + case "winrm": + return winrm.New(s) default: - return nil, fmt.Errorf("Connection type '%s' not supported", connType) + return nil, fmt.Errorf("connection type '%s' not supported", connType) } } diff --git a/communicator/communicator_test.go b/communicator/communicator_test.go index d2bdd0aafbe9..33a91cd6f365 100644 --- a/communicator/communicator_test.go +++ b/communicator/communicator_test.go @@ -17,8 +17,14 @@ func TestCommunicator_new(t *testing.T) { if _, err := New(r); err == nil { t.Fatalf("expected error with telnet") } + r.Ephemeral.ConnInfo["type"] = "ssh" if _, err := New(r); err != nil { t.Fatalf("err: %v", err) } + + r.Ephemeral.ConnInfo["type"] = "winrm" + if _, err := New(r); err != nil { + t.Fatalf("err: %v", err) + } } diff --git a/communicator/ssh/communicator.go b/communicator/ssh/communicator.go index cf949c8a3f0d..8dd629adf7f4 100644 --- a/communicator/ssh/communicator.go +++ b/communicator/ssh/communicator.go @@ -23,47 +23,54 @@ const ( DefaultShebang = "#!/bin/sh\n" ) -type communicator struct { - connInfo *ConnectionInfo - config *SSHConfig +// Communicator represents the SSH communicator +type Communicator struct { + connInfo *connectionInfo client *ssh.Client + config *sshConfig conn net.Conn address string } -// SSHConfig is the structure used to configure the SSH communicator. -type SSHConfig struct { +type sshConfig struct { // The configuration of the Go SSH connection - Config *ssh.ClientConfig + config *ssh.ClientConfig - // Connection returns a new connection. The current connection + // connection returns a new connection. The current connection // in use will be closed as part of the Close method, or in the // case an error occurs. - Connection func() (net.Conn, error) + connection func() (net.Conn, error) - // NoPty, if true, will not request a pty from the remote end. - NoPty bool + // noPty, if true, will not request a pty from the remote end. + noPty bool - // SSHAgentConn is a pointer to the UNIX connection for talking with the + // sshAgentConn is a pointer to the UNIX connection for talking with the // ssh-agent. - SSHAgentConn net.Conn + sshAgentConn net.Conn } -// New creates a new communicator implementation over SSH. This takes -// an already existing TCP connection and SSH configuration. -func New(s *terraform.InstanceState) (*communicator, error) { - connInfo, err := ParseConnectionInfo(s) +// New creates a new communicator implementation over SSH. +func New(s *terraform.InstanceState) (*Communicator, error) { + connInfo, err := parseConnectionInfo(s) if err != nil { return nil, err } - comm := &communicator{connInfo: connInfo} + config, err := prepareSSHConfig(connInfo) + if err != nil { + return nil, err + } + + comm := &Communicator{ + connInfo: connInfo, + config: config, + } return comm, nil } // Connect implementation of communicator.Communicator interface -func (c *communicator) Connect(o terraform.UIOutput) (err error) { +func (c *Communicator) Connect(o terraform.UIOutput) (err error) { if c.conn != nil { c.conn.Close() } @@ -72,19 +79,14 @@ func (c *communicator) Connect(o terraform.UIOutput) (err error) { c.conn = nil c.client = nil - c.config, err = PrepareSSHConfig(c.connInfo) - if err != nil { - return err - } - if o != nil { o.Output(fmt.Sprintf( "Connecting to remote host via SSH...\n"+ " Host: %s\n"+ " User: %s\n"+ - " Password: %v\n"+ - " Private key: %v"+ - " SSH Agent: %v", + " Password: %t\n"+ + " Private key: %t\n"+ + " SSH Agent: %t", c.connInfo.Host, c.connInfo.User, c.connInfo.Password != "", c.connInfo.KeyFile != "", @@ -93,7 +95,7 @@ func (c *communicator) Connect(o terraform.UIOutput) (err error) { } log.Printf("connecting to TCP connection for SSH") - c.conn, err = c.config.Connection() + c.conn, err = c.config.connection() if err != nil { // Explicitly set this to the REAL nil. Connection() can return // a nil implementation of net.Conn which will make the @@ -109,7 +111,7 @@ func (c *communicator) Connect(o terraform.UIOutput) (err error) { log.Printf("handshaking with SSH") host := fmt.Sprintf("%s:%d", c.connInfo.Host, c.connInfo.Port) - sshConn, sshChan, req, err := ssh.NewClientConn(c.conn, host, c.config.Config) + sshConn, sshChan, req, err := ssh.NewClientConn(c.conn, host, c.config.config) if err != nil { log.Printf("handshake error: %s", err) return err @@ -125,26 +127,26 @@ func (c *communicator) Connect(o terraform.UIOutput) (err error) { } // Disconnect implementation of communicator.Communicator interface -func (c *communicator) Disconnect() error { - if c.config.SSHAgentConn != nil { - return c.config.SSHAgentConn.Close() +func (c *Communicator) Disconnect() error { + if c.config.sshAgentConn != nil { + return c.config.sshAgentConn.Close() } return nil } // Timeout implementation of communicator.Communicator interface -func (c *communicator) Timeout() time.Duration { +func (c *Communicator) Timeout() time.Duration { return c.connInfo.TimeoutVal } -// Timeout implementation of communicator.Communicator interface -func (c *communicator) ScriptPath() string { +// ScriptPath implementation of communicator.Communicator interface +func (c *Communicator) ScriptPath() string { return c.connInfo.ScriptPath } // Start implementation of communicator.Communicator interface -func (c *communicator) Start(cmd *remote.Cmd) error { +func (c *Communicator) Start(cmd *remote.Cmd) error { session, err := c.newSession() if err != nil { return err @@ -155,7 +157,7 @@ func (c *communicator) Start(cmd *remote.Cmd) error { session.Stdout = cmd.Stdout session.Stderr = cmd.Stderr - if !c.config.NoPty { + if !c.config.noPty { // Request a PTY termModes := ssh.TerminalModes{ ssh.ECHO: 0, // do not echo @@ -196,7 +198,7 @@ func (c *communicator) Start(cmd *remote.Cmd) error { } // Upload implementation of communicator.Communicator interface -func (c *communicator) Upload(path string, input io.Reader) error { +func (c *Communicator) Upload(path string, input io.Reader) error { // The target directory and file for talking the SCP protocol targetDir := filepath.Dir(path) targetFile := filepath.Base(path) @@ -214,7 +216,7 @@ func (c *communicator) Upload(path string, input io.Reader) error { } // UploadScript implementation of communicator.Communicator interface -func (c *communicator) UploadScript(path string, input io.Reader) error { +func (c *Communicator) UploadScript(path string, input io.Reader) error { script := bytes.NewBufferString(DefaultShebang) script.ReadFrom(input) @@ -236,7 +238,7 @@ func (c *communicator) UploadScript(path string, input io.Reader) error { } // UploadDir implementation of communicator.Communicator interface -func (c *communicator) UploadDir(dst string, src string, excl []string) error { +func (c *Communicator) UploadDir(dst string, src string) error { log.Printf("Upload dir '%s' to '%s'", src, dst) scpFunc := func(w io.Writer, r *bufio.Reader) error { uploadEntries := func() error { @@ -265,12 +267,7 @@ func (c *communicator) UploadDir(dst string, src string, excl []string) error { return c.scpSession("scp -rvt "+dst, scpFunc) } -// Download implementation of communicator.Communicator interface -func (c *communicator) Download(string, io.Writer) error { - panic("not implemented yet") -} - -func (c *communicator) newSession() (session *ssh.Session, err error) { +func (c *Communicator) newSession() (session *ssh.Session, err error) { log.Println("opening new ssh session") if c.client == nil { err = errors.New("client not available") @@ -290,7 +287,7 @@ func (c *communicator) newSession() (session *ssh.Session, err error) { return session, nil } -func (c *communicator) scpSession(scpCommand string, f func(io.Writer, *bufio.Reader) error) error { +func (c *Communicator) scpSession(scpCommand string, f func(io.Writer, *bufio.Reader) error) error { session, err := c.newSession() if err != nil { return err diff --git a/communicator/ssh/provisioner.go b/communicator/ssh/provisioner.go index 228eb81b8fe1..affa05625326 100644 --- a/communicator/ssh/provisioner.go +++ b/communicator/ssh/provisioner.go @@ -17,7 +17,7 @@ import ( ) const ( - // DefaultUser is used if there is no default user given + // DefaultUser is used if there is no user given DefaultUser = "root" // DefaultPort is used if there is no port given @@ -31,10 +31,10 @@ const ( DefaultTimeout = 5 * time.Minute ) -// ConnectionInfo is decoded from the ConnInfo of the resource. These are the +// connectionInfo is decoded from the ConnInfo of the resource. These are the // only keys we look at. If a KeyFile is given, that is used instead // of a password. -type ConnectionInfo struct { +type connectionInfo struct { User string Password string KeyFile string `mapstructure:"key_file"` @@ -46,10 +46,10 @@ type ConnectionInfo struct { TimeoutVal time.Duration `mapstructure:"-"` } -// ParseConnectionInfo is used to convert the ConnInfo of the InstanceState into +// parseConnectionInfo is used to convert the ConnInfo of the InstanceState into // a ConnectionInfo struct -func ParseConnectionInfo(s *terraform.InstanceState) (*ConnectionInfo, error) { - connInfo := &ConnectionInfo{} +func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) { + connInfo := &connectionInfo{} decConf := &mapstructure.DecoderConfig{ WeaklyTypedInput: true, Result: connInfo, @@ -88,9 +88,9 @@ func safeDuration(dur string, defaultDur time.Duration) time.Duration { return d } -// PrepareSSHConfig is used to turn the *ConnectionInfo provided into a +// prepareSSHConfig is used to turn the *ConnectionInfo provided into a // usable *SSHConfig for client initialization. -func PrepareSSHConfig(connInfo *ConnectionInfo) (*SSHConfig, error) { +func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) { var conn net.Conn var err error @@ -154,10 +154,10 @@ func PrepareSSHConfig(connInfo *ConnectionInfo) (*SSHConfig, error) { ssh.KeyboardInteractive(PasswordKeyboardInteractive(connInfo.Password))) } host := fmt.Sprintf("%s:%d", connInfo.Host, connInfo.Port) - config := &SSHConfig{ - Config: sshConf, - Connection: ConnectFunc("tcp", host), - SSHAgentConn: conn, + config := &sshConfig{ + config: sshConf, + connection: ConnectFunc("tcp", host), + sshAgentConn: conn, } return config, nil } diff --git a/communicator/ssh/provisioner_test.go b/communicator/ssh/provisioner_test.go index 150524c65754..33c2b7b7b9cd 100644 --- a/communicator/ssh/provisioner_test.go +++ b/communicator/ssh/provisioner_test.go @@ -21,7 +21,7 @@ func TestProvisioner_connInfo(t *testing.T) { }, } - conf, err := ParseConnectionInfo(r) + conf, err := parseConnectionInfo(r) if err != nil { t.Fatalf("err: %v", err) } diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go new file mode 100644 index 000000000000..b723d7ff3ee2 --- /dev/null +++ b/communicator/winrm/communicator.go @@ -0,0 +1,186 @@ +package winrm + +import ( + "fmt" + "io" + "log" + "time" + + "github.com/hashicorp/terraform/communicator/remote" + "github.com/hashicorp/terraform/terraform" + "github.com/masterzen/winrm/winrm" + "github.com/packer-community/winrmcp/winrmcp" +) + +// Communicator represents the WinRM communicator +type Communicator struct { + connInfo *connectionInfo + client *winrm.Client + endpoint *winrm.Endpoint +} + +// New creates a new communicator implementation over WinRM. +//func New(endpoint *winrm.Endpoint, user string, password string, timeout time.Duration) (*communicator, error) { +func New(s *terraform.InstanceState) (*Communicator, error) { + connInfo, err := parseConnectionInfo(s) + if err != nil { + return nil, err + } + + endpoint := &winrm.Endpoint{ + Host: connInfo.Host, + Port: connInfo.Port, + HTTPS: connInfo.HTTPS, + Insecure: connInfo.Insecure, + CACert: connInfo.CACert, + } + + comm := &Communicator{ + connInfo: connInfo, + endpoint: endpoint, + } + + return comm, nil +} + +// Connect implementation of communicator.Communicator interface +func (c *Communicator) Connect(o terraform.UIOutput) error { + if c.client != nil { + return nil + } + + params := winrm.DefaultParameters() + params.Timeout = formatDuration(c.Timeout()) + + client, err := winrm.NewClientWithParameters( + c.endpoint, c.connInfo.User, c.connInfo.Password, params) + if err != nil { + return err + } + + if o != nil { + o.Output(fmt.Sprintf( + "Connecting to remote host via WinRM...\n"+ + " Host: %s\n"+ + " Port: %d\n"+ + " User: %s\n"+ + " Password: %t\n"+ + " HTTPS: %t\n"+ + " Insecure: %t\n"+ + " CACert: %t", + c.connInfo.Host, + c.connInfo.Port, + c.connInfo.User, + c.connInfo.Password != "", + c.connInfo.HTTPS, + c.connInfo.Insecure, + c.connInfo.CACert != nil, + )) + } + + log.Printf("connecting to remote shell using WinRM") + shell, err := client.CreateShell() + if err != nil { + log.Printf("connection error: %s", err) + return err + } + + err = shell.Close() + if err != nil { + log.Printf("error closing connection: %s", err) + return err + } + + if o != nil { + o.Output("Connected!") + } + + c.client = client + + return nil +} + +// Disconnect implementation of communicator.Communicator interface +func (c *Communicator) Disconnect() error { + c.client = nil + return nil +} + +// Timeout implementation of communicator.Communicator interface +func (c *Communicator) Timeout() time.Duration { + return c.connInfo.TimeoutVal +} + +// ScriptPath implementation of communicator.Communicator interface +func (c *Communicator) ScriptPath() string { + return c.connInfo.ScriptPath +} + +// Start implementation of communicator.Communicator interface +func (c *Communicator) Start(rc *remote.Cmd) error { + log.Printf("starting remote command: %s", rc.Command) + + err := c.Connect(nil) + if err != nil { + return err + } + + shell, err := c.client.CreateShell() + if err != nil { + return err + } + + cmd, err := shell.Execute(rc.Command) + if err != nil { + return err + } + + go runCommand(shell, cmd, rc) + return nil +} + +func runCommand(shell *winrm.Shell, cmd *winrm.Command, rc *remote.Cmd) { + defer shell.Close() + + go io.Copy(rc.Stdout, cmd.Stdout) + go io.Copy(rc.Stderr, cmd.Stderr) + + cmd.Wait() + rc.SetExited(cmd.ExitCode()) +} + +// Upload implementation of communicator.Communicator interface +func (c *Communicator) Upload(path string, input io.Reader) error { + wcp, err := c.newCopyClient() + if err != nil { + return err + } + return wcp.Write(path, input) +} + +// UploadScript implementation of communicator.Communicator interface +func (c *Communicator) UploadScript(path string, input io.Reader) error { + return c.Upload(path, input) +} + +// UploadDir implementation of communicator.Communicator interface +func (c *Communicator) UploadDir(dst string, src string) error { + log.Printf("Upload dir '%s' to '%s'", src, dst) + wcp, err := c.newCopyClient() + if err != nil { + return err + } + return wcp.Copy(src, dst) +} + +func (c *Communicator) newCopyClient() (*winrmcp.Winrmcp, error) { + addr := fmt.Sprintf("%s:%d", c.endpoint.Host, c.endpoint.Port) + return winrmcp.New(addr, &winrmcp.Config{ + Auth: winrmcp.Auth{ + User: c.connInfo.User, + Password: c.connInfo.Password, + }, + OperationTimeout: c.Timeout(), + MaxOperationsPerShell: 15, // lowest common denominator + }) +} diff --git a/communicator/winrm/communicator_test.go b/communicator/winrm/communicator_test.go new file mode 100644 index 000000000000..54f77496131b --- /dev/null +++ b/communicator/winrm/communicator_test.go @@ -0,0 +1 @@ +package winrm diff --git a/communicator/winrm/provisioner.go b/communicator/winrm/provisioner.go new file mode 100644 index 000000000000..ec12e4dcd642 --- /dev/null +++ b/communicator/winrm/provisioner.go @@ -0,0 +1,109 @@ +package winrm + +import ( + "fmt" + "log" + "strings" + "time" + + "github.com/hashicorp/terraform/terraform" + "github.com/mitchellh/mapstructure" +) + +const ( + // DefaultUser is used if there is no user given + DefaultUser = "Administrator" + + // DefaultPort is used if there is no port given + DefaultPort = 5985 + + // DefaultScriptPath is used as the path to copy the file to + // for remote execution if not provided otherwise. + DefaultScriptPath = "C:/Temp/script.cmd" + + // DefaultTimeout is used if there is no timeout given + DefaultTimeout = 5 * time.Minute +) + +// connectionInfo is decoded from the ConnInfo of the resource. These are the +// only keys we look at. If a KeyFile is given, that is used instead +// of a password. +type connectionInfo struct { + User string + Password string + Host string + Port int + HTTPS bool + Insecure bool + CACert *[]byte `mapstructure:"ca_cert"` + Timeout string + ScriptPath string `mapstructure:"script_path"` + TimeoutVal time.Duration `mapstructure:"-"` +} + +// parseConnectionInfo is used to convert the ConnInfo of the InstanceState into +// a ConnectionInfo struct +func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) { + connInfo := &connectionInfo{} + decConf := &mapstructure.DecoderConfig{ + WeaklyTypedInput: true, + Result: connInfo, + } + dec, err := mapstructure.NewDecoder(decConf) + if err != nil { + return nil, err + } + if err := dec.Decode(s.Ephemeral.ConnInfo); err != nil { + return nil, err + } + 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") { + connInfo.ScriptPath = DefaultScriptPath + } + if connInfo.Timeout != "" { + connInfo.TimeoutVal = safeDuration(connInfo.Timeout, DefaultTimeout) + } else { + connInfo.TimeoutVal = DefaultTimeout + } + return connInfo, nil +} + +// safeDuration returns either the parsed duration or a default value +func safeDuration(dur string, defaultDur time.Duration) time.Duration { + d, err := time.ParseDuration(dur) + if err != nil { + log.Printf("Invalid duration '%s', using default of %s", dur, defaultDur) + return defaultDur + } + return d +} + +func formatDuration(duration time.Duration) string { + h := int(duration.Hours()) + m := int(duration.Minutes()) - (h * 60) + s := int(duration.Seconds()) - (h*3600 + m*60) + + res := "PT" + if h > 0 { + res = fmt.Sprintf("%s%dH", res, h) + } + if m > 0 { + res = fmt.Sprintf("%s%dM", res, m) + } + if s > 0 { + res = fmt.Sprintf("%s%dS", res, s) + } + + return res +} diff --git a/communicator/winrm/provisioner_test.go b/communicator/winrm/provisioner_test.go new file mode 100644 index 000000000000..9a271ae59ef9 --- /dev/null +++ b/communicator/winrm/provisioner_test.go @@ -0,0 +1,103 @@ +package winrm + +import ( + "testing" + + "github.com/hashicorp/terraform/terraform" +) + +func TestProvisioner_connInfo(t *testing.T) { + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "winrm", + "user": "Administrator", + "password": "supersecret", + "host": "127.0.0.1", + "port": "5985", + "https": "true", + "timeout": "30s", + }, + }, + } + + conf, err := parseConnectionInfo(r) + if err != nil { + t.Fatalf("err: %v", err) + } + + if conf.User != "Administrator" { + t.Fatalf("expected: %v: got: %v", "Administrator", conf) + } + if conf.Password != "supersecret" { + t.Fatalf("expected: %v: got: %v", "supersecret", conf) + } + if conf.Host != "127.0.0.1" { + t.Fatalf("expected: %v: got: %v", "127.0.0.1", conf) + } + if conf.Port != 5985 { + t.Fatalf("expected: %v: got: %v", 5985, conf) + } + if conf.HTTPS != true { + t.Fatalf("expected: %v: got: %v", true, conf) + } + if conf.Timeout != "30s" { + t.Fatalf("expected: %v: got: %v", "30s", conf) + } + if conf.ScriptPath != DefaultScriptPath { + t.Fatalf("expected: %v: got: %v", DefaultScriptPath, conf) + } +} + +func TestProvisioner_formatDuration(t *testing.T) { + cases := map[string]struct { + InstanceState *terraform.InstanceState + Result string + }{ + "testSeconds": { + InstanceState: &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "timeout": "90s", + }, + }, + }, + + Result: "PT1M30S", + }, + "testMinutes": { + InstanceState: &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "timeout": "5m", + }, + }, + }, + + Result: "PT5M", + }, + "testHours": { + InstanceState: &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "timeout": "1h", + }, + }, + }, + + Result: "PT1H", + }, + } + + for name, tc := range cases { + conf, err := parseConnectionInfo(tc.InstanceState) + if err != nil { + t.Fatalf("err: %v", err) + } + + result := formatDuration(conf.TimeoutVal) + if result != tc.Result { + t.Fatalf("%s: expected: %s got: %s", name, tc.Result, result) + } + } +} From 907eee24f27ccd17da4554173f0042263d8bd405 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 10 Apr 2015 21:28:28 +0200 Subject: [PATCH 04/10] Updating the docs --- .../provisioners/connection.html.markdown | 51 ++++++++++++++----- .../docs/provisioners/file.html.markdown | 17 +++++-- .../provisioners/remote-exec.html.markdown | 2 +- 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/website/source/docs/provisioners/connection.html.markdown b/website/source/docs/provisioners/connection.html.markdown index 6d289c6dadb7..8012b2c86967 100644 --- a/website/source/docs/provisioners/connection.html.markdown +++ b/website/source/docs/provisioners/connection.html.markdown @@ -3,13 +3,13 @@ layout: "docs" page_title: "Provisioner Connections" sidebar_current: "docs-provisioners-connection" description: |- - Many provisioners require access to the remote resource. For example, a provisioner may need to use ssh to connect to the resource. + Many provisioners require access to the remote resource. For example, a provisioner may need to use SSH or WinRM to connect to the resource. --- # Provisioner Connections Many provisioners require access to the remote resource. For example, -a provisioner may need to use ssh to connect to the resource. +a provisioner may need to use SSH or WinRM to connect to the resource. Terraform uses a number of defaults when connecting to a resource, but these can be overridden using `connection` block in either a `resource` or `provisioner`. @@ -21,7 +21,7 @@ subsequent provisioners connect as a user with more limited permissions. ## Example usage ``` -# Copies the file as the root user using a password +# Copies the file as the root user using SSH provisioner "file" { source = "conf/myapp.conf" destination = "/etc/myapp.conf" @@ -30,28 +30,53 @@ provisioner "file" { password = "${var.root_password}" } } + +# Copies the file as the Administrator user using WinRM +provisioner "file" { + source = "conf/myapp.conf" + destination = "C:/App/myapp.conf" + connection { + type = "winrm" + user = "Administrator" + password = "${var.admin_password}" + } +} ``` ## Argument Reference -The following arguments are supported: +**The following arguments are supported by all connection types:** -* `type` - The connection type that should be used. This defaults to "ssh". The type - of connection supported depends on the provisioner. +* `type` - The connection type that should be used. Valid types are "ssh" and "winrm" + This defaults to "ssh". -* `user` - The user that we should use for the connection. This defaults to "root". +* `user` - The user that we should use for the connection. Defaults to "root" when + using type "ssh" and defaults to "Administrator" when using type "winrm". -* `password` - The password we should use for the connection. +* `password` - The password we should use for the connection. In some cases this is + provided by the provider. + +* `host` - The address of the resource to connect to. This is provided by the provider. + +* `port` - The port to connect to. Defaults to 22 when using type "ssh" and defaults + to 5985 when using type "winrm". + +* `timeout` - The timeout to wait for the connection to become available. This defaults + to 5 minutes. Should be provided as a string like "30s" or "5m". + +* `script_path` - The path used to copy scripts to meant for remote execution. + +**Additional arguments only supported by the "ssh" connection type:** * `key_file` - The SSH key to use for the connection. This takes preference over the - password if provided. + password if provided. * `agent` - Set to true to enable using ssh-agent to authenticate. -* `host` - The address of the resource to connect to. This is provided by the provider. +**Additional arguments only supported by the "winrm" connection type:** -* `port` - The port to connect to. This defaults to 22. +* `https` - Set to true to connect using HTTPS instead of HTTP. -* `timeout` - The timeout to wait for the connection to become available. This defaults - to 5 minutes. Should be provided as a string like "30s" or "5m". +* `insecure` - Set to true to not validate the HTTPS certificate chain. +* `cacert` - The CA certificate to validate against. diff --git a/website/source/docs/provisioners/file.html.markdown b/website/source/docs/provisioners/file.html.markdown index 692ce8e5739c..70a266c3b8dc 100644 --- a/website/source/docs/provisioners/file.html.markdown +++ b/website/source/docs/provisioners/file.html.markdown @@ -9,8 +9,8 @@ description: |- # File Provisioner The `file` provisioner is used to copy files or directories from the machine -executing Terraform to the newly created resource. The `file` provisioner only -supports `ssh` type [connections](/docs/provisioners/connection.html). +executing Terraform to the newly created resource. The `file` provisioner +supports both `ssh` and `winrm` type [connections](/docs/provisioners/connection.html). ## Example usage @@ -29,6 +29,12 @@ resource "aws_instance" "web" { source = "conf/configs.d" destination = "/etc" } + + # Copies all files and folders in apps/app1 to D:/IIS/webapp1 + provisioner "file" { + source = "apps/app1/" + destination = "D:/IIS/webapp1" + } } ``` @@ -47,8 +53,10 @@ The following arguments are supported: The file provisioner is also able to upload a complete directory to the remote machine. When uploading a directory, there are a few important things you should know. -First, the destination directory must already exist. If you need to create it, -use a remote-exec provisioner just prior to the file provisioner in order to create the directory. +First, when using the `ssh` connection type the destination directory must already exist. +If you need to create it, use a remote-exec provisioner just prior to the file provisioner +in order to create the directory. When using the `winrm` connection type the destination +directory will be created for you if it doesn't already exist. Next, the existence of a trailing slash on the source path will determine whether the directory name will be embedded within the destination, or whether the destination will @@ -63,4 +71,3 @@ If the source, however, is `/foo/` (a trailing slash is present), and the destin This behavior was adopted from the standard behavior of rsync. Note that under the covers, rsync may or may not be used. - diff --git a/website/source/docs/provisioners/remote-exec.html.markdown b/website/source/docs/provisioners/remote-exec.html.markdown index 181907b69125..79b7de9c0eb2 100644 --- a/website/source/docs/provisioners/remote-exec.html.markdown +++ b/website/source/docs/provisioners/remote-exec.html.markdown @@ -12,7 +12,7 @@ The `remote-exec` provisioner invokes a script on a remote resource after it is created. This can be used to run a configuration management tool, bootstrap into a cluster, etc. To invoke a local process, see the `local-exec` [provisioner](/docs/provisioners/local-exec.html) instead. The `remote-exec` -provisioner only supports `ssh` type [connections](/docs/provisioners/connection.html). +provisioner supports both `ssh` and `winrm` type [connections](/docs/provisioners/connection.html). ## Example usage From 41748003c00f8c727eb06dd2fce1c091c6747e9a Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 30 Apr 2015 18:02:33 +0200 Subject: [PATCH 05/10] Updated the PR according to the review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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… --- .../remote-exec/resource_provisioner.go | 6 ++-- communicator/ssh/communicator.go | 7 +++- communicator/ssh/communicator_test.go | 30 ++++++++++++++++ communicator/ssh/provisioner.go | 4 ++- communicator/winrm/communicator.go | 7 +++- communicator/winrm/communicator_test.go | 34 +++++++++++++++++++ communicator/winrm/provisioner.go | 24 ++++++++----- 7 files changed, 99 insertions(+), 13 deletions(-) diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go index b3e3d1cfd21e..f0905b4ef96c 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner.go +++ b/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, } diff --git a/communicator/ssh/communicator.go b/communicator/ssh/communicator.go index 8dd629adf7f4..c8534c16da4b 100644 --- a/communicator/ssh/communicator.go +++ b/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 diff --git a/communicator/ssh/communicator_test.go b/communicator/ssh/communicator_test.go index e23ad9625b91..24571f0af53e 100644 --- a/communicator/ssh/communicator_test.go +++ b/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) + } + } +} diff --git a/communicator/ssh/provisioner.go b/communicator/ssh/provisioner.go index affa05625326..12d7048e72c3 100644 --- a/communicator/ssh/provisioner.go +++ b/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 } diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go index b723d7ff3ee2..3080acdd4498 100644 --- a/communicator/winrm/communicator.go +++ b/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 diff --git a/communicator/winrm/communicator_test.go b/communicator/winrm/communicator_test.go index 54f77496131b..0d12c023769d 100644 --- a/communicator/winrm/communicator_test.go +++ b/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) + } + } +} diff --git a/communicator/winrm/provisioner.go b/communicator/winrm/provisioner.go index ec12e4dcd642..59c0ba7dde75 100644 --- a/communicator/winrm/provisioner.go +++ b/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 } From 2689601bc3977e402a3aea2756695b51e8264bd7 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 1 May 2015 17:08:58 +0200 Subject: [PATCH 06/10] And here are the tests... MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pretty nice test coverage this way, covering all WinRM actions/logic by using the winrmtest package. I had to extend/update/fix that package a little here and there, but it now serves a nice purpose for testing WinRM stuff… --- communicator/winrm/communicator_test.go | 118 ++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/communicator/winrm/communicator_test.go b/communicator/winrm/communicator_test.go index 0d12c023769d..ff0e9530981a 100644 --- a/communicator/winrm/communicator_test.go +++ b/communicator/winrm/communicator_test.go @@ -1,10 +1,128 @@ package winrm import ( + "bytes" + "io" "regexp" + "strconv" "testing" + + "github.com/dylanmei/winrmtest" + "github.com/hashicorp/terraform/communicator/remote" + "github.com/hashicorp/terraform/terraform" ) +func newMockWinRMServer(t *testing.T) *winrmtest.Remote { + wrm := winrmtest.NewRemote() + + wrm.CommandFunc( + "echo foo", + "", + func(out, err io.Writer) int { + out.Write([]byte("foo")) + return 0 + }) + + wrm.CommandFunc( + "file-copy", + `^echo c29tZXRoaW5n >> ".*"$`, + func(out, err io.Writer) int { + return 0 + }) + + wrm.CommandFunc( + "file-encode", + `^powershell.exe -EncodedCommand .*$`, + func(out, err io.Writer) int { + return 0 + }) + + wrm.CommandFunc( + "powershell", + "", + func(out, err io.Writer) int { + return 0 + }) + + return wrm +} + +func TestStart(t *testing.T) { + wrm := newMockWinRMServer(t) + defer wrm.Close() + + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "winrm", + "user": "user", + "password": "pass", + "host": wrm.Host, + "port": strconv.Itoa(wrm.Port), + "timeout": "30000s", + }, + }, + } + + //time.Sleep(time.Duration(30000 * time.Second)) + + c, err := New(r) + if err != nil { + t.Fatalf("error creating communicator: %s", err) + } + + var cmd remote.Cmd + stdout := new(bytes.Buffer) + cmd.Command = "echo foo" + cmd.Stdout = stdout + + err = c.Start(&cmd) + if err != nil { + t.Fatalf("error executing remote command: %s", err) + } + cmd.Wait() + + if stdout.String() != "foo" { + t.Fatalf("bad command response: expected %q, got %q", "foo", stdout.String()) + } +} + +func TestUpload(t *testing.T) { + t.Skip() + + wrm := newMockWinRMServer(t) + defer wrm.Close() + + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "winrm", + "user": "user", + "password": "pass", + "host": wrm.Host, + "port": strconv.Itoa(wrm.Port), + "timeout": "30s", + }, + }, + } + + c, err := New(r) + if err != nil { + t.Fatalf("error creating communicator: %s", err) + } + + err = c.Connect(nil) + if err != nil { + t.Fatalf("error connecting communicator: %s", err) + } + defer c.Disconnect() + + err = c.Upload("C:/Temp/terraform.cmd", bytes.NewReader([]byte("something"))) + if err != nil { + t.Fatalf("error uploading file: %s", err) + } +} + func TestScriptPath(t *testing.T) { cases := []struct { Input string From b6660a1abe0eb365c7731fddfb2f54867f213240 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 1 May 2015 18:33:49 +0200 Subject: [PATCH 07/10] Updated test as the winrmtest package was tweaked a little... MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The winrmtest package update is merged now, so this can be merged now as well… --- communicator/winrm/communicator_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/communicator/winrm/communicator_test.go b/communicator/winrm/communicator_test.go index ff0e9530981a..5f71f34dd835 100644 --- a/communicator/winrm/communicator_test.go +++ b/communicator/winrm/communicator_test.go @@ -16,30 +16,26 @@ func newMockWinRMServer(t *testing.T) *winrmtest.Remote { wrm := winrmtest.NewRemote() wrm.CommandFunc( - "echo foo", - "", + winrmtest.MatchText("echo foo"), func(out, err io.Writer) int { out.Write([]byte("foo")) return 0 }) wrm.CommandFunc( - "file-copy", - `^echo c29tZXRoaW5n >> ".*"$`, + winrmtest.MatchPattern(`^echo c29tZXRoaW5n >> ".*"$`), func(out, err io.Writer) int { return 0 }) wrm.CommandFunc( - "file-encode", - `^powershell.exe -EncodedCommand .*$`, + winrmtest.MatchPattern(`^powershell.exe -EncodedCommand .*$`), func(out, err io.Writer) int { return 0 }) wrm.CommandFunc( - "powershell", - "", + winrmtest.MatchText("powershell"), func(out, err io.Writer) int { return 0 }) From a1a1ea5cf98ad2d174ae1ebc9d4895f3e9628108 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 1 May 2015 18:42:23 +0200 Subject: [PATCH 08/10] Removing stray comments/commands --- communicator/winrm/communicator_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/communicator/winrm/communicator_test.go b/communicator/winrm/communicator_test.go index 5f71f34dd835..cf3a94ee8976 100644 --- a/communicator/winrm/communicator_test.go +++ b/communicator/winrm/communicator_test.go @@ -55,13 +55,11 @@ func TestStart(t *testing.T) { "password": "pass", "host": wrm.Host, "port": strconv.Itoa(wrm.Port), - "timeout": "30000s", + "timeout": "30s", }, }, } - //time.Sleep(time.Duration(30000 * time.Second)) - c, err := New(r) if err != nil { t.Fatalf("error creating communicator: %s", err) @@ -84,8 +82,6 @@ func TestStart(t *testing.T) { } func TestUpload(t *testing.T) { - t.Skip() - wrm := newMockWinRMServer(t) defer wrm.Close() From 7f408cf8aaa0769cf5954527b547122a7ec1fc5d Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 1 May 2015 22:26:11 +0200 Subject: [PATCH 09/10] Adding an import needed for the tests This will likely be a temp fix until `make updated eps` is made a little smarter by @phinze :wink: --- communicator/winrm/communicator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go index 3080acdd4498..a4881153a96c 100644 --- a/communicator/winrm/communicator.go +++ b/communicator/winrm/communicator.go @@ -13,6 +13,9 @@ import ( "github.com/hashicorp/terraform/terraform" "github.com/masterzen/winrm/winrm" "github.com/packer-community/winrmcp/winrmcp" + + // This import is a bit strange, but it's needed so `make updatedeps` can see and + _ "github.com/dylanmei/winrmtest" ) // Communicator represents the WinRM communicator @@ -23,7 +26,6 @@ type Communicator struct { } // New creates a new communicator implementation over WinRM. -//func New(endpoint *winrm.Endpoint, user string, password string, timeout time.Duration) (*communicator, error) { func New(s *terraform.InstanceState) (*Communicator, error) { connInfo, err := parseConnectionInfo(s) if err != nil { From e55169b39b8d50ba8f1b0a924b68449b1207d47e Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 1 May 2015 22:28:12 +0200 Subject: [PATCH 10/10] Typo... --- communicator/winrm/communicator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/communicator/winrm/communicator.go b/communicator/winrm/communicator.go index a4881153a96c..ad1d1d30e820 100644 --- a/communicator/winrm/communicator.go +++ b/communicator/winrm/communicator.go @@ -14,7 +14,7 @@ import ( "github.com/masterzen/winrm/winrm" "github.com/packer-community/winrmcp/winrmcp" - // This import is a bit strange, but it's needed so `make updatedeps` can see and + // This import is a bit strange, but it's needed so `make updatedeps` can see and download it _ "github.com/dylanmei/winrmtest" )