Rewrite remoteload_test integration tests (#4783)

* Better error message when git clone fails * support file:// URLs * rewrite remoteload_test * lint and test fix * fixes for kverey's comments * document file:// remote load * replace assert with require where appropriate * add tests for file:// without git suffix * fixes plus pr review from natasha * fixes for review, lint * revert changes to error handling * fix skipped test
2026-06-13 10:00:56 +00:00 · 2022-09-19 12:13:18 -04:00
parent d6e40a3f6c
commit e62480d11c
16 changed files with 633 additions and 450 deletions
--- a/api/internal/git/cloner.go
+++ b/api/internal/git/cloner.go
@@ -22,15 +22,11 @@ func ClonerUsingGitExec(repoSpec *RepoSpec) error {
 	if err = r.run("init"); err != nil {
 		return err
 	}
-	if err = r.run(
-		"remote", "add", "origin", repoSpec.CloneSpec()); err != nil {
-		return err
-	}
 	ref := "HEAD"
 	if repoSpec.Ref != "" {
 		ref = repoSpec.Ref
 	}
-	if err = r.run("fetch", "--depth=1", "origin", ref); err != nil {
+	if err = r.run("fetch", "--depth=1", repoSpec.CloneSpec(), ref); err != nil {
 		return err
 	}
 	if err = r.run("checkout", "FETCH_HEAD"); err != nil {
--- a/api/internal/git/gitrunner.go
+++ b/api/internal/git/gitrunner.go
@@ -46,9 +46,9 @@ func (r gitRunner) run(args ...string) error {
 		cmd.String(),
 		r.duration,
 		func() error {
-			_, err := cmd.CombinedOutput()
+			out, err := cmd.CombinedOutput()
 			if err != nil {
-				return errors.Wrapf(err, "git cmd = '%s'", cmd.String())
+				return errors.Wrapf(err, "failed to run '%s': %s", cmd.String(), string(out))
 			}
 			return err
 		})
--- a/api/internal/git/repospec.go
+++ b/api/internal/git/repospec.go
@@ -119,8 +119,12 @@ func parseGitURL(n string) (
 		return
 	}
 	host, n = parseHostSpec(n)
-	gitSuff = gitSuffix
+	isLocal := strings.HasPrefix(host, "file://")
+	if !isLocal {
+		gitSuff = gitSuffix
+	}
 	if strings.Contains(n, gitSuffix) {
+		gitSuff = gitSuffix
 		index := strings.Index(n, gitSuffix)
 		orgRepo = n[0:index]
 		n = n[index+len(gitSuffix):]
@@ -131,6 +135,19 @@ func parseGitURL(n string) (
 		return
 	}

+	if isLocal {
+		if idx := strings.Index(n, "//"); idx > 0 {
+			orgRepo = n[:idx]
+			n = n[idx+2:]
+			path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
+			return
+		}
+		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
+		orgRepo = path
+		path = ""
+		return
+	}
+
 	i := strings.Index(n, "/")
 	if i < 1 {
 		path, gitRef, gitTimeout, gitSubmodules = peelQuery(n)
@@ -200,7 +217,7 @@ func parseHostSpec(n string) (string, string) {
 	// Start accumulating the host part.
 	for _, p := range []string{
 		// Order matters here.
-		"git::", "gh:", "ssh://", "https://", "http://",
+		"git::", "gh:", "ssh://", "https://", "http://", "file://",
 		"git@", "github.com:", "github.com/"} {
 		if len(p) < len(n) && strings.ToLower(n[:len(p)]) == p {
 			n = n[len(p):]
--- a/api/internal/git/repospec_test.go
+++ b/api/internal/git/repospec_test.go
@@ -11,6 +11,7 @@ import (
 	"time"

 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )

 func TestNewRepoSpecFromUrl_Permute(t *testing.T) {
@@ -61,6 +62,7 @@ func TestNewRepoSpecFromUrl_Permute(t *testing.T) {
 					rs, err := NewRepoSpecFromURL(uri)
 					if err != nil {
 						t.Errorf("problem %v", err)
+						continue
 					}
 					if rs.Host != hostSpec {
 						bad = append(bad, []string{"host", uri, rs.Host, hostSpec})
@@ -120,6 +122,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 		repoSpec  RepoSpec
 		cloneSpec string
 		absPath   string
+		skip      string
 	}{
 		{
 			name:      "t1",
@@ -285,11 +288,134 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 				GitSuffix: ".git",
 			},
 		},
+		{
+			name:      "t15",
+			input:     "https://github.com/kubernetes-sigs/kustomize//examples/multibases/dev/?ref=v1.0.6",
+			cloneSpec: "https://github.com/kubernetes-sigs/kustomize.git",
+			absPath:   notCloned.Join("/examples/multibases/dev"),
+			repoSpec: RepoSpec{
+				Host:      "https://github.com/",
+				OrgRepo:   "kubernetes-sigs/kustomize",
+				Path:      "/examples/multibases/dev/",
+				Ref:       "v1.0.6",
+				GitSuffix: ".git",
+			},
+		},
+		{
+			name:      "t16",
+			input:     "file://a/b/c/someRepo.git/somepath?ref=someBranch",
+			cloneSpec: "file://a/b/c/someRepo.git",
+			absPath:   notCloned.Join("somepath"),
+			repoSpec: RepoSpec{
+				Host:      "file://",
+				OrgRepo:   "a/b/c/someRepo",
+				Path:      "somepath",
+				Ref:       "someBranch",
+				GitSuffix: ".git",
+			},
+		},
+		{
+			name:      "t17",
+			input:     "file://a/b/c/someRepo//somepath?ref=someBranch",
+			cloneSpec: "file://a/b/c/someRepo",
+			absPath:   notCloned.Join("somepath"),
+			repoSpec: RepoSpec{
+				Host:    "file://",
+				OrgRepo: "a/b/c/someRepo",
+				Path:    "somepath",
+				Ref:     "someBranch",
+			},
+		},
+		{
+			name:      "t18",
+			input:     "file://a/b/c/someRepo?ref=someBranch",
+			cloneSpec: "file://a/b/c/someRepo",
+			absPath:   notCloned.String(),
+			repoSpec: RepoSpec{
+				Host:    "file://",
+				OrgRepo: "a/b/c/someRepo",
+				Ref:     "someBranch",
+			},
+		},
+		{
+			name:      "t19",
+			input:     "file:///a/b/c/someRepo?ref=someBranch",
+			cloneSpec: "file:///a/b/c/someRepo",
+			absPath:   notCloned.String(),
+			repoSpec: RepoSpec{
+				Host:    "file://",
+				OrgRepo: "/a/b/c/someRepo",
+				Ref:     "someBranch",
+			},
+		},
+		{
+			name:      "t20",
+			input:     "ssh://git@github.com/kubernetes-sigs/kustomize//examples/multibases/dev?ref=v1.0.6",
+			cloneSpec: "git@github.com:kubernetes-sigs/kustomize.git",
+			absPath:   notCloned.Join("examples/multibases/dev"),
+			repoSpec: RepoSpec{
+				Host:      "git@github.com:",
+				OrgRepo:   "kubernetes-sigs/kustomize",
+				Path:      "/examples/multibases/dev",
+				Ref:       "v1.0.6",
+				GitSuffix: ".git",
+			},
+		},
+		{
+			name:      "t21",
+			input:     "file:///a/b/c/someRepo",
+			cloneSpec: "file:///a/b/c/someRepo",
+			absPath:   notCloned.String(),
+			repoSpec: RepoSpec{
+				Host:    "file://",
+				OrgRepo: "/a/b/c/someRepo",
+			},
+		},
+		{
+			name:      "t22",
+			input:     "file:///",
+			cloneSpec: "file:///",
+			absPath:   notCloned.String(),
+			repoSpec: RepoSpec{
+				Host:    "file://",
+				OrgRepo: "/",
+			},
+		},
+		{
+			name:      "t23",
+			skip:      "the `//` repo separator does not work",
+			input:     "https://fake-git-hosting.org/path/to/repo//examples/multibases/dev",
+			cloneSpec: "https://fake-git-hosting.org/path/to.git",
+			absPath:   notCloned.Join("/examples/multibases/dev"),
+			repoSpec: RepoSpec{
+				Host:      "https://fake-git-hosting.org/",
+				OrgRepo:   "path/to/repo",
+				Path:      "/examples/multibases/dev",
+				GitSuffix: ".git",
+			},
+		},
+		{
+			name:      "t24",
+			skip:      "the `//` repo separator does not work",
+			input:     "ssh://alice@acme.co/path/to/repo//examples/multibases/dev",
+			cloneSpec: "ssh://alice@acme.co/path/to/repo.git",
+			absPath:   notCloned.Join("/examples/multibases/dev"),
+			repoSpec: RepoSpec{
+				Host:      "ssh://alice@acme.co",
+				OrgRepo:   "path/to/repo",
+				Path:      "/examples/multibases/dev",
+				GitSuffix: ".git",
+			},
+		},
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
+			if tc.skip != "" {
+				t.Skip(tc.skip)
+			}
+
 			rs, err := NewRepoSpecFromURL(tc.input)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, tc.cloneSpec, rs.CloneSpec(), "cloneSpec mismatch")
 			assert.Equal(t, tc.absPath, rs.AbsPath(), "absPath mismatch")
 			// some values have defaults. Clear them here so test cases remain compact.