From efd4a27c1fa3a29a4c1b53d9d4e087ce95771733 Mon Sep 17 00:00:00 2001 From: Matt Kulka Date: Sat, 1 Mar 2025 16:32:37 +0000 Subject: [PATCH 1/6] [communicator] add `proxy_command` support to connection block --- .../v1.12/ENHANCEMENTS-20250301-165740.yaml | 5 + internal/communicator/shared/shared.go | 4 + internal/communicator/ssh/communicator.go | 188 +++++++++++++++++- .../communicator/ssh/communicator_test.go | 50 +++++ internal/communicator/ssh/provisioner.go | 74 ++++--- internal/communicator/ssh/provisioner_test.go | 57 ++++++ internal/terraform/node_resource_validate.go | 4 + .../resources/provisioners/connection.mdx | 28 +++ 8 files changed, 383 insertions(+), 27 deletions(-) create mode 100644 .changes/v1.12/ENHANCEMENTS-20250301-165740.yaml diff --git a/.changes/v1.12/ENHANCEMENTS-20250301-165740.yaml b/.changes/v1.12/ENHANCEMENTS-20250301-165740.yaml new file mode 100644 index 000000000000..3a35740d9dba --- /dev/null +++ b/.changes/v1.12/ENHANCEMENTS-20250301-165740.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: You can now utilize an SSH proxy command with the `proxy_command` argument in `connection` blocks of resources. +time: 2025-03-01T16:57:40.050317317Z +custom: + Issue: "394" diff --git a/internal/communicator/shared/shared.go b/internal/communicator/shared/shared.go index 68ee53366e60..34a41fc427ef 100644 --- a/internal/communicator/shared/shared.go +++ b/internal/communicator/shared/shared.go @@ -95,6 +95,10 @@ var ConnectionBlockSupersetSchema = &configschema.Block{ Type: cty.String, Optional: true, }, + "proxy_command": { + Type: cty.String, + Optional: true, + }, "bastion_host": { Type: cty.String, Optional: true, diff --git a/internal/communicator/ssh/communicator.go b/internal/communicator/ssh/communicator.go index 1cec681bde9e..d27b4879ff95 100644 --- a/internal/communicator/ssh/communicator.go +++ b/internal/communicator/ssh/communicator.go @@ -15,10 +15,12 @@ import ( "math/rand" "net" "os" + "os/exec" "path/filepath" "strconv" "strings" "sync" + "syscall" "time" "github.com/apparentlymart/go-shquot/shquot" @@ -31,6 +33,9 @@ import ( _ "github.com/hashicorp/terraform/internal/logging" ) +// execCommand is a variable for testing that allows us to mock exec.Command +var execCommand = exec.Command + const ( // DefaultShebang is added at the top of a SSH script file DefaultShebang = "#!/bin/sh\n" @@ -174,7 +179,12 @@ func (c *Communicator) Connect(o provisioners.UIOutput) (err error) { )) } - if c.connInfo.ProxyHost != "" { + if c.connInfo.ProxyCommand != "" { + o.Output(fmt.Sprintf( + "Using configured proxy command: %s", + c.connInfo.ProxyCommand, + )) + } else if c.connInfo.ProxyHost != "" { o.Output(fmt.Sprintf( "Using configured proxy host...\n"+ " ProxyHost: %s\n"+ @@ -329,7 +339,18 @@ func (c *Communicator) Disconnect() error { if c.conn != nil { conn := c.conn c.conn = nil - return conn.Close() + + // Close the connection + err := conn.Close() + if err != nil { + log.Printf("[ERROR] Error closing connection: %s", err) + return err + } + + // Ensure the proxy command is terminated if this is a proxy command connection + if proxyConn, ok := conn.(*proxyCommandConn); ok { + proxyConn.Close() + } } return nil @@ -603,7 +624,7 @@ func (c *Communicator) scpSession(scpCommand string, f func(io.Writer, *bufio.Re if err != nil { if exitErr, ok := err.(*ssh.ExitError); ok { - // Otherwise, we have an ExitErorr, meaning we can just read + // Otherwise, we have an ExitError, meaning we can just read // the exit status log.Printf("[ERROR] %s", exitErr) @@ -897,3 +918,164 @@ func quoteShell(args []string, targetPlatform string) (string, error) { return "", fmt.Errorf("Cannot quote shell command, target platform unknown: %s", targetPlatform) } + +// ProxyCommandConnectFunc is a convenience method for returning a function +// that connects to a host using a proxy command. +func ProxyCommandConnectFunc(proxyCommand, addr string) func() (net.Conn, error) { + return func() (net.Conn, error) { + log.Printf("[DEBUG] Connecting to %s using proxy command: %s", addr, proxyCommand) + + // Replace %h and %p in the proxy command with the host and port + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, fmt.Errorf("Error parsing address: %s", err) + } + + command := strings.Replace(proxyCommand, "%h", host, -1) + command = strings.Replace(command, "%p", port, -1) + + // Split the command into command and args + cmdParts := strings.Fields(command) + if len(cmdParts) == 0 { + return nil, fmt.Errorf("Invalid proxy command: %s", proxyCommand) + } + + // Create a buffer to capture stderr + stderrBuf := new(bytes.Buffer) + + // Create proxy command + cmd := execCommand(cmdParts[0], cmdParts[1:]...) + cmd.Stderr = stderrBuf + + // Set up the command to run in its own process group + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, // Create a new process group + } + + // Start the command with pipes + stdin, err := cmd.StdinPipe() + if err != nil { + return nil, fmt.Errorf("Error creating stdin pipe for proxy command: %s", err) + } + + stdout, err := cmd.StdoutPipe() + if err != nil { + stdin.Close() + return nil, fmt.Errorf("Error creating stdout pipe for proxy command: %s", err) + } + + if err := cmd.Start(); err != nil { + stdin.Close() + stdout.Close() + return nil, fmt.Errorf("Error starting proxy command: %s", err) + } + + // Create a wrapper that manages the command and pipes + conn := &proxyCommandConn{ + cmd: cmd, + stderr: stderrBuf, + stdinPipe: stdin, + stdoutPipe: stdout, + } + + return conn, nil + } +} + +type proxyCommandConn struct { + cmd *exec.Cmd + stderr *bytes.Buffer + stdinPipe io.WriteCloser + stdoutPipe io.ReadCloser + closed bool + mutex sync.Mutex +} + +// Read reads data from the connection (the command's stdout) +func (c *proxyCommandConn) Read(b []byte) (int, error) { + if c.closed { + return 0, io.EOF + } + return c.stdoutPipe.Read(b) +} + +// Write writes data to the connection (the command's stdin) +func (c *proxyCommandConn) Write(b []byte) (int, error) { + if c.closed { + return 0, io.ErrClosedPipe + } + return c.stdinPipe.Write(b) +} + +func (c *proxyCommandConn) Close() error { + c.mutex.Lock() + defer c.mutex.Unlock() + + if c.closed { + return nil + } + c.closed = true + + log.Print("[DEBUG] Closing proxy command connection") + + // Close pipes and hangup + if err := c.stdinPipe.Close(); err != nil { + log.Printf("[ERROR] Error closing stdin pipe: %s", err) + } + if err := c.stdoutPipe.Close(); err != nil { + log.Printf("[ERROR] Error closing stdout pipe: %s", err) + } + + // Send hangup the proxy command and any child processes + if c.cmd.Process != nil { + pgid, err := syscall.Getpgid(c.cmd.Process.Pid) + if err != nil { + log.Printf("[ERROR] Error getting process group ID: %s", err) + // Fall back to killing just the process if we can't get the process group + if err := c.cmd.Process.Signal(syscall.SIGHUP); err != nil { + log.Printf("[ERROR] Error sending SIGHUP to proxy command: %s", err) + } + } else { + // Kill the entire process group + if err := syscall.Kill(-pgid, syscall.SIGHUP); err != nil { + log.Printf("[ERROR] Error sending SIGHUP to process group: %s", err) + } + } + + // Wait for the process to finish to avoid zombie processes + go func() { + if err := c.cmd.Wait(); err != nil { + // This is expected since we're killing the process + log.Printf("[DEBUG] Proxy command exited: %s", err) + } + }() + } + + // If the command failed, log the stderr output + if c.stderr.Len() > 0 { + log.Printf("[ERROR] Proxy command stderr: %s", c.stderr.String()) + } + + return nil +} + +// Required methods to implement net.Conn interface +func (c *proxyCommandConn) LocalAddr() net.Addr { + return &net.UnixAddr{Name: "local", Net: "unix"} +} + +func (c *proxyCommandConn) RemoteAddr() net.Addr { + return &net.UnixAddr{Name: "remote", Net: "unix"} +} + +func (c *proxyCommandConn) SetDeadline(t time.Time) error { + return nil // Not implemented +} + +func (c *proxyCommandConn) SetReadDeadline(t time.Time) error { + return nil // Not implemented +} + +func (c *proxyCommandConn) SetWriteDeadline(t time.Time) error { + return nil // Not implemented +} diff --git a/internal/communicator/ssh/communicator_test.go b/internal/communicator/ssh/communicator_test.go index 77ad0bdc1de3..c8385798418c 100644 --- a/internal/communicator/ssh/communicator_test.go +++ b/internal/communicator/ssh/communicator_test.go @@ -16,6 +16,7 @@ import ( "math/rand" "net" "os" + "os/exec" "path/filepath" "regexp" "strconv" @@ -735,6 +736,55 @@ func TestScriptPath_randSeed(t *testing.T) { } } +func TestProxyCommandPlaceholders(t *testing.T) { + // Instead of using a real command, we'll mock the exec.Command function + // to verify the command string has the placeholders replaced correctly + + // Save the original command function and restore it after the test + origCommand := execCommand + defer func() { execCommand = origCommand }() + + // Create a variable to capture the command and arguments + var capturedCommand string + var capturedArgs []string + + // Mock the exec.Command function + execCommand = func(command string, args ...string) *exec.Cmd { + capturedCommand = command + capturedArgs = args + + // Return a command that will succeed but not do anything + cmd := exec.Command("echo", "test") + return cmd + } + + // Call the function with a proxy command that has placeholders + proxyCommand := "ssh -W %h:%p bastion-host.example.com" + addr := "example.com:2222" + + // Create and call the connect function + connectFunc := ProxyCommandConnectFunc(proxyCommand, addr) + connectFunc() + + // Verify the placeholders were replaced correctly + expectedCommand := "ssh" + expectedArgs := []string{"-W", "example.com:2222", "bastion-host.example.com"} + + if capturedCommand != expectedCommand { + t.Fatalf("Command mismatch. Expected: %q, Got: %q", expectedCommand, capturedCommand) + } + + if len(capturedArgs) != len(expectedArgs) { + t.Fatalf("Argument count mismatch. Expected: %d, Got: %d", len(expectedArgs), len(capturedArgs)) + } + + for i, arg := range expectedArgs { + if capturedArgs[i] != arg { + t.Fatalf("Argument mismatch at index %d. Expected: %q, Got: %q", i, arg, capturedArgs[i]) + } + } +} + var testClientPublicKey = `ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDE6A1c4n+OtEPEFlNKTZf2i03L3NylSYmvmJ8OLmzLuPZmJBJt4G3VZ/60s1aKzwLKrTq20S+ONG4zvnK5zIPoauoNNdUJKbg944hB4OE+HDbrBhk7SH+YWCsCILBoSXwAVdUEic6FWf/SeqBSmTBySHvpuNOw16J+SK6Ardx8k64F2tRkZuC6AmOZijgKa/sQKjWAIVPk34ECM6OLfPc3kKUEfkdpYLvuMfuRMfSTlxn5lFC0b0SovK9aWfNMBH9iXLQkieQ5rXoyzUC7mwgnASgl8cqw1UrToiUuhvneduXBhbQfmC/Upv+tL6dSSk+0DlgVKEHuJmc8s8+/qpdL` func acceptUserPass(goodUser, goodPass string) func(ssh.ConnMetadata, []byte) (*ssh.Permissions, error) { diff --git a/internal/communicator/ssh/provisioner.go b/internal/communicator/ssh/provisioner.go index f2f458684a96..ae802904eef2 100644 --- a/internal/communicator/ssh/provisioner.go +++ b/internal/communicator/ssh/provisioner.go @@ -70,6 +70,7 @@ type connectionInfo struct { ProxyPort uint16 ProxyUserName string ProxyUserPassword string + ProxyCommand string BastionUser string BastionPassword string @@ -151,6 +152,8 @@ func decodeConnInfo(v cty.Value) (*connectionInfo, error) { } case "agent_identity": connInfo.AgentIdentity = v.AsString() + case "proxy_command": + connInfo.ProxyCommand = v.AsString() } } return connInfo, nil @@ -275,37 +278,60 @@ func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) { return nil, err } - var p *proxyInfo + var connectFunc func() (net.Conn, error) + // Check for conflicting connection methods + connectionMethodCount := 0 + if connInfo.ProxyCommand != "" { + connectionMethodCount++ + } if connInfo.ProxyHost != "" { - p = newProxyInfo( - fmt.Sprintf("%s:%d", connInfo.ProxyHost, connInfo.ProxyPort), - connInfo.ProxyScheme, - connInfo.ProxyUserName, - connInfo.ProxyUserPassword, - ) + connectionMethodCount++ + } + if connInfo.BastionHost != "" { + connectionMethodCount++ } - connectFunc := ConnectFunc("tcp", host, p) + if connectionMethodCount > 1 { + log.Printf("[WARN] Multiple connection methods specified (proxy_command, proxy_host, bastion_host). Using proxy_command if specified, then proxy_host, then bastion_host.") + } - var bastionConf *ssh.ClientConfig - if connInfo.BastionHost != "" { - bastionHost := fmt.Sprintf("%s:%d", connInfo.BastionHost, connInfo.BastionPort) - - bastionConf, err = buildSSHClientConfig(sshClientConfigOpts{ - user: connInfo.BastionUser, - host: bastionHost, - privateKey: connInfo.BastionPrivateKey, - password: connInfo.BastionPassword, - hostKey: connInfo.HostKey, - certificate: connInfo.BastionCertificate, - sshAgent: sshAgent, - }) - if err != nil { - return nil, err + // If a proxy command is specified, use it + if connInfo.ProxyCommand != "" { + connectFunc = ProxyCommandConnectFunc(connInfo.ProxyCommand, host) + } else { + // Otherwise, use the standard connection methods + var p *proxyInfo + + if connInfo.ProxyHost != "" { + p = newProxyInfo( + fmt.Sprintf("%s:%d", connInfo.ProxyHost, connInfo.ProxyPort), + connInfo.ProxyScheme, + connInfo.ProxyUserName, + connInfo.ProxyUserPassword, + ) } - connectFunc = BastionConnectFunc("tcp", bastionHost, bastionConf, "tcp", host, p) + connectFunc = ConnectFunc("tcp", host, p) + + if connInfo.BastionHost != "" { + bastionHost := fmt.Sprintf("%s:%d", connInfo.BastionHost, connInfo.BastionPort) + + bastionConf, err := buildSSHClientConfig(sshClientConfigOpts{ + user: connInfo.BastionUser, + host: bastionHost, + privateKey: connInfo.BastionPrivateKey, + password: connInfo.BastionPassword, + hostKey: connInfo.HostKey, + certificate: connInfo.BastionCertificate, + sshAgent: sshAgent, + }) + if err != nil { + return nil, err + } + + connectFunc = BastionConnectFunc("tcp", bastionHost, bastionConf, "tcp", host, p) + } } config := &sshConfig{ diff --git a/internal/communicator/ssh/provisioner_test.go b/internal/communicator/ssh/provisioner_test.go index 47daf372560b..4423eb1f0a5e 100644 --- a/internal/communicator/ssh/provisioner_test.go +++ b/internal/communicator/ssh/provisioner_test.go @@ -186,6 +186,32 @@ func TestProvisioner_connInfoProxy(t *testing.T) { } } +func TestProvisioner_connInfoProxyCommand(t *testing.T) { + v := cty.ObjectVal(map[string]cty.Value{ + "type": cty.StringVal("ssh"), + "user": cty.StringVal("root"), + "password": cty.StringVal("supersecret"), + "private_key": cty.StringVal("someprivatekeycontents"), + "host": cty.StringVal("example.com"), + "port": cty.StringVal("22"), + "timeout": cty.StringVal("30s"), + "proxy_command": cty.StringVal("ssh -W %h:%p bastion-host.example.com"), + }) + + conf, err := parseConnectionInfo(v) + if err != nil { + t.Fatalf("err: %v", err) + } + + if conf.Host != "example.com" { + t.Fatalf("bad: %v", conf) + } + + if conf.ProxyCommand != "ssh -W %h:%p bastion-host.example.com" { + t.Fatalf("bad: %v", conf) + } +} + func TestProvisioner_stringBastionPort(t *testing.T) { v := cty.ObjectVal(map[string]cty.Value{ "type": cty.StringVal("ssh"), @@ -227,3 +253,34 @@ func TestProvisioner_invalidPortNumber(t *testing.T) { t.Errorf("unexpected error\n got: %s\nwant: %s", got, want) } } + +func TestProvisioner_multipleConnectionMethods(t *testing.T) { + // Create a connection info with all three connection methods specified + v := cty.ObjectVal(map[string]cty.Value{ + "type": cty.StringVal("ssh"), + "user": cty.StringVal("root"), + "password": cty.StringVal("supersecret"), + "private_key": cty.StringVal("someprivatekeycontents"), + "host": cty.StringVal("example.com"), + "port": cty.StringVal("22"), + "proxy_command": cty.StringVal("ssh -W %h:%p bastion-host.example.com"), + "proxy_host": cty.StringVal("proxy.example.com"), + "bastion_host": cty.StringVal("bastion.example.com"), + }) + + conf, err := parseConnectionInfo(v) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Verify all three connection methods are parsed correctly + if conf.ProxyCommand != "ssh -W %h:%p bastion-host.example.com" { + t.Fatalf("bad proxy_command: %v", conf.ProxyCommand) + } + if conf.ProxyHost != "proxy.example.com" { + t.Fatalf("bad proxy_host: %v", conf.ProxyHost) + } + if conf.BastionHost != "bastion.example.com" { + t.Fatalf("bad bastion_host: %v", conf.BastionHost) + } +} diff --git a/internal/terraform/node_resource_validate.go b/internal/terraform/node_resource_validate.go index f0dad0c2b5ba..e865c66bd567 100644 --- a/internal/terraform/node_resource_validate.go +++ b/internal/terraform/node_resource_validate.go @@ -231,6 +231,10 @@ var connectionBlockSupersetSchema = &configschema.Block{ Type: cty.String, Optional: true, }, + "proxy_command": { + Type: cty.String, + Optional: true, + }, "bastion_host": { Type: cty.String, Optional: true, diff --git a/website/docs/language/resources/provisioners/connection.mdx b/website/docs/language/resources/provisioners/connection.mdx index 333b58e4c164..9163d19acc42 100644 --- a/website/docs/language/resources/provisioners/connection.mdx +++ b/website/docs/language/resources/provisioners/connection.mdx @@ -137,6 +137,34 @@ The `ssh` connection also supports the following fields to facilitate connection | `proxy_port` | The port to use connect to the proxy host. | | | `proxy_user_name` | The username to use connect to the private proxy host. This argument should be specified only if authentication is required for the HTTP Proxy server. | | | `proxy_user_password` | The password to use connect to the private proxy host. This argument should be specified only if authentication is required for the HTTP Proxy server. | | +| `proxy_command` | A command to execute that will pipe stdin and stdout through to establish the connection. This is useful for connecting through custom networking setups. You can use `%h` and `%p` placeholders for the destination host and port. For example: `ssh -W %h:%p bastion-host.example.com`. | | + +-> **Note:** You should only specify one connection method at a time. If multiple connection methods are specified (proxy_command, proxy_host, bastion_host), Terraform will use them in the following order of precedence: proxy_command, proxy_host, bastion_host. + +### Example using proxy_command + +```hcl +resource "aws_instance" "example" { + # ... instance configuration ... + + # Use a provisioner with proxy_command + provisioner "remote-exec" { + inline = [ + "echo 'Hello from the remote instance'", + ] + + connection { + type = "ssh" + user = "ubuntu" + host = self.private_ip + private_key = file("~/.ssh/id_rsa") + + # Use a proxy command to connect to a proxy through nc + proxy_command = "nc -X connect -x proxy.example.com:8080 %h %p" + } + } +} +``` ## How Provisioners Execute Remote Scripts From 6215cc93ed846616d01d3455ff924fa2745ee111 Mon Sep 17 00:00:00 2001 From: Matt Kulka Date: Wed, 26 Mar 2025 11:56:24 +0000 Subject: [PATCH 2/6] address feedback, remove template string and process management --- internal/communicator/ssh/communicator.go | 68 +++++++------------ .../communicator/ssh/communicator_test.go | 50 -------------- internal/communicator/ssh/provisioner_test.go | 8 +-- .../resources/provisioners/connection.mdx | 4 +- 4 files changed, 29 insertions(+), 101 deletions(-) diff --git a/internal/communicator/ssh/communicator.go b/internal/communicator/ssh/communicator.go index d27b4879ff95..5b80f3dc0fb9 100644 --- a/internal/communicator/ssh/communicator.go +++ b/internal/communicator/ssh/communicator.go @@ -33,9 +33,6 @@ import ( _ "github.com/hashicorp/terraform/internal/logging" ) -// execCommand is a variable for testing that allows us to mock exec.Command -var execCommand = exec.Command - const ( // DefaultShebang is added at the top of a SSH script file DefaultShebang = "#!/bin/sh\n" @@ -925,17 +922,8 @@ func ProxyCommandConnectFunc(proxyCommand, addr string) func() (net.Conn, error) return func() (net.Conn, error) { log.Printf("[DEBUG] Connecting to %s using proxy command: %s", addr, proxyCommand) - // Replace %h and %p in the proxy command with the host and port - host, port, err := net.SplitHostPort(addr) - if err != nil { - return nil, fmt.Errorf("Error parsing address: %s", err) - } - - command := strings.Replace(proxyCommand, "%h", host, -1) - command = strings.Replace(command, "%p", port, -1) - // Split the command into command and args - cmdParts := strings.Fields(command) + cmdParts := strings.Fields(proxyCommand) if len(cmdParts) == 0 { return nil, fmt.Errorf("Invalid proxy command: %s", proxyCommand) } @@ -943,8 +931,11 @@ func ProxyCommandConnectFunc(proxyCommand, addr string) func() (net.Conn, error) // Create a buffer to capture stderr stderrBuf := new(bytes.Buffer) - // Create proxy command - cmd := execCommand(cmdParts[0], cmdParts[1:]...) + // Create context for command lifecycle management + ctx, cancel := context.WithCancel(context.Background()) + + // Create proxy command with context + cmd := exec.CommandContext(ctx, cmdParts[0], cmdParts[1:]...) cmd.Stderr = stderrBuf // Set up the command to run in its own process group @@ -955,18 +946,20 @@ func ProxyCommandConnectFunc(proxyCommand, addr string) func() (net.Conn, error) // Start the command with pipes stdin, err := cmd.StdinPipe() if err != nil { + cancel() return nil, fmt.Errorf("Error creating stdin pipe for proxy command: %s", err) } stdout, err := cmd.StdoutPipe() if err != nil { stdin.Close() + cancel() return nil, fmt.Errorf("Error creating stdout pipe for proxy command: %s", err) } if err := cmd.Start(); err != nil { stdin.Close() - stdout.Close() + cancel() return nil, fmt.Errorf("Error starting proxy command: %s", err) } @@ -976,6 +969,8 @@ func ProxyCommandConnectFunc(proxyCommand, addr string) func() (net.Conn, error) stderr: stderrBuf, stdinPipe: stdin, stdoutPipe: stdout, + ctx: ctx, + cancel: cancel, } return conn, nil @@ -989,21 +984,31 @@ type proxyCommandConn struct { stdoutPipe io.ReadCloser closed bool mutex sync.Mutex + ctx context.Context + cancel context.CancelFunc } // Read reads data from the connection (the command's stdout) func (c *proxyCommandConn) Read(b []byte) (int, error) { + c.mutex.Lock() if c.closed { + c.mutex.Unlock() return 0, io.EOF } + c.mutex.Unlock() + return c.stdoutPipe.Read(b) } // Write writes data to the connection (the command's stdin) func (c *proxyCommandConn) Write(b []byte) (int, error) { + c.mutex.Lock() if c.closed { + c.mutex.Unlock() return 0, io.ErrClosedPipe } + c.mutex.Unlock() + return c.stdinPipe.Write(b) } @@ -1017,39 +1022,12 @@ func (c *proxyCommandConn) Close() error { c.closed = true log.Print("[DEBUG] Closing proxy command connection") - - // Close pipes and hangup if err := c.stdinPipe.Close(); err != nil { log.Printf("[ERROR] Error closing stdin pipe: %s", err) } - if err := c.stdoutPipe.Close(); err != nil { - log.Printf("[ERROR] Error closing stdout pipe: %s", err) - } - // Send hangup the proxy command and any child processes - if c.cmd.Process != nil { - pgid, err := syscall.Getpgid(c.cmd.Process.Pid) - if err != nil { - log.Printf("[ERROR] Error getting process group ID: %s", err) - // Fall back to killing just the process if we can't get the process group - if err := c.cmd.Process.Signal(syscall.SIGHUP); err != nil { - log.Printf("[ERROR] Error sending SIGHUP to proxy command: %s", err) - } - } else { - // Kill the entire process group - if err := syscall.Kill(-pgid, syscall.SIGHUP); err != nil { - log.Printf("[ERROR] Error sending SIGHUP to process group: %s", err) - } - } - - // Wait for the process to finish to avoid zombie processes - go func() { - if err := c.cmd.Wait(); err != nil { - // This is expected since we're killing the process - log.Printf("[DEBUG] Proxy command exited: %s", err) - } - }() - } + // Cancel the context to signal the command to terminate + c.cancel() // If the command failed, log the stderr output if c.stderr.Len() > 0 { diff --git a/internal/communicator/ssh/communicator_test.go b/internal/communicator/ssh/communicator_test.go index c8385798418c..77ad0bdc1de3 100644 --- a/internal/communicator/ssh/communicator_test.go +++ b/internal/communicator/ssh/communicator_test.go @@ -16,7 +16,6 @@ import ( "math/rand" "net" "os" - "os/exec" "path/filepath" "regexp" "strconv" @@ -736,55 +735,6 @@ func TestScriptPath_randSeed(t *testing.T) { } } -func TestProxyCommandPlaceholders(t *testing.T) { - // Instead of using a real command, we'll mock the exec.Command function - // to verify the command string has the placeholders replaced correctly - - // Save the original command function and restore it after the test - origCommand := execCommand - defer func() { execCommand = origCommand }() - - // Create a variable to capture the command and arguments - var capturedCommand string - var capturedArgs []string - - // Mock the exec.Command function - execCommand = func(command string, args ...string) *exec.Cmd { - capturedCommand = command - capturedArgs = args - - // Return a command that will succeed but not do anything - cmd := exec.Command("echo", "test") - return cmd - } - - // Call the function with a proxy command that has placeholders - proxyCommand := "ssh -W %h:%p bastion-host.example.com" - addr := "example.com:2222" - - // Create and call the connect function - connectFunc := ProxyCommandConnectFunc(proxyCommand, addr) - connectFunc() - - // Verify the placeholders were replaced correctly - expectedCommand := "ssh" - expectedArgs := []string{"-W", "example.com:2222", "bastion-host.example.com"} - - if capturedCommand != expectedCommand { - t.Fatalf("Command mismatch. Expected: %q, Got: %q", expectedCommand, capturedCommand) - } - - if len(capturedArgs) != len(expectedArgs) { - t.Fatalf("Argument count mismatch. Expected: %d, Got: %d", len(expectedArgs), len(capturedArgs)) - } - - for i, arg := range expectedArgs { - if capturedArgs[i] != arg { - t.Fatalf("Argument mismatch at index %d. Expected: %q, Got: %q", i, arg, capturedArgs[i]) - } - } -} - var testClientPublicKey = `ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDE6A1c4n+OtEPEFlNKTZf2i03L3NylSYmvmJ8OLmzLuPZmJBJt4G3VZ/60s1aKzwLKrTq20S+ONG4zvnK5zIPoauoNNdUJKbg944hB4OE+HDbrBhk7SH+YWCsCILBoSXwAVdUEic6FWf/SeqBSmTBySHvpuNOw16J+SK6Ardx8k64F2tRkZuC6AmOZijgKa/sQKjWAIVPk34ECM6OLfPc3kKUEfkdpYLvuMfuRMfSTlxn5lFC0b0SovK9aWfNMBH9iXLQkieQ5rXoyzUC7mwgnASgl8cqw1UrToiUuhvneduXBhbQfmC/Upv+tL6dSSk+0DlgVKEHuJmc8s8+/qpdL` func acceptUserPass(goodUser, goodPass string) func(ssh.ConnMetadata, []byte) (*ssh.Permissions, error) { diff --git a/internal/communicator/ssh/provisioner_test.go b/internal/communicator/ssh/provisioner_test.go index 4423eb1f0a5e..761731fc0d69 100644 --- a/internal/communicator/ssh/provisioner_test.go +++ b/internal/communicator/ssh/provisioner_test.go @@ -195,7 +195,7 @@ func TestProvisioner_connInfoProxyCommand(t *testing.T) { "host": cty.StringVal("example.com"), "port": cty.StringVal("22"), "timeout": cty.StringVal("30s"), - "proxy_command": cty.StringVal("ssh -W %h:%p bastion-host.example.com"), + "proxy_command": cty.StringVal("ssh -W example.com:2222 bastion-host.example.com"), }) conf, err := parseConnectionInfo(v) @@ -207,7 +207,7 @@ func TestProvisioner_connInfoProxyCommand(t *testing.T) { t.Fatalf("bad: %v", conf) } - if conf.ProxyCommand != "ssh -W %h:%p bastion-host.example.com" { + if conf.ProxyCommand != "ssh -W example.com:2222 bastion-host.example.com" { t.Fatalf("bad: %v", conf) } } @@ -263,7 +263,7 @@ func TestProvisioner_multipleConnectionMethods(t *testing.T) { "private_key": cty.StringVal("someprivatekeycontents"), "host": cty.StringVal("example.com"), "port": cty.StringVal("22"), - "proxy_command": cty.StringVal("ssh -W %h:%p bastion-host.example.com"), + "proxy_command": cty.StringVal("ssh -W example.com:2222 bastion-host.example.com"), "proxy_host": cty.StringVal("proxy.example.com"), "bastion_host": cty.StringVal("bastion.example.com"), }) @@ -274,7 +274,7 @@ func TestProvisioner_multipleConnectionMethods(t *testing.T) { } // Verify all three connection methods are parsed correctly - if conf.ProxyCommand != "ssh -W %h:%p bastion-host.example.com" { + if conf.ProxyCommand != "ssh -W example.com:2222 bastion-host.example.com" { t.Fatalf("bad proxy_command: %v", conf.ProxyCommand) } if conf.ProxyHost != "proxy.example.com" { diff --git a/website/docs/language/resources/provisioners/connection.mdx b/website/docs/language/resources/provisioners/connection.mdx index 9163d19acc42..04125f439704 100644 --- a/website/docs/language/resources/provisioners/connection.mdx +++ b/website/docs/language/resources/provisioners/connection.mdx @@ -137,7 +137,7 @@ The `ssh` connection also supports the following fields to facilitate connection | `proxy_port` | The port to use connect to the proxy host. | | | `proxy_user_name` | The username to use connect to the private proxy host. This argument should be specified only if authentication is required for the HTTP Proxy server. | | | `proxy_user_password` | The password to use connect to the private proxy host. This argument should be specified only if authentication is required for the HTTP Proxy server. | | -| `proxy_command` | A command to execute that will pipe stdin and stdout through to establish the connection. This is useful for connecting through custom networking setups. You can use `%h` and `%p` placeholders for the destination host and port. For example: `ssh -W %h:%p bastion-host.example.com`. | | +| `proxy_command` | A command to execute that will pipe stdin and stdout through to establish the connection. This is useful for connecting through custom networking setups. | | -> **Note:** You should only specify one connection method at a time. If multiple connection methods are specified (proxy_command, proxy_host, bastion_host), Terraform will use them in the following order of precedence: proxy_command, proxy_host, bastion_host. @@ -160,7 +160,7 @@ resource "aws_instance" "example" { private_key = file("~/.ssh/id_rsa") # Use a proxy command to connect to a proxy through nc - proxy_command = "nc -X connect -x proxy.example.com:8080 %h %p" + proxy_command = "nc -X connect -x proxy.example.com:8080 ${self.private_ip} 22" } } } From e57afc49436a5771cc3972ede4aa3771b7a2dca8 Mon Sep 17 00:00:00 2001 From: Matt Kulka Date: Wed, 26 Mar 2025 12:14:57 +0000 Subject: [PATCH 3/6] remove process group --- internal/communicator/ssh/communicator.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/communicator/ssh/communicator.go b/internal/communicator/ssh/communicator.go index 5b80f3dc0fb9..8fdb8d83828d 100644 --- a/internal/communicator/ssh/communicator.go +++ b/internal/communicator/ssh/communicator.go @@ -20,7 +20,6 @@ import ( "strconv" "strings" "sync" - "syscall" "time" "github.com/apparentlymart/go-shquot/shquot" @@ -938,11 +937,6 @@ func ProxyCommandConnectFunc(proxyCommand, addr string) func() (net.Conn, error) cmd := exec.CommandContext(ctx, cmdParts[0], cmdParts[1:]...) cmd.Stderr = stderrBuf - // Set up the command to run in its own process group - cmd.SysProcAttr = &syscall.SysProcAttr{ - Setpgid: true, // Create a new process group - } - // Start the command with pipes stdin, err := cmd.StdinPipe() if err != nil { From f30e4a0f7cbb18efcbf409cc9a8f9f36900bdba8 Mon Sep 17 00:00:00 2001 From: Matt Kulka Date: Thu, 17 Apr 2025 08:04:05 -0400 Subject: [PATCH 4/6] rebase & feedback --- internal/communicator/ssh/communicator.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/internal/communicator/ssh/communicator.go b/internal/communicator/ssh/communicator.go index 8fdb8d83828d..b028e19726fe 100644 --- a/internal/communicator/ssh/communicator.go +++ b/internal/communicator/ssh/communicator.go @@ -996,13 +996,6 @@ func (c *proxyCommandConn) Read(b []byte) (int, error) { // Write writes data to the connection (the command's stdin) func (c *proxyCommandConn) Write(b []byte) (int, error) { - c.mutex.Lock() - if c.closed { - c.mutex.Unlock() - return 0, io.ErrClosedPipe - } - c.mutex.Unlock() - return c.stdinPipe.Write(b) } @@ -1022,6 +1015,7 @@ func (c *proxyCommandConn) Close() error { // Cancel the context to signal the command to terminate c.cancel() + c.cmd.Wait() // If the command failed, log the stderr output if c.stderr.Len() > 0 { From 94f5a72cc710b15f6d2af36657aa6769d335a681 Mon Sep 17 00:00:00 2001 From: Matt Kulka Date: Thu, 17 Apr 2025 08:06:45 -0400 Subject: [PATCH 5/6] move changelog to v1.13 --- .changes/{v1.12 => v1.13}/ENHANCEMENTS-20250301-165740.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changes/{v1.12 => v1.13}/ENHANCEMENTS-20250301-165740.yaml (100%) diff --git a/.changes/v1.12/ENHANCEMENTS-20250301-165740.yaml b/.changes/v1.13/ENHANCEMENTS-20250301-165740.yaml similarity index 100% rename from .changes/v1.12/ENHANCEMENTS-20250301-165740.yaml rename to .changes/v1.13/ENHANCEMENTS-20250301-165740.yaml From f2c5affa6b23917c51af2065b31dfd5f01b9bcd5 Mon Sep 17 00:00:00 2001 From: Matt Kulka Date: Thu, 17 Apr 2025 18:25:54 -0400 Subject: [PATCH 6/6] feedback --- internal/communicator/ssh/communicator.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/communicator/ssh/communicator.go b/internal/communicator/ssh/communicator.go index b028e19726fe..fc97d3fad043 100644 --- a/internal/communicator/ssh/communicator.go +++ b/internal/communicator/ssh/communicator.go @@ -342,11 +342,6 @@ func (c *Communicator) Disconnect() error { log.Printf("[ERROR] Error closing connection: %s", err) return err } - - // Ensure the proxy command is terminated if this is a proxy command connection - if proxyConn, ok := conn.(*proxyCommandConn); ok { - proxyConn.Close() - } } return nil