chore: fix tests on darwin for SSH unix forwarding

coder · deansheather · Jan 6, 2023 · Dec 20, 2022 · Dec 20, 2022 · Jan 4, 2023
commit a0f70de92002afad03e3bf48c8146ad175703c5a
diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -410,7 +410,7 @@ func TestAgent_UnixLocalForwarding(t *testing.T) {
 		t.Skip("unix domain sockets are not fully supported on Windows")
 	}
 
-	tmpdir := t.TempDir()
+	tmpdir := tempDirUnixSocket(t)
 	remoteSocketPath := filepath.Join(tmpdir, "remote-socket")
 	localSocketPath := filepath.Join(tmpdir, "local-socket")
 
@@ -467,7 +467,7 @@ func TestAgent_UnixRemoteForwarding(t *testing.T) {
 		t.Skip("unix domain sockets are not fully supported on Windows")
 	}
 
-	tmpdir := t.TempDir()
+	tmpdir := tempDirUnixSocket(t)
 	remoteSocketPath := filepath.Join(tmpdir, "remote-socket")
 	localSocketPath := filepath.Join(tmpdir, "local-socket")
 
@@ -1135,3 +1135,25 @@ func (*client) PostWorkspaceAgentAppHealth(_ context.Context, _ codersdk.PostWor
 func (*client) PostWorkspaceAgentVersion(_ context.Context, _ string) error {
 	return nil
 }
+
+// tempDirUnixSocket returns a temporary directory that can safely hold unix
+// sockets (probably).
+//
+// During tests on darwin we hit the max path length limit for unix sockets
+// pretty easily in the default location, so this function uses /tmp instead to
+// get shorter paths.
+func tempDirUnixSocket(t *testing.T) string {
+	if runtime.GOOS == "darwin" {
+		testName := strings.ReplaceAll(t.Name(), "/", "_")
+		dir, err := os.MkdirTemp("/tmp", fmt.Sprintf("coder-test-%s-", testName))
+		require.NoError(t, err, "create temp dir for gpg test")
+
+		t.Cleanup(func() {
+			err := os.RemoveAll(dir)
+			assert.NoError(t, err, "remove temp dir", dir)
+		})
+		return dir
+	}
+
+	return t.TempDir()
+}
diff --git a/cli/ssh.go b/cli/ssh.go
@@ -239,7 +239,7 @@ func ssh() *cobra.Command {
 	cliflag.BoolVarP(cmd.Flags(), &shuffle, "shuffle", "", "CODER_SSH_SHUFFLE", false, "Specifies whether to choose a random workspace")
 	_ = cmd.Flags().MarkHidden("shuffle")
 	cliflag.BoolVarP(cmd.Flags(), &forwardAgent, "forward-agent", "A", "CODER_SSH_FORWARD_AGENT", false, "Specifies whether to forward the SSH agent specified in $SSH_AUTH_SOCK")
-	cliflag.BoolVarP(cmd.Flags(), &forwardGPG, "forward-gpg", "G", "CODER_SSH_FORWARD_GPG", false, "Specifies whether to forward the GPG agent. Unsupported on Windows workspaces, but supports all clients. Requires gnupg (gpg, gpgconf) on both the client and workspace. The GPG agent must already be running and will not be started for you.")
+	cliflag.BoolVarP(cmd.Flags(), &forwardGPG, "forward-gpg", "G", "CODER_SSH_FORWARD_GPG", false, "Specifies whether to forward the GPG agent. Unsupported on Windows workspaces, but supports all clients. Requires gnupg (gpg, gpgconf) on both the client and workspace. The GPG agent must already be running locally and will not be started for you. If a GPG agent is already running in the workspace, it will be attempted to be killed. It is recommended that you set GPG_TTY, TTY or SSH_TTY to $(tty) beforehand.")
 	cliflag.StringVarP(cmd.Flags(), &identityAgent, "identity-agent", "", "CODER_SSH_IDENTITY_AGENT", "", "Specifies which identity agent to use (overrides $SSH_AUTH_SOCK), forward agent must also be enabled")
 	cliflag.DurationVarP(cmd.Flags(), &wsPollInterval, "workspace-poll-interval", "", "CODER_WORKSPACE_POLL_INTERVAL", workspacePollInterval, "Specifies how often to poll for workspace automated shutdown.")
 	return cmd
@@ -448,6 +448,13 @@ func uploadGPGKeys(ctx context.Context, sshClient *gossh.Client) error {
 set -eux
 agent_socket=$(gpgconf --list-dir agent-socket)
 echo "$agent_socket"
+if [ -S "$agent_socket" ]; then
+  echo "agent socket exists, attempting to kill it" >&2
+  gpgconf --kill gpg-agent
+  rm -f "$agent_socket"
+  sleep 2
+fi
+
 test ! -S "$agent_socket"
 `)
 	agentSocket := strings.TrimSpace(string(agentSocketBytes))

diff --git a/cli/ssh_test.go b/cli/ssh_test.go
@@ -7,8 +7,10 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"errors"
+	"fmt"
 	"io"
 	"net"
+	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
@@ -230,7 +232,7 @@ func TestSSH(t *testing.T) {
 		})
 
 		// Start up ssh agent listening on unix socket.
-		tmpdir := t.TempDir()
+		tmpdir := tempDirUnixSocket(t)
 		agentSock := filepath.Join(tmpdir, "agent.sock")
 		l, err := net.Listen("unix", agentSock)
 		require.NoError(t, err)
@@ -370,7 +372,7 @@ p7KeSZdlk47pMBGOfnvEmoQ=
 		}
 
 		// Setup GPG home directory on the "client".
-		gnupgHomeClient := t.TempDir()
+		gnupgHomeClient := tempDirUnixSocket(t)
 		t.Setenv("GNUPGHOME", gnupgHomeClient)
 
 		// Get the agent extra socket path.
@@ -426,7 +428,7 @@ Expire-Date: 0
 		}()
 
 		// Get the agent socket path in the "workspace".
-		gnupgHomeWorkspace := t.TempDir()
+		gnupgHomeWorkspace := tempDirUnixSocket(t)
 
 		stdout = bytes.NewBuffer(nil)
 		stderr = bytes.NewBuffer(nil)
@@ -473,9 +475,8 @@ Expire-Date: 0
 
 		// Check the GNUPGHOME was correctly inherited via shell.
 		pty.WriteLine("env && echo env-''-command-done")
-		pty.ExpectMatch("GNUPGHOME=")
-		require.Equal(t, pty.ReadLine(), gnupgHomeWorkspace)
-		pty.ExpectMatch("env--command-done")
+		match := pty.ExpectMatch("env--command-done")
+		require.Contains(t, match, "GNUPGHOME="+gnupgHomeWorkspace, match)
 
 		// Get the agent extra socket path in the "workspace" via shell.
 		pty.WriteLine("gpgconf --list-dir agent-socket && echo gpgconf-''-agentsocket-command-done")
@@ -574,3 +575,25 @@ func (*stdioConn) SetReadDeadline(_ time.Time) error {
 func (*stdioConn) SetWriteDeadline(_ time.Time) error {
 	return nil
 }
+
+// tempDirUnixSocket returns a temporary directory that can safely hold unix
+// sockets (probably).
+//
+// During tests on darwin we hit the max path length limit for unix sockets
+// pretty easily in the default location, so this function uses /tmp instead to
+// get shorter paths.
+func tempDirUnixSocket(t *testing.T) string {
+	if runtime.GOOS == "darwin" {
+		testName := strings.ReplaceAll(t.Name(), "/", "_")
+		dir, err := os.MkdirTemp("/tmp", fmt.Sprintf("coder-test-%s-", testName))
+		require.NoError(t, err, "create temp dir for gpg test")
+
+		t.Cleanup(func() {
+			err := os.RemoveAll(dir)
+			assert.NoError(t, err, "remove temp dir", dir)
+		})
+		return dir
+	}
+
+	return t.TempDir()
+}
diff --git a/cli/testdata/coder_ssh_--help.golden b/cli/testdata/coder_ssh_--help.golden
@@ -11,7 +11,11 @@ Flags:
                                            Unsupported on Windows workspaces, but supports all
                                            clients. Requires gnupg (gpg, gpgconf) on both the
                                            client and workspace. The GPG agent must already be
-                                           running and will not be started for you.
+                                           running locally and will not be started for you. If
+                                           a GPG agent is already running in the workspace, it
+                                           will be attempted to be killed. It is recommended
+                                           that you set GPG_TTY, TTY or SSH_TTY to $(tty)
+                                           beforehand.
                                            Consumes $CODER_SSH_FORWARD_GPG
   -h, --help                               help for ssh
       --identity-agent string              Specifies which identity agent to use (overrides

diff --git a/scripts/develop.sh b/scripts/develop.sh
@@ -121,7 +121,7 @@ fatal() {
 	trap 'fatal "Script encountered an error"' ERR
 
 	cdroot
-	start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable
+	start_cmd API "" "${CODER_DEV_SHIM}" server --http-address 0.0.0.0:3000 --swagger-enable --access-url "http://127.0.0.1:3000"
 
 	echo '== Waiting for Coder to become ready'
 	# Start the timeout in the background so interrupting this script