diff --git a/api/internal/git/repospec.go b/api/internal/git/repospec.go index a1c29e6c71..06b0552056 100644 --- a/api/internal/git/repospec.go +++ b/api/internal/git/repospec.go @@ -7,6 +7,7 @@ import ( "fmt" "net/url" "path/filepath" + "regexp" "strconv" "strings" "time" @@ -86,17 +87,17 @@ func NewRepoSpecFromURL(n string) (*RepoSpec, error) { if filepath.IsAbs(n) { return nil, fmt.Errorf("uri looks like abs path: %s", n) } - host, orgRepo, path, gitRef, gitSubmodules, suffix, gitTimeout := parseGitURL(n) - if orgRepo == "" { + rs, err := parseGitURL(n) + if err != nil { + return nil, err + } + if rs.OrgRepo == "" { return nil, fmt.Errorf("url lacks orgRepo: %s", n) } - if host == "" { + if rs.Host == "" { return nil, fmt.Errorf("url lacks host: %s", n) } - return &RepoSpec{ - raw: n, Host: host, OrgRepo: orgRepo, - Dir: notCloned, Path: path, Ref: gitRef, GitSuffix: suffix, - Submodules: gitSubmodules, Timeout: gitTimeout}, nil + return rs, nil } const ( @@ -108,61 +109,68 @@ const ( // From strings like git@github.com:someOrg/someRepo.git or // https://github.com/someOrg/someRepo?ref=someHash, extract // the parts. -func parseGitURL(n string) ( - host string, orgRepo string, path string, gitRef string, gitSubmodules bool, gitSuff string, gitTimeout time.Duration) { +func parseGitURL(n string) (*RepoSpec, error) { + var err error + rs := &RepoSpec{raw: n, Dir: notCloned} if strings.Contains(n, gitDelimiter) { index := strings.Index(n, gitDelimiter) // Adding _git/ to host - host = normalizeGitHostSpec(n[:index+len(gitDelimiter)]) - orgRepo = strings.Split(strings.Split(n[index+len(gitDelimiter):], "/")[0], "?")[0] - path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[index+len(gitDelimiter)+len(orgRepo):]) - return + rs.Host, err = normalizeGitHostSpec(n[:index+len(gitDelimiter)]) + if err != nil { + return nil, err + } + rs.OrgRepo = strings.Split(strings.Split(n[index+len(gitDelimiter):], "/")[0], "?")[0] + rs.Path, rs.Ref, rs.Timeout, rs.Submodules = peelQuery(n[index+len(gitDelimiter)+len(rs.OrgRepo):]) + return rs, nil } - host, n = parseHostSpec(n) - isLocal := strings.HasPrefix(host, "file://") + rs.Host, n, err = parseHostSpec(n) + if err != nil { + return nil, err + } + isLocal := strings.HasPrefix(rs.Host, "file://") if !isLocal { - gitSuff = gitSuffix + rs.GitSuffix = gitSuffix } if strings.Contains(n, gitSuffix) { - gitSuff = gitSuffix + rs.GitSuffix = gitSuffix index := strings.Index(n, gitSuffix) - orgRepo = n[0:index] + rs.OrgRepo = n[0:index] n = n[index+len(gitSuffix):] if len(n) > 0 && n[0] == '/' { n = n[1:] } - path, gitRef, gitTimeout, gitSubmodules = peelQuery(n) - return + rs.Path, rs.Ref, rs.Timeout, rs.Submodules = peelQuery(n) + return rs, nil } if isLocal { if idx := strings.Index(n, "//"); idx > 0 { - orgRepo = n[:idx] + rs.OrgRepo = n[:idx] n = n[idx+2:] - path, gitRef, gitTimeout, gitSubmodules = peelQuery(n) - return + rs.Path, rs.Ref, rs.Timeout, rs.Submodules = peelQuery(n) + return rs, nil } - path, gitRef, gitTimeout, gitSubmodules = peelQuery(n) - orgRepo = path - path = "" - return + rs.Path, rs.Ref, rs.Timeout, rs.Submodules = peelQuery(n) + rs.OrgRepo = rs.Path + rs.Path = "" + return rs, nil } i := strings.Index(n, "/") if i < 1 { - path, gitRef, gitTimeout, gitSubmodules = peelQuery(n) - return + rs.Path, rs.Ref, rs.Timeout, rs.Submodules = peelQuery(n) + return rs, nil } j := strings.Index(n[i+1:], "/") if j >= 0 { j += i + 1 - orgRepo = n[:j] - path, gitRef, gitTimeout, gitSubmodules = peelQuery(n[j+1:]) - return + rs.OrgRepo = n[:j] + rs.Path, rs.Ref, rs.Timeout, rs.Submodules = peelQuery(n[j+1:]) + return rs, nil } - path = "" - orgRepo, gitRef, gitTimeout, gitSubmodules = peelQuery(n) - return host, orgRepo, path, gitRef, gitSubmodules, gitSuff, gitTimeout + rs.Path = "" + rs.OrgRepo, rs.Ref, rs.Timeout, rs.Submodules = peelQuery(n) + return rs, nil } // Clone git submodules by default. @@ -212,18 +220,26 @@ func peelQuery(arg string) (string, string, time.Duration, bool) { return parsed.Path, ref, duration, submodules } -func parseHostSpec(n string) (string, string) { +var userRegexp = regexp.MustCompile(`^([a-zA-Z][a-zA-Z0-9-]*)@`) + +func parseHostSpec(n string) (string, string, error) { var host string - // Start accumulating the host part. - for _, p := range []string{ - // Order matters here. - "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):] - host += p + consumeHostStrings := func(parts []string) { + for _, p := range parts { + if len(p) < len(n) && strings.ToLower(n[:len(p)]) == p { + n = n[len(p):] + host += p + } } } + // Start accumulating the host part. + // Order matters here. + consumeHostStrings([]string{"git::", "gh:", "ssh://", "https://", "http://", "file://"}) + if p := userRegexp.FindString(n); p != "" { + n = n[len(p):] + host += p + } + consumeHostStrings([]string{"github.com:", "github.com/"}) if host == "git@" { i := strings.Index(n, "/") if i > -1 { @@ -236,7 +252,7 @@ func parseHostSpec(n string) (string, string) { n = n[i+1:] } } - return host, n + return host, n, nil } // If host is a http(s) or ssh URL, grab the domain part. @@ -252,22 +268,50 @@ func parseHostSpec(n string) (string, string) { } } - return normalizeGitHostSpec(host), n + host, err := normalizeGitHostSpec(host) + return host, n, err } -func normalizeGitHostSpec(host string) string { +var githubRegexp = regexp.MustCompile(`^(?:ssh://)?([a-zA-Z][a-zA-Z0-9-]*)@(github.com[:/]?)`) + +func normalizeGitHostSpec(host string) (string, error) { s := strings.ToLower(host) - if strings.Contains(s, "github.com") { - if strings.Contains(s, "git@") || strings.Contains(s, "ssh:") { - host = "git@github.com:" - } else { - host = "https://github.com/" - } - } - if strings.HasPrefix(s, "git::") { + + // The git:: syntax is meant to force the Git protocol (separate from SSH + // and HTTPS), but we drop it here, to preserve past behavior. + isGitProtocol := strings.HasPrefix(s, "git::") + if isGitProtocol { host = strings.TrimPrefix(s, "git::") } - return host + + // Special treatment for github.com + if strings.Contains(host, "github.com") { + m := githubRegexp.FindStringSubmatch(host) + if m == nil { + return "https://github.com/", nil + } + userName, realHost := m[1], m[2] + + if realHost == "github.com/" { + realHost = "github.com:" + } + + const gitUser = "git" + isGitUser := userName == gitUser || userName == "" + if userName == "" { + userName = gitUser + } + + switch { + case isGitProtocol && !isGitUser: + return "", fmt.Errorf("git protocol on github.com only allows git@ user") + case isGitProtocol: + return "git@github.com:", nil + default: + return fmt.Sprintf("%s@%s", userName, realHost), nil + } + } + return host, nil } // 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 ad63b4aa02..80c3b8a4b0 100644 --- a/api/internal/git/repospec_test.go +++ b/api/internal/git/repospec_test.go @@ -35,6 +35,9 @@ func TestNewRepoSpecFromUrl_Permute(t *testing.T) { {"git::https://git.example.com/", "https://git.example.com/"}, {"git@github.com:", "git@github.com:"}, {"git@github.com/", "git@github.com:"}, + {"org-12345@github.com:", "org-12345@github.com:"}, + {"org-12345@github.com/", "org-12345@github.com:"}, + {"git::git@github.com:", "git@github.com:"}, } var orgRepos = []string{"someOrg/someRepo", "kubernetes/website"} var pathNames = []string{"README.md", "foo/krusty.txt", ""} @@ -99,16 +102,19 @@ func TestNewRepoSpecFromUrlErrors(t *testing.T) { {"htxxxtp://github.com/", "url lacks host"}, {"ssh://git.example.com", "url lacks orgRepo"}, {"git::___", "url lacks orgRepo"}, + {"git::org-12345@github.com:kubernetes-sigs/kustomize", "git protocol on github.com only allows git@ user"}, } for _, testCase := range badData { - _, err := NewRepoSpecFromURL(testCase.url) - if err == nil { - t.Error("expected error") - } - if !strings.Contains(err.Error(), testCase.error) { - t.Errorf("unexpected error: %s", err) - } + t.Run(testCase.error, func(t *testing.T) { + _, err := NewRepoSpecFromURL(testCase.url) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), testCase.error) { + t.Errorf("unexpected error: %s", err) + } + }) } } @@ -407,6 +413,51 @@ func TestNewRepoSpecFromUrl_Smoke(t *testing.T) { GitSuffix: ".git", }, }, + { + name: "t25", + input: "https://org-12345@github.com/kubernetes-sigs/kustomize", + cloneSpec: "https://github.com/kubernetes-sigs/kustomize.git", + absPath: notCloned.String(), + repoSpec: RepoSpec{ + Host: "https://github.com/", + OrgRepo: "kubernetes-sigs/kustomize", + GitSuffix: ".git", + }, + }, + { + name: "t26", + input: "ssh://org-12345@github.com/kubernetes-sigs/kustomize", + cloneSpec: "org-12345@github.com:kubernetes-sigs/kustomize.git", + absPath: notCloned.String(), + repoSpec: RepoSpec{ + Host: "org-12345@github.com:", + OrgRepo: "kubernetes-sigs/kustomize", + GitSuffix: ".git", + }, + }, + { + name: "t27", + input: "org-12345@github.com/kubernetes-sigs/kustomize", + cloneSpec: "org-12345@github.com:kubernetes-sigs/kustomize.git", + absPath: notCloned.String(), + repoSpec: RepoSpec{ + Host: "org-12345@github.com:", + OrgRepo: "kubernetes-sigs/kustomize", + GitSuffix: ".git", + }, + }, + { + name: "t28", + input: "git@github.com/someorg/somerepo/somepath", + cloneSpec: "git@github.com:someorg/somerepo.git", + absPath: notCloned.Join("somepath"), + repoSpec: RepoSpec{ + Host: "git@github.com:", + OrgRepo: "someorg/somerepo", + Path: "somepath", + GitSuffix: ".git", + }, + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) {