From 05257687b97a4adf495113813a14dec3964b741e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 4 Oct 2024 09:25:22 +0000 Subject: [PATCH 1/4] fix: extract folder correctly from repo URLs with trailing slash Closes #380 Closes coder/customers#690 --- options/defaults.go | 11 +++++++---- options/defaults_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/options/defaults.go b/options/defaults.go index 220480d8..0278aabf 100644 --- a/options/defaults.go +++ b/options/defaults.go @@ -2,6 +2,7 @@ package options import ( "fmt" + "path" "strings" "github.com/go-git/go-billy/v5/osfs" @@ -25,12 +26,14 @@ func DefaultWorkspaceFolder(repoURL string) string { if err != nil { return EmptyWorkspaceDir } - name := strings.Split(parsed.Path, "/") - hasOwnerAndRepo := len(name) >= 2 - if !hasOwnerAndRepo { + repo := path.Base(parsed.Path) + // Giturls parsing never actually fails since ParseLocal never + // errors and places the entire URL in the Path field. This check + // ensures it's at least a Unix path containing forwardslash. + if repo == repoURL || repo == "" { return EmptyWorkspaceDir } - repo := strings.TrimSuffix(name[len(name)-1], ".git") + repo = strings.TrimSuffix(repo, ".git") return fmt.Sprintf("/workspaces/%s", repo) } diff --git a/options/defaults_test.go b/options/defaults_test.go index 4387c084..6e5fcf11 100644 --- a/options/defaults_test.go +++ b/options/defaults_test.go @@ -40,6 +40,41 @@ func TestDefaultWorkspaceFolder(t *testing.T) { gitURL: "https://github.com/coder/envbuilder.git#feature-branch", expected: "/workspaces/envbuilder", }, + { + name: "trailing", + gitURL: "https://github.com/coder/envbuilder.git/", + expected: "/workspaces/envbuilder", + }, + { + name: "trailing-x2", + gitURL: "https://github.com/coder/envbuilder.git//", + expected: "/workspaces/envbuilder", + }, + { + name: "fragment-trailing", + gitURL: "https://github.com/coder/envbuilder.git/#refs/heads/feature-branch", + expected: "/workspaces/envbuilder", + }, + { + name: "space", + gitURL: "https://github.com/coder/env%20builder.git", + expected: "/workspaces/env builder", + }, + { + name: "no .git", + gitURL: "https://github.com/coder/envbuilder", + expected: "/workspaces/envbuilder", + }, + { + name: "Unix path", + gitURL: "/repo", + expected: "/workspaces/repo", + }, + { + name: "Unix subpath", + gitURL: "/path/to/repo", + expected: "/workspaces/repo", + }, { name: "empty", gitURL: "", @@ -65,6 +100,10 @@ func TestDefaultWorkspaceFolder(t *testing.T) { name: "website URL", invalidURL: "www.google.com", }, + { + name: "Unix root", + invalidURL: "/", + }, } for _, tt := range invalidTests { t.Run(tt.name, func(t *testing.T) { From 250431468106a422eba0ddba733c250fb2f58796 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 4 Oct 2024 10:02:52 +0000 Subject: [PATCH 2/4] add review suggestion --- options/defaults_test.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/options/defaults_test.go b/options/defaults_test.go index 6e5fcf11..ec45e29b 100644 --- a/options/defaults_test.go +++ b/options/defaults_test.go @@ -35,11 +35,6 @@ func TestDefaultWorkspaceFolder(t *testing.T) { gitURL: "https://username:password@github.com/coder/envbuilder.git", expected: "/workspaces/envbuilder", }, - { - name: "fragment", - gitURL: "https://github.com/coder/envbuilder.git#feature-branch", - expected: "/workspaces/envbuilder", - }, { name: "trailing", gitURL: "https://github.com/coder/envbuilder.git/", @@ -50,21 +45,36 @@ func TestDefaultWorkspaceFolder(t *testing.T) { gitURL: "https://github.com/coder/envbuilder.git//", expected: "/workspaces/envbuilder", }, + { + name: "no .git", + gitURL: "https://github.com/coder/envbuilder", + expected: "/workspaces/envbuilder", + }, + { + name: "trailing no .git", + gitURL: "https://github.com/coder/envbuilder/", + expected: "/workspaces/envbuilder", + }, + { + name: "fragment", + gitURL: "https://github.com/coder/envbuilder.git#feature-branch", + expected: "/workspaces/envbuilder", + }, { name: "fragment-trailing", gitURL: "https://github.com/coder/envbuilder.git/#refs/heads/feature-branch", expected: "/workspaces/envbuilder", }, + { + name: "fragment-trailing no .git", + gitURL: "https://github.com/coder/envbuilder/#refs/heads/feature-branch", + expected: "/workspaces/envbuilder", + }, { name: "space", gitURL: "https://github.com/coder/env%20builder.git", expected: "/workspaces/env builder", }, - { - name: "no .git", - gitURL: "https://github.com/coder/envbuilder", - expected: "/workspaces/envbuilder", - }, { name: "Unix path", gitURL: "/repo", From 381bf7384ef9ebe6ddcf5be3b0a5f9a70003cd89 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 4 Oct 2024 11:44:30 +0000 Subject: [PATCH 3/4] fix when base is . --- integration/integration_test.go | 8 +++++--- options/defaults.go | 2 +- options/defaults_test.go | 4 ++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index b7332c04..7fd732d6 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -844,15 +844,17 @@ func TestContainerEnv(t *testing.T) { require.NoError(t, err) output := execContainer(t, ctr, "cat /env") - require.Contains(t, strings.TrimSpace(output), - `DEVCONTAINER=true + want := `DEVCONTAINER=true DEVCONTAINER_CONFIG=/workspaces/empty/.devcontainer/devcontainer.json ENVBUILDER=true FROM_CONTAINER_ENV=bar FROM_DOCKERFILE=foo FROM_REMOTE_ENV=baz PATH=/usr/local/bin:/bin:/go/bin:/opt -REMOTE_BAR=bar`) +REMOTE_BAR=bar` + if diff := cmp.Diff(want, strings.TrimSpace(output)); diff != "" { + require.Failf(t, "env mismatch", "diff (-want +got):\n%s", diff) + } } func TestUnsetOptionsEnv(t *testing.T) { diff --git a/options/defaults.go b/options/defaults.go index 0278aabf..761e1dee 100644 --- a/options/defaults.go +++ b/options/defaults.go @@ -30,7 +30,7 @@ func DefaultWorkspaceFolder(repoURL string) string { // Giturls parsing never actually fails since ParseLocal never // errors and places the entire URL in the Path field. This check // ensures it's at least a Unix path containing forwardslash. - if repo == repoURL || repo == "" { + if repo == repoURL || repo == "." || repo == "" { return EmptyWorkspaceDir } repo = strings.TrimSuffix(repo, ".git") diff --git a/options/defaults_test.go b/options/defaults_test.go index ec45e29b..643bd1f4 100644 --- a/options/defaults_test.go +++ b/options/defaults_test.go @@ -114,6 +114,10 @@ func TestDefaultWorkspaceFolder(t *testing.T) { name: "Unix root", invalidURL: "/", }, + { + name: "Git URL with no path", + invalidURL: "http://127.0.0.1:41073", + }, } for _, tt := range invalidTests { t.Run(tt.name, func(t *testing.T) { From 09f57b7115ea9e89f0394db28833e063f9f394da Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 4 Oct 2024 11:48:09 +0000 Subject: [PATCH 4/4] handle one more edge case of path.Base --- options/defaults.go | 2 +- options/defaults_test.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/options/defaults.go b/options/defaults.go index 761e1dee..2d6cd52f 100644 --- a/options/defaults.go +++ b/options/defaults.go @@ -30,7 +30,7 @@ func DefaultWorkspaceFolder(repoURL string) string { // Giturls parsing never actually fails since ParseLocal never // errors and places the entire URL in the Path field. This check // ensures it's at least a Unix path containing forwardslash. - if repo == repoURL || repo == "." || repo == "" { + if repo == repoURL || repo == "/" || repo == "." || repo == "" { return EmptyWorkspaceDir } repo = strings.TrimSuffix(repo, ".git") diff --git a/options/defaults_test.go b/options/defaults_test.go index 643bd1f4..de57365d 100644 --- a/options/defaults_test.go +++ b/options/defaults_test.go @@ -114,6 +114,10 @@ func TestDefaultWorkspaceFolder(t *testing.T) { name: "Unix root", invalidURL: "/", }, + { + name: "Path consists entirely of slash", + invalidURL: "//", + }, { name: "Git URL with no path", invalidURL: "http://127.0.0.1:41073",