feat: support cloning over SSH via private key auth #170

johnstcn · 2024-05-02T14:57:05Z

Relates to #31

Currently running into issues with testing end-to-end flow.
However, I've tested with a private git repo:

$ docker run -it --rm -e GIT_URL="ssh://git@github.com/johnstcn/test-private-devcontainer" -e INIT_SCRIPT=bash -e GIT_SSH_PRIVATE_KEY_PATH=/.ssh/id_ed25519 -v /Users/cian/.ssh/id_ed25519:/.ssh/id_ed25519 envbuilder:latest
envbuilder - Build development environments from repositories in a container
ssh keyscan: github.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg=
#1: 📦 Cloning ssh://git@github.com/johnstcn/test-private-devcontainer to /workspaces/test-private-devcontainer...
#1: Enumerating objects: 6, done.
#1: Counting objects:  16% (1/6)
#1: Counting objects:  33% (2/6)
#1: Counting objects:  50% (3/6)
#1: Counting objects:  66% (4/6)
#1: Counting obj
#1: ects:  83% (5/6)
#1: Counting objects: 100% (6/6)
#1: Counting objects: 100% (6/6), done.
#1: Compressing objects:  16% (1/6)
#1: Compressing objects:  33% (2/6)
#1: Compressing objects:  50% (3/6)
#1: Compressing objects:  66% (4/6)
#1: Compressing objects:  83% (5/6)
#1: Compressing objects: 100% (6/6)
#1: Compressing objects: 100% (6/6), done.
#1: Total 6 (delta 0), reused 6 (delta 0), pack-reused 0
#1: 📦 Cloned repository! [1.434724587s]

One potential issue for which I don't have a solution is that passing an SSH key in as a volume will result in the file being available in the resulting workspace. This may not be desirable; an alternative would be to pass in the SSH key as a base64-encoded file (similar to known_hosts) and clean it up after the build completes.

After this is merged, I'd like to do a follow-up PR to refactor some of the envbuilder setup code.

mafredri

an alternative would be to pass in the SSH key as a base64-encoded file (similar to known_hosts) and clean it up after the build completes.

I think this would be a solid first step at least. We can always iterate/add support for other use-cases later.

envbuilder.go

git.go

git_test.go

envbuilder.go

git.go

testutil/gittest/gittest.go

BrunoQuaresma

Left a few comments but mostly are for my own learning 🙏

dannykopping

LGTM! Mostly nits

envbuilder.go

dannykopping · 2024-05-03T08:23:55Z

git_test.go

+func TestCloneRepoSSH(t *testing.T) {
+	t.Parallel()
+
+	t.Run("AuthSuccess", func(t *testing.T) {


Nit: maybe I'm missing something but since all of these tests share a basic structure, you could reduce them down to table tests to more clearly express the intent of what each test is doing and what its result should be; the boilerplate impedes readability.

Agreed in principle; but I'd prefer to do this when I have more test cases from which to abstract.

git_test.go

options.go

…uth setup

johnstcn · 2024-05-03T10:16:13Z

I've simplifed the logic by removing the need to parse the git URL at all and removing the known_hosts business; if folks need to validate host keys they can simply mount in a known_hosts file and set SSH_KNOWN_HOSTS.
I've also extracted the logic for setting up repo auth into a separate function and added dedicated tests for this.

johnstcn · 2024-05-03T11:00:41Z

Bonus points: SSH_AUTH_SOCK also works!

docker run -it --rm -e GIT_URL="ssh://git@github.com/johnstcn/test-private-devcontainer" -e SSH_AUTH_SOCK=/tmp/ssh-Vsi58Zsh72/agent.1567 -v /tmp/ssh-Vsi58Zsh72/agent.1567:/tmp/ssh-Vsi58Zsh72/agent.1567 -e INIT_SCRIPT=bash envbuilder:latest
envbuilder - Build development environments from repositories in a container
#1: 📦 Cloning ssh://git@github.com/johnstcn/test-private-devcontainer to /workspaces/test-private-devcontainer...
#1: 🔑 Using SSH authentication!
#1: 🔑 No SSH key found, falling back to agent!
#1: 🔓 SSH_KNOWN_HOSTS not set, accepting all host keys!
#1: 🔑 Got host key: github.com,20.26.156.215 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg=

dannykopping

Slick! Love the changes, last comment from me

git_test.go

dannykopping · 2024-05-03T11:17:18Z

git_test.go

-func TestParseGitURL(t *testing.T) {
-	t.Parallel()
+// nolint:paralleltest // t.Setenv for SSH_AUTH_SOCK
+func TestSetupRepoAuth(t *testing.T) {


mafredri

Looks great!

git.go

mafredri · 2024-05-03T12:08:40Z

git.go

+			options.Logger(codersdk.LogLevelError, "#1: ❌ Failed to connect to SSH agent: %s", err.Error())
+			return nil // nothing else we can do
+		}
+		if os.Getenv("SSH_KNOWN_HOSTS") == "" {


Suggestion: If this was moved higher up, we could avoid duplicating it.

I tried doing this, but we need to either return *gitssh.PublicKeys or *gitssh.PublicKeysCallback. We can't just assign HostKeyCallback to transport.AuthMethods as it's an interface type :(

mafredri · 2024-05-03T12:09:42Z

git_test.go

+		require.Nil(t, auth)
+	})
+
+	t.Run("HTTP/NoAuth", func(t *testing.T) {


Suggestion: could avoid prefix if these were nested in an otherwise empty t.Run. Feel free to disregard if you don't want to do the change. 😄

I think one level of nesting is as deep as we need here in the test :-)

git.go

johnstcn added 2 commits May 2, 2024 12:30

add SSH_PRIVATE_KEY_PATH option, add some tests

acff47a

handle known_hosts

8ff5c1a

johnstcn self-assigned this May 2, 2024

johnstcn added 2 commits May 2, 2024 15:11

add test for unknown host key

015c67a

fixup! add test for unknown host key

054abc8

johnstcn requested review from mafredri, dannykopping, mtojek and BrunoQuaresma May 2, 2024 15:54

mafredri reviewed May 2, 2024

View reviewed changes