Merge pull request #4983 from KnVerey/repospec_separators

Refactor parseGitURL

api/internal/git/repospec.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
@@ -79,99 +80,154 @@ func (x *RepoSpec) Cleaner(fSys filesys.FileSystem) func() error {
 	return func() error { return fSys.RemoveAll(x.Dir.String()) }
 }
 
+const (
+	refQuery         = "?ref="
+	gitSuffix        = ".git"
+	gitRootDelimiter = "_git/"
+	pathSeparator    = string(filepath.Separator)
+)
+
 // NewRepoSpecFromURL parses git-like urls.
 // From strings like git@github.com:someOrg/someRepo.git or
 // https://github.com/someOrg/someRepo?ref=someHash, extract
-// the parts.
+// the different parts of URL, set into a RepoSpec object and return RepoSpec object.
+// It MUST return an error if the input is not a git-like URL, as this is used by some code paths
+// to distinguish between local and remote paths.
+//
+// In particular, NewRepoSpecFromURL separates the URL used to clone the repo from the
+// elements Kustomize uses for other purposes (e.g. query params that turn into args, and
+// the path to the kustomization root within the repo).
 func NewRepoSpecFromURL(n string) (*RepoSpec, error) {
+	repoSpec := &RepoSpec{raw: n, Dir: notCloned, Timeout: defaultTimeout, Submodules: defaultSubmodules}
 	if filepath.IsAbs(n) {
 		return nil, fmt.Errorf("uri looks like abs path: %s", n)
 	}
-	repoSpecVal := parseGitURL(n)
-	if repoSpecVal.RepoPath == "" {
-		return nil, fmt.Errorf("url lacks repoPath: %s", n)
-	}
-	if repoSpecVal.Host == "" {
-		return nil, fmt.Errorf("url lacks host: %s", n)
-	}
-	cleanedPath := filepath.Clean(strings.TrimPrefix(repoSpecVal.KustRootPath, string(filepath.Separator)))
-	if pathElements := strings.Split(cleanedPath, string(filepath.Separator)); len(pathElements) > 0 &&
-		pathElements[0] == filesys.ParentDir {
-		return nil, fmt.Errorf("url path exits repo: %s", n)
-	}
-	return repoSpecVal, nil
-}
 
-const (
+	// Parse the query first. This is safe because according to rfc3986 "?" is only allowed in the
-	refQuery     = "?ref="
+	// query and is not recognized %-encoded.
-	gitSuffix    = ".git"
+	// Note that parseQuery returns default values for empty parameters.
-	gitDelimiter = "_git/"
+	n, query, _ := strings.Cut(n, "?")
-)
-
-// From strings like git@github.com:someOrg/someRepo.git or
-// https://github.com/someOrg/someRepo?ref=someHash, extract
-// the different parts of URL , set into a RepoSpec object and return RepoSpec object.
-func parseGitURL(n string) *RepoSpec {
-	repoSpec := &RepoSpec{raw: n, Dir: notCloned, Timeout: defaultTimeout, Submodules: defaultSubmodules}
-	// parse query first
-	// safe because according to rfc3986: ? only allowed in query
-	// and not recognized %-encoded
-	beforeQuery, query, _ := strings.Cut(n, "?")
-	n = beforeQuery
-	// if no query, defaults returned
 	repoSpec.Ref, repoSpec.Timeout, repoSpec.Submodules = parseQuery(query)
 
-	if strings.Contains(n, gitDelimiter) {
+	var err error
-		index := strings.Index(n, gitDelimiter)
+
-		// Adding _git/ to host
+	// Parse the host (e.g. scheme, username, domain) segment.
-		repoSpec.Host = normalizeGitHostSpec(n[:index+len(gitDelimiter)])
+	repoSpec.Host, n, err = extractHost(n)
-		repoSpec.RepoPath = strings.Split(n[index+len(gitDelimiter):], "/")[0]
+	if err != nil {
-		repoSpec.KustRootPath = parsePath(n[index+len(gitDelimiter)+len(repoSpec.RepoPath):])
+		return nil, err
-		return repoSpec
-	}
-	repoSpec.Host, n = extractHost(n)
-	isLocal := strings.HasPrefix(repoSpec.Host, "file://")
-	if !isLocal {
-		repoSpec.GitSuffix = gitSuffix
-	}
-	if strings.Contains(n, gitSuffix) {
-		repoSpec.GitSuffix = gitSuffix
-		index := strings.Index(n, gitSuffix)
-		repoSpec.RepoPath = n[0:index]
-		n = n[index+len(gitSuffix):]
-		if len(n) > 0 && n[0] == '/' {
-			n = n[1:]
-		}
-		repoSpec.KustRootPath = parsePath(n)
-		return repoSpec
 	}
 
-	if isLocal {
+	// In some cases, we're given a path to a git repo + a path to the kustomization root within
-		if idx := strings.Index(n, "//"); idx > 0 {
+	// that repo. We need to split them so that we can ultimately give the repo only to the cloner.
-			repoSpec.RepoPath = n[:idx]
+	repoSpec.RepoPath, repoSpec.KustRootPath, err = parsePathParts(n, defaultRepoPathLength(repoSpec.Host))
-			n = n[idx+2:]
+	if err != nil {
-			repoSpec.KustRootPath = parsePath(n)
+		return nil, err
-			return repoSpec
-		}
-		repoSpec.RepoPath = parsePath(n)
-		return repoSpec
 	}
 
-	i := strings.Index(n, "/")
+	// If the repo name ends in .git, isolate it. It will be added back by the clone spec function.
-	if i < 1 {
+	if idx := strings.Index(repoSpec.RepoPath, gitSuffix); idx >= 0 {
-		repoSpec.KustRootPath = parsePath(n)
+		repoSpec.GitSuffix = gitSuffix
-		return repoSpec
+		repoSpec.RepoPath = repoSpec.RepoPath[:idx]
 	}
-	j := strings.Index(n[i+1:], "/")
+	// Force the .git suffix URLs for services whose clone URL is the repo URL + .git.
-	if j >= 0 {
+	// This allows us to support the repo URL as an input instead of the actual clone URL.
-		j += i + 1
+	if legacyAddGitSuffix(repoSpec.Host, repoSpec.RepoPath) {
-		repoSpec.RepoPath = n[:j]
+		repoSpec.GitSuffix = gitSuffix
-		repoSpec.KustRootPath = parsePath(n[j+1:])
-		return repoSpec
 	}
-	repoSpec.KustRootPath = ""
+
-	repoSpec.RepoPath = parsePath(n)
+	return repoSpec, nil
-	return repoSpec
+}
+
+// legacyAddGitSuffix returns true if the .git suffix has historically been added to the repoSpec
+// (but not necessarily the cloneSpec) for the given host and repoPath.
+// TODO(@knverey): Remove repoSpec.gitSuffix entirely.
+// The .git suffix is a popular convention, but not universally used. Kustomize seems to force it
+// for non-local because of Github, which now handles suffix-less URLs just fine, as do Gitlab and Bitbucket.
+func legacyAddGitSuffix(host, repoPath string) bool {
+	return !strings.Contains(repoPath, gitRootDelimiter) &&
+		!strings.HasPrefix(host, fileScheme)
+}
+
+const allSegments = -999999
+const orgRepoSegments = 2
+
+func defaultRepoPathLength(host string) int {
+	if strings.HasPrefix(host, fileScheme) {
+		return allSegments
+	}
+	return orgRepoSegments
+}
+
+// parsePathParts splits the repo path that will ultimately be passed to git to clone the
+// repo from the kustomization root path, which Kustomize will execute the build in after the repo
+// is cloned.
+//
+// We first try to do this based on explicit markers in the URL (e.g. _git, .git or //).
+// If none are present, we try to apply a historical default repo path length that is derived from
+// Github URLs. If there aren't enough segments, we have historically considered the URL invalid.
+func parsePathParts(n string, defaultSegmentLength int) (string, string, error) {
+	repoPath, kustRootPath, success := tryExplicitMarkerSplit(n)
+	if !success {
+		repoPath, kustRootPath, success = tryDefaultLengthSplit(n, defaultSegmentLength)
+	}
+
+	// Validate the result
+	if !success || len(repoPath) == 0 {
+		return "", "", fmt.Errorf("failed to parse repo path segment")
+	}
+	if kustRootPathExitsRepo(kustRootPath) {
+		return "", "", fmt.Errorf("url path exits repo: %s", n)
+	}
+
+	return repoPath, strings.TrimPrefix(kustRootPath, pathSeparator), nil
+}
+
+func tryExplicitMarkerSplit(n string) (string, string, bool) {
+	// Look for the _git delimiter, which by convention is expected to be ONE directory above the repo root.
+	// If found, split on the NEXT path element, which is the repo root.
+	// Example: https://username@dev.azure.com/org/project/_git/repo/path/to/kustomization/root
+	if gitRootIdx := strings.Index(n, gitRootDelimiter); gitRootIdx >= 0 {
+		gitRootPath := n[:gitRootIdx+len(gitRootDelimiter)]
+		subpathSegments := strings.Split(n[gitRootIdx+len(gitRootDelimiter):], pathSeparator)
+		return gitRootPath + subpathSegments[0], strings.Join(subpathSegments[1:], pathSeparator), true
+
+		// Look for a double-slash in the path, which if present separates the repo root from the kust path.
+		// It is a convention, not a real path element, so do not preserve it in the returned value.
+		// Example: https://github.com/org/repo//path/to/kustomozation/root
+	} else if repoRootIdx := strings.Index(n, "//"); repoRootIdx >= 0 {
+		return n[:repoRootIdx], n[repoRootIdx+2:], true
+
+		// Look for .git in the path, which if present is part of the directory name of the git repo.
+		// This means we want to grab everything up to and including that suffix
+		// Example: https://github.com/org/repo.git/path/to/kustomozation/root
+	} else if gitSuffixIdx := strings.Index(n, gitSuffix); gitSuffixIdx >= 0 {
+		upToGitSuffix := n[:gitSuffixIdx+len(gitSuffix)]
+		afterGitSuffix := n[gitSuffixIdx+len(gitSuffix):]
+		return upToGitSuffix, afterGitSuffix, true
+	}
+	return "", "", false
+}
+
+func tryDefaultLengthSplit(n string, defaultSegmentLength int) (string, string, bool) {
+	// If the default is to take all segments, do so.
+	if defaultSegmentLength == allSegments {
+		return n, "", true
+
+		// If the default is N segments, make sure we have at least that many and take them if so.
+		// If we have less than N, we have historically considered the URL invalid.
+	} else if segments := strings.Split(n, pathSeparator); len(segments) >= defaultSegmentLength {
+		firstNSegments := strings.Join(segments[:defaultSegmentLength], pathSeparator)
+		rest := strings.Join(segments[defaultSegmentLength:], pathSeparator)
+		return firstNSegments, rest, true
+	}
+	return "", "", false
+}
+
+func kustRootPathExitsRepo(kustRootPath string) bool {
+	cleanedPath := filepath.Clean(strings.TrimPrefix(kustRootPath, pathSeparator))
+	pathElements := strings.Split(cleanedPath, pathSeparator)
+	return len(pathElements) > 0 &&
+		pathElements[0] == filesys.ParentDir
 }
 
 // Clone git submodules by default.
@@ -219,16 +275,7 @@ func parseQuery(query string) (string, time.Duration, bool) {
 	return ref, duration, submodules
 }
 
-func parsePath(n string) string {
+func extractHost(n string) (string, string, error) {
-	parsed, err := url.Parse(n)
-	// TODO(annasong): decide how to handle error, i.e. return error, empty string, etc.
-	if err != nil {
-		return n
-	}
-	return parsed.Path
-}
-
-func extractHost(n string) (string, string) {
 	n = ignoreForcedGitProtocol(n)
 	scheme, n := extractScheme(n)
 	username, n := extractUsername(n)
@@ -238,19 +285,20 @@ func extractHost(n string) (string, string) {
 	// Validate the username and scheme before attempting host/path parsing, because if the parsing
 	// so far has not succeeded, we will not be able to extract the host and path correctly.
 	if err := validateScheme(scheme, acceptSCP); err != nil {
-		// TODO: return this error instead.
+		return "", "", err
-		return "", n
 	}
 
 	// Now that we have extracted a valid scheme+username, we can parse host itself.
 
 	// The file protocol specifies an absolute path to a local git repo.
 	// Everything after the scheme (including any 'username' we found) is actually part of that path.
-	if scheme == "file://" {
+	if scheme == fileScheme {
-		return scheme, username + n
+		return scheme, username + n, nil
+	}
+	var host, rest = n, ""
+	if sepIndex := findPathSeparator(n, acceptSCP); sepIndex >= 0 {
+		host, rest = n[:sepIndex+1], n[sepIndex+1:]
 	}
-	sepIndex := findPathSeparator(n, acceptSCP)
-	host, rest := n[:sepIndex+1], n[sepIndex+1:]
 
 	// Github URLs are strictly normalized in a way that may discard scheme and username components.
 	if stdGithub {
@@ -259,10 +307,9 @@ func extractHost(n string) (string, string) {
 
 	// Host is required, so do not concat the scheme and username if we didn't find one.
 	if host == "" {
-		// TODO: This should return an error.
+		return "", "", errors.Errorf("failed to parse host segment")
-		return "", n
 	}
-	return scheme + username + host, rest
+	return scheme + username + host, rest, nil
 }
 
 // ignoreForcedGitProtocol strips the "git::" prefix from URLs.
@@ -298,7 +345,7 @@ func validateScheme(scheme string, acceptSCPStyle bool) error {
 		if !acceptSCPStyle {
 			return fmt.Errorf("failed to parse scheme")
 		}
-	case "ssh://", "file://", "https://", "http://":
+	case sshScheme, fileScheme, httpsScheme, httpScheme:
 		// These are all supported schemes
 	default:
 		// At time of writing, we should never end up here because we do not parse out
@@ -308,8 +355,13 @@ func validateScheme(scheme string, acceptSCPStyle bool) error {
 	return nil
 }
 
+const fileScheme = "file://"
+const httpScheme = "http://"
+const httpsScheme = "https://"
+const sshScheme = "ssh://"
+
 func extractScheme(s string) (string, string) {
-	for _, prefix := range []string{"ssh://", "https://", "http://", "file://"} {
+	for _, prefix := range []string{sshScheme, httpsScheme, httpScheme, fileScheme} {
 		if rest, found := trimPrefixIgnoreCase(s, prefix); found {
 			return prefix, rest
 		}
@@ -340,7 +392,7 @@ func trimPrefixIgnoreCase(s, prefix string) (string, bool) {
 }
 
 func findPathSeparator(hostPath string, acceptSCP bool) int {
-	sepIndex := strings.Index(hostPath, "/")
+	sepIndex := strings.Index(hostPath, pathSeparator)
 	if acceptSCP {
 		// The colon acts as a delimiter in scp-style ssh URLs only if not prefixed by '/'.
 		if colonIndex := strings.Index(hostPath, ":"); colonIndex > 0 && colonIndex < sepIndex {
@@ -350,30 +402,13 @@ func findPathSeparator(hostPath string, acceptSCP bool) int {
 	return sepIndex
 }
 
-const normalizedHTTPGithub = "https://github.com/"
 const gitUsername = "git@"
-const normalizedSCPGithub = gitUsername + "github.com:"
 
 func normalizeGithubHostParts(scheme, username string) (string, string, string) {
-	if strings.HasPrefix(scheme, "ssh://") || username != "" {
+	if strings.HasPrefix(scheme, sshScheme) || username != "" {
 		return "", username, "github.com:"
 	}
-	return "https://", "", "github.com/"
+	return httpsScheme, "", "github.com/"
-}
-
-func normalizeGitHostSpec(host string) string {
-	s := strings.ToLower(host)
-	if strings.Contains(s, "github.com") {
-		if strings.Contains(s, gitUsername) || strings.Contains(s, "ssh:") {
-			host = normalizedSCPGithub
-		} else {
-			host = normalizedHTTPGithub
-		}
-	}
-	if strings.HasPrefix(s, "git::") {
-		host = strings.TrimPrefix(s, "git::")
-	}
-	return host
 }
 
 // The format of Azure repo URL is documented

api/internal/git/repospec_test.go

@@ -82,27 +82,31 @@ func TestNewRepoSpecFromUrlErrors(t *testing.T) {
 		},
 		"relative path": {
 			"../../tmp",
-			"url lacks host",
+			"failed to parse scheme",
+		},
+		"local path that looks somewhat like a github url": {
+			"src/github.com/org/repo/path",
+			"failed to parse scheme",
 		},
 		"no_slashes": {
 			"iauhsdiuashduas",
-			"url lacks repoPath",
+			"failed to parse scheme",
 		},
 		"bad_scheme": {
 			"htxxxtp://github.com/",
-			"url lacks host",
+			"failed to parse scheme",
 		},
 		"no_org_repo": {
 			"ssh://git.example.com",
-			"url lacks repoPath",
+			"failed to parse repo path segment",
 		},
 		"hashicorp_git_only": {
 			"git::___",
-			"url lacks repoPath",
+			"failed to parse scheme",
 		},
 		"query_after_host": {
 			"https://host?ref=group/version/minor_version",
-			"url lacks repoPath",
+			"failed to parse repo path segment",
 		},
 		"path_exits_repo": {
 			"https://github.com/org/repo.git//path/../../exits/repo",
@@ -110,11 +114,27 @@ func TestNewRepoSpecFromUrlErrors(t *testing.T) {
 		},
 		"bad github separator": {
 			"github.com!org/repo.git//path",
-			"url lacks host",
+			"failed to parse scheme",
 		},
 		"mysterious gh: prefix previously supported is no longer handled": {
 			"gh:org/repo",
-			"url lacks repoPath",
+			"failed to parse scheme",
+		},
+		"invalid Github url missing orgrepo": {
+			"https://github.com/thisisa404.yaml",
+			"failed to parse repo path segment",
+		},
+		"file protocol with excessive slashes": { // max valid is three: two for the scheme and one for the root
+			"file:////tmp//path/to/whatever",
+			"failed to parse repo path segment",
+		},
+		"unsupported protocol after username (invalid)": {
+			"git@scp://github.com/org/repo.git//path",
+			"failed to parse repo path segment",
+		},
+		"supported protocol after username (invalid)": {
+			"git@ssh://github.com/org/repo.git//path",
+			"failed to parse repo path segment",
 		},
 	}
 
@@ -137,7 +157,6 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 		repoSpec  RepoSpec
 		cloneSpec string
 		absPath   string
-		skip      string
 	}{
 		{
 			name:      "t1",
@@ -201,7 +220,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			},
 		},
 		{
-			name:      "t6",
+			name:      "non-github_scp",
 			input:     "git@gitlab2.sqtools.ru:infra/kubernetes/thanos-base.git?ref=v0.1.0",
 			cloneSpec: "git@gitlab2.sqtools.ru:infra/kubernetes/thanos-base.git",
 			absPath:   notCloned.String(),
@@ -213,14 +232,14 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			},
 		},
 		{
-			name:      "non-github_scp",
+			name:      "non-github_scp with path delimiter",
 			input:     "git@bitbucket.org:company/project.git//path?ref=branch",
 			cloneSpec: "git@bitbucket.org:company/project.git",
 			absPath:   notCloned.Join("path"),
 			repoSpec: RepoSpec{
 				Host:         "git@bitbucket.org:",
 				RepoPath:     "company/project",
-				KustRootPath: "/path",
+				KustRootPath: "path",
 				Ref:          "branch",
 				GitSuffix:    ".git",
 			},
@@ -233,7 +252,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "git@bitbucket.org/",
 				RepoPath:     "company/project",
-				KustRootPath: "/path",
+				KustRootPath: "path",
 				Ref:          "branch",
 				GitSuffix:    ".git",
 			},
@@ -246,52 +265,52 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "ssh://git@bitbucket.org/",
 				RepoPath:     "company/project",
-				KustRootPath: "/path",
+				KustRootPath: "path",
 				Ref:          "branch",
 				GitSuffix:    ".git",
 			},
 		},
 		{
-			name:      "t8",
+			name:      "_git host delimiter in non-github url",
 			input:     "https://itfs.mycompany.com/collection/project/_git/somerepos",
 			cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos",
 			absPath:   notCloned.String(),
 			repoSpec: RepoSpec{
-				Host:     "https://itfs.mycompany.com/collection/project/_git/",
+				Host:     "https://itfs.mycompany.com/",
-				RepoPath: "somerepos",
+				RepoPath: "collection/project/_git/somerepos",
 			},
 		},
 		{
-			name:      "t9",
+			name:      "_git host delimiter in non-github url with params",
 			input:     "https://itfs.mycompany.com/collection/project/_git/somerepos?version=v1.0.0",
 			cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos",
 			absPath:   notCloned.String(),
 			repoSpec: RepoSpec{
-				Host:     "https://itfs.mycompany.com/collection/project/_git/",
+				Host:     "https://itfs.mycompany.com/",
-				RepoPath: "somerepos",
+				RepoPath: "collection/project/_git/somerepos",
 				Ref:      "v1.0.0",
 			},
 		},
 		{
-			name:      "t10",
+			name:      "_git host delimiter in non-github url with kust root path and params",
 			input:     "https://itfs.mycompany.com/collection/project/_git/somerepos/somedir?version=v1.0.0",
 			cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos",
 			absPath:   notCloned.Join("somedir"),
 			repoSpec: RepoSpec{
-				Host:         "https://itfs.mycompany.com/collection/project/_git/",
+				Host:         "https://itfs.mycompany.com/",
-				RepoPath:     "somerepos",
+				RepoPath:     "collection/project/_git/somerepos",
-				KustRootPath: "/somedir",
+				KustRootPath: "somedir",
 				Ref:          "v1.0.0",
 			},
 		},
 		{
-			name:      "t11",
+			name:      "_git host delimiter in non-github url with no kust root path",
 			input:     "git::https://itfs.mycompany.com/collection/project/_git/somerepos",
 			cloneSpec: "https://itfs.mycompany.com/collection/project/_git/somerepos",
 			absPath:   notCloned.String(),
 			repoSpec: RepoSpec{
-				Host:     "https://itfs.mycompany.com/collection/project/_git/",
+				Host:     "https://itfs.mycompany.com/",
-				RepoPath: "somerepos",
+				RepoPath: "collection/project/_git/somerepos",
 			},
 		},
 		{
@@ -337,13 +356,13 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "https://github.com/",
 				RepoPath:     "kubernetes-sigs/kustomize",
-				KustRootPath: "/examples/multibases/dev/",
+				KustRootPath: "examples/multibases/dev/",
 				Ref:          "v1.0.6",
 				GitSuffix:    ".git",
 			},
 		},
 		{
-			name:      "t16",
+			name:      "file protocol with git-suffixed repo path and params",
 			input:     "file://a/b/c/someRepo.git/somepath?ref=someBranch",
 			cloneSpec: "file://a/b/c/someRepo.git",
 			absPath:   notCloned.Join("somepath"),
@@ -356,7 +375,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			},
 		},
 		{
-			name:      "t17",
+			name:      "file protocol with two slashes, with kust root path and params",
 			input:     "file://a/b/c/someRepo//somepath?ref=someBranch",
 			cloneSpec: "file://a/b/c/someRepo",
 			absPath:   notCloned.Join("somepath"),
@@ -368,7 +387,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			},
 		},
 		{
-			name:      "t18",
+			name:      "file protocol with two slashes, with ref and no kust root path",
 			input:     "file://a/b/c/someRepo?ref=someBranch",
 			cloneSpec: "file://a/b/c/someRepo",
 			absPath:   notCloned.String(),
@@ -379,7 +398,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			},
 		},
 		{
-			name:      "t19",
+			name:      "file protocol with three slashes, with ref and no kust root path",
 			input:     "file:///a/b/c/someRepo?ref=someBranch",
 			cloneSpec: "file:///a/b/c/someRepo",
 			absPath:   notCloned.String(),
@@ -397,13 +416,13 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "git@github.com:",
 				RepoPath:     "kubernetes-sigs/kustomize",
-				KustRootPath: "/examples/multibases/dev",
+				KustRootPath: "examples/multibases/dev",
 				Ref:          "v1.0.6",
 				GitSuffix:    ".git",
 			},
 		},
 		{
-			name:      "t21",
+			name:      "file protocol with three slashes, no kust root path or params",
 			input:     "file:///a/b/c/someRepo",
 			cloneSpec: "file:///a/b/c/someRepo",
 			absPath:   notCloned.String(),
@@ -413,7 +432,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			},
 		},
 		{
-			name:      "t22",
+			name:      "file protocol with three slashes, no repo or kust root path or params",
 			input:     "file:///",
 			cloneSpec: "file:///",
 			absPath:   notCloned.String(),
@@ -423,28 +442,26 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			},
 		},
 		{
-			name:      "t23",
+			name:      "double-slash path delimiter https",
-			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",
+			cloneSpec: "https://fake-git-hosting.org/path/to/repo.git",
 			absPath:   notCloned.Join("/examples/multibases/dev"),
 			repoSpec: RepoSpec{
 				Host:         "https://fake-git-hosting.org/",
 				RepoPath:     "path/to/repo",
-				KustRootPath: "/examples/multibases/dev",
+				KustRootPath: "examples/multibases/dev",
 				GitSuffix:    ".git",
 			},
 		},
 		{
-			name:      "t24",
+			name:      "double-slash path delimeter ssh",
-			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",
+				Host:         "ssh://alice@acme.co/",
 				RepoPath:     "path/to/repo",
-				KustRootPath: "/examples/multibases/dev",
+				KustRootPath: "examples/multibases/dev",
 				GitSuffix:    ".git",
 			},
 		},
@@ -504,7 +521,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "ssh://myusername@bitbucket.org/",
 				RepoPath:     "ourteamname/ourrepositoryname",
-				KustRootPath: "/path",
+				KustRootPath: "path",
 				Ref:          "branch",
 				GitSuffix:    ".git",
 			},
@@ -517,7 +534,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "file://",
 				RepoPath:     "git@home/path/to/repository",
-				KustRootPath: "/path",
+				KustRootPath: "path",
 				Ref:          "branch",
 				GitSuffix:    ".git",
 			},
@@ -530,7 +547,7 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "http://git@home.com/",
 				RepoPath:     "path/to/repository",
-				KustRootPath: "/path",
+				KustRootPath: "path",
 				Ref:          "branch",
 				GitSuffix:    ".git",
 			},
@@ -543,35 +560,11 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 			repoSpec: RepoSpec{
 				Host:         "https://git@home.com/",
 				RepoPath:     "path/to/repository",
-				KustRootPath: "/path",
+				KustRootPath: "path",
 				Ref:          "branch",
 				GitSuffix:    ".git",
 			},
 		},
-		{
-			name:      "unsupported protocol after username (invalid but currently passed through to git)",
-			input:     "git@scp://github.com/org/repo.git//path",
-			cloneSpec: "git@scp://github.com/org/repo.git",
-			absPath:   notCloned.Join("path"),
-			repoSpec: RepoSpec{
-				Host:         "git@scp:",
-				RepoPath:     "//github.com/org/repo",
-				KustRootPath: "/path",
-				GitSuffix:    ".git",
-			},
-		},
-		{
-			name:      "supported protocol after username (invalid but currently passed through to git)",
-			input:     "git@ssh://github.com/org/repo.git//path",
-			cloneSpec: "git@ssh://github.com/org/repo.git",
-			absPath:   notCloned.Join("path"),
-			repoSpec: RepoSpec{
-				Host:         "git@ssh:",
-				RepoPath:     "//github.com/org/repo",
-				KustRootPath: "/path",
-				GitSuffix:    ".git",
-			},
-		},
 		{
 			name:      "complex github ssh url from docs",
 			input:     "ssh://git@ssh.github.com:443/YOUR-USERNAME/YOUR-REPOSITORY.git",
@@ -596,13 +589,43 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) {
 				GitSuffix:    ".git",
 			},
 		},
+		{
+			name:      "gitlab URLs with explicit git suffix",
+			input:     "git@gitlab.com:gitlab-tests/sample-project.git",
+			cloneSpec: "git@gitlab.com:gitlab-tests/sample-project.git",
+			absPath:   notCloned.String(),
+			repoSpec: RepoSpec{
+				Host:      "git@gitlab.com:",
+				RepoPath:  "gitlab-tests/sample-project",
+				GitSuffix: ".git",
+			},
+		},
+		{
+			name:      "gitlab URLs without explicit git suffix",
+			input:     "git@gitlab.com:gitlab-tests/sample-project",
+			cloneSpec: "git@gitlab.com:gitlab-tests/sample-project.git",
+			absPath:   notCloned.String(),
+			repoSpec: RepoSpec{
+				Host:      "git@gitlab.com:",
+				RepoPath:  "gitlab-tests/sample-project",
+				GitSuffix: ".git",
+			},
+		},
+		{
+			name:      "azure host with _git and // path separator",
+			input:     "https://username@dev.azure.com/org/project/_git/repo//path/to/kustomization/root",
+			cloneSpec: "https://username@dev.azure.com/org/project/_git/repo",
+			absPath:   notCloned.Join("path/to/kustomization/root"),
+			repoSpec: RepoSpec{
+				Host:         "https://username@dev.azure.com/",
+				RepoPath:     "org/project/_git/repo",
+				KustRootPath: "path/to/kustomization/root",
+				GitSuffix:    "",
+			},
+		},
 	}
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			if tc.skip != "" {
-				t.Skip(tc.skip)
-			}
-
 			rs, err := NewRepoSpecFromURL(tc.input)
 			require.NoError(t, err)
 			assert.Equal(t, tc.cloneSpec, rs.CloneSpec(), "cloneSpec mismatch")

api/krusty/remoteloader_test.go

@@ -188,14 +188,6 @@ resources:
 			kustomization: `
 resources:
 - file://$ROOT/simple.git?timeout=500
-`,
-			expected: simpleBuild,
-		},
-		{
-			name: "triple slash absolute path",
-			kustomization: `
-resources:
-- file:///$ROOT/simple.git
 `,
 			expected: simpleBuild,
 		},