From 00208394d6754a85a6e2882b61817198e3382c1e Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Tue, 3 Jan 2023 18:40:38 -0500 Subject: [PATCH] Refactor parseGitURL to consolidate code paths, improve error handling --- api/internal/git/repospec.go | 271 +++++++++++++++++------------- api/internal/git/repospec_test.go | 173 ++++++++++--------- api/krusty/remoteloader_test.go | 8 - 3 files changed, 251 insertions(+), 201 deletions(-) diff --git a/api/internal/git/repospec.go b/api/internal/git/repospec.go index 8f9c24777..3d50e412a 100644 --- a/api/internal/git/repospec.go +++ b/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 ( - refQuery = "?ref=" - gitSuffix = ".git" - gitDelimiter = "_git/" -) - -// 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 + // Parse the query first. This is safe because according to rfc3986 "?" is only allowed in the + // query and is not recognized %-encoded. + // Note that parseQuery returns default values for empty parameters. + n, query, _ := strings.Cut(n, "?") repoSpec.Ref, repoSpec.Timeout, repoSpec.Submodules = parseQuery(query) - if strings.Contains(n, gitDelimiter) { - index := strings.Index(n, gitDelimiter) - // Adding _git/ to host - repoSpec.Host = normalizeGitHostSpec(n[:index+len(gitDelimiter)]) - repoSpec.RepoPath = strings.Split(n[index+len(gitDelimiter):], "/")[0] - repoSpec.KustRootPath = parsePath(n[index+len(gitDelimiter)+len(repoSpec.RepoPath):]) - 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 + var err error + + // Parse the host (e.g. scheme, username, domain) segment. + repoSpec.Host, n, err = extractHost(n) + if err != nil { + return nil, err } - if isLocal { - if idx := strings.Index(n, "//"); idx > 0 { - repoSpec.RepoPath = n[:idx] - n = n[idx+2:] - repoSpec.KustRootPath = parsePath(n) - return repoSpec - } - repoSpec.RepoPath = parsePath(n) - return repoSpec + // In some cases, we're given a path to a git repo + a path to the kustomization root within + // that repo. We need to split them so that we can ultimately give the repo only to the cloner. + repoSpec.RepoPath, repoSpec.KustRootPath, err = parsePathParts(n, defaultRepoPathLength(repoSpec.Host)) + if err != nil { + return nil, err } - i := strings.Index(n, "/") - if i < 1 { - repoSpec.KustRootPath = parsePath(n) - return repoSpec + // If the repo name ends in .git, isolate it. It will be added back by the clone spec function. + if idx := strings.Index(repoSpec.RepoPath, gitSuffix); idx >= 0 { + repoSpec.GitSuffix = gitSuffix + repoSpec.RepoPath = repoSpec.RepoPath[:idx] } - j := strings.Index(n[i+1:], "/") - if j >= 0 { - j += i + 1 - repoSpec.RepoPath = n[:j] - repoSpec.KustRootPath = parsePath(n[j+1:]) - return repoSpec + // Force the .git suffix URLs for services whose clone URL is the repo URL + .git. + // This allows us to support the repo URL as an input instead of the actual clone URL. + if legacyAddGitSuffix(repoSpec.Host, repoSpec.RepoPath) { + repoSpec.GitSuffix = gitSuffix } - repoSpec.KustRootPath = "" - repoSpec.RepoPath = parsePath(n) - return repoSpec + + return repoSpec, nil +} + +// 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 { - 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) { +func extractHost(n string) (string, string, error) { 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 "", n + return "", "", err } // 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://" { - return scheme, username + n + if scheme == fileScheme { + 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 "", n + return "", "", errors.Errorf("failed to parse host segment") } - 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/" -} - -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 + return httpsScheme, "", "github.com/" } // The format of Azure repo URL is documented diff --git a/api/internal/git/repospec_test.go b/api/internal/git/repospec_test.go index 84a878164..aec3c31ae 100644 --- a/api/internal/git/repospec_test.go +++ b/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/", - RepoPath: "somerepos", + Host: "https://itfs.mycompany.com/", + 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/", - RepoPath: "somerepos", + Host: "https://itfs.mycompany.com/", + 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/", - RepoPath: "somerepos", - KustRootPath: "/somedir", + Host: "https://itfs.mycompany.com/", + RepoPath: "collection/project/_git/somerepos", + 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/", - RepoPath: "somerepos", + Host: "https://itfs.mycompany.com/", + 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", - skip: "the `//` repo separator does not work", + name: "double-slash path delimiter https", 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", - skip: "the `//` repo separator does not work", + name: "double-slash path delimeter ssh", 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") diff --git a/api/krusty/remoteloader_test.go b/api/krusty/remoteloader_test.go index 5a7950dab..ac458ead0 100644 --- a/api/krusty/remoteloader_test.go +++ b/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, },