diff --git a/pkg/git/cloner.go b/pkg/git/cloner.go index 27e6ef712..40c61ffba 100644 --- a/pkg/git/cloner.go +++ b/pkg/git/cloner.go @@ -46,7 +46,7 @@ func ClonerUsingGitExec(spec string) (*RepoSpec, error) { cmd := exec.Command( gitProgram, "clone", - repoSpec.repo, + repoSpec.CloneSpec(), repoSpec.cloneDir.String()) var out bytes.Buffer cmd.Stdout = &out diff --git a/pkg/git/repospec.go b/pkg/git/repospec.go index 90b5f38cc..046d92be9 100644 --- a/pkg/git/repospec.go +++ b/pkg/git/repospec.go @@ -17,27 +17,30 @@ limitations under the License. package git import ( + "fmt" "path/filepath" - "regexp" "strings" - "github.com/pkg/errors" "sigs.k8s.io/kustomize/pkg/fs" ) -// RepoSpec specifies a git repo and a branch and path therein. +// RepoSpec specifies a git repository and a branch and path therein. type RepoSpec struct { // Raw, original spec, used to look for cycles. // TODO(monopole): Drop raw, use processed fields instead. raw string - // Repo, e.g. github.com/kubernetes-sigs/kustomize - repo string + // Host, e.g. github.com + host string - // ConfirmedDir where the repo is cloned to. + // orgRepo name (organization/repoName), + // e.g. kubernetes-sigs/kustomize + orgRepo string + + // ConfirmedDir where the orgRepo is cloned to. cloneDir fs.ConfirmedDir - // Relative path in the repo, and in the cloneDir, + // Relative path in the repository, and in the cloneDir, // to a Kustomization. path string @@ -45,6 +48,14 @@ type RepoSpec struct { ref string } +// CloneSpec returns a string suitable for "git clone {spec}". +func (x *RepoSpec) CloneSpec() string { + if isAzureHost(x.host) || isAWSHost(x.host) { + return x.host + x.orgRepo + } + return x.host + x.orgRepo + gitSuffix +} + func (x *RepoSpec) CloneDir() fs.ConfirmedDir { return x.cloneDir } @@ -74,17 +85,23 @@ func IsRepoUrl(arg string) bool { isAzureHost(arg) || isAWSHost(arg)) } +// From strings like git@github.com:someOrg/someRepo.git or +// https://github.com/someOrg/someRepo?ref=someHash, extract +// the parts. func NewRepoSpecFromUrl(n string) (*RepoSpec, error) { - host, repo, path, gitRef, err := parseGithubUrl(n) - if err != nil { - return nil, err + if filepath.IsAbs(n) { + return nil, fmt.Errorf("uri looks like abs path: %s", n) } - if isAzureHost(host) || isAWSHost(host) { - repo = host + repo - } else { - repo = host + repo + ".git" + host, orgRepo, path, gitRef := parseGithubUrl(n) + if orgRepo == "" { + return nil, fmt.Errorf("url lacks orgRepo: %s", n) } - return &RepoSpec{raw: n, repo: repo, path: path, ref: gitRef}, nil + if host == "" { + return nil, fmt.Errorf("url lacks host: %s", n) + } + return &RepoSpec{ + raw: n, host: host, orgRepo: orgRepo, + path: path, ref: gitRef}, nil } func NewRepoSpec( @@ -101,33 +118,29 @@ const ( // https://github.com/someOrg/someRepo?ref=someHash, extract // the parts. func parseGithubUrl(n string) ( - host string, repo string, path string, gitRef string, err error) { + host string, orgRepo string, path string, gitRef string) { host, n = parseHostSpec(n) - host = normalizeGitHostSpec(host) - if strings.HasSuffix(n, gitSuffix) { - repo = n[0 : len(n)-len(gitSuffix)] - return - } if strings.Contains(n, gitSuffix) { index := strings.Index(n, gitSuffix) - repo = n[0:index] + orgRepo = n[0:index] n = n[index+len(gitSuffix):] path, gitRef = peelQuery(n) return } + i := strings.Index(n, "/") if i < 1 { - return "", "", "", "", errors.New("no separator") + return "", "", "", "" } j := strings.Index(n[i+1:], "/") if j >= 0 { j += i + 1 - repo = n[:j] + orgRepo = n[:j] path, gitRef = peelQuery(n[j+1:]) } else { path = "" - repo, gitRef = peelQuery(n) + orgRepo, gitRef = peelQuery(n) } return } @@ -142,28 +155,45 @@ func peelQuery(arg string) (string, string) { func parseHostSpec(n string) (string, string) { var host string + // Start accumulating the host part. for _, p := range []string{ // Order matters here. "git::", "gh:", "ssh://", "https://", "http://", "git@", "github.com:", "github.com/"} { - if strings.ToLower(n[:len(p)]) == p { + if len(p) < len(n) && strings.ToLower(n[:len(p)]) == p { n = n[len(p):] - host = host + p + host += p } } + if host == "git@" { + i := strings.Index(n, "/") + if i > -1 { + host += n[:i+1] + n = n[i+1:] + } else { + i = strings.Index(n, ":") + if i > -1 { + host += n[:i+1] + n = n[i+1:] + } + } + return host, n + } // If host is a http(s) or ssh URL, grab the domain part. for _, p := range []string{ "ssh://", "https://", "http://"} { - if strings.HasSuffix(strings.ToLower(host), p) { - index := regexp.MustCompile("^(.*?)/").FindStringIndex(n) - if len(index) > 0 { - host = host + n[0:index[len(index)-1]] - n = n[index[len(index)-1]:] + if strings.HasSuffix(host, p) { + i := strings.Index(n, "/") + if i > -1 { + host = host + n[0:i+1] + n = n[i+1:] } + break } } - return host, n + + return normalizeGitHostSpec(host), n } func normalizeGitHostSpec(host string) string { diff --git a/pkg/git/repospec_test.go b/pkg/git/repospec_test.go index dd1c76fee..fca5f45ca 100644 --- a/pkg/git/repospec_test.go +++ b/pkg/git/repospec_test.go @@ -19,6 +19,7 @@ package git import ( "fmt" "path/filepath" + "strings" "testing" ) @@ -101,15 +102,16 @@ func TestIsRepoURL(t *testing.T) { } } -var repoNames = []string{"someOrg/someRepo", "kubernetes/website"} +var orgRepos = []string{"someOrg/someRepo", "kubernetes/website"} -var paths = []string{"README.md", "foo/krusty.txt", ""} +var pathNames = []string{"README.md", "foo/krusty.txt", ""} -var hrefArgs = []string{"someBranch", ""} +var hrefArgs = []string{"someBranch", "master", "v0.1.0", ""} -var extractFmts = map[string]string{ +var hostNamesRawAndNormalizedOld = map[string]string{ "gh:%s": "gh:", "GH:%s": "gh:", + "git@github.com:%s": "git@github.com:", "gitHub.com/%s": "https://github.com/", "https://github.com/%s": "https://github.com/", "hTTps://github.com/%s": "https://github.com/", @@ -118,40 +120,132 @@ var extractFmts = map[string]string{ "git::http://git.example.com/%s": "http://git.example.com/", "git::https://git.example.com/%s": "https://git.example.com/", "ssh://git.example.com:7999/%s": "ssh://git.example.com:7999/", + "https://git-codecommit.us-east-2.amazonaws.com/%s": "https://git-codecommit.us-east-2.amazonaws.com/", } -func TestParseGithubUrl(t *testing.T) { - for _, repoName := range repoNames { - for _, pathName := range paths { - for extractFmt, hostSpec := range extractFmts { +var hostNamesRawAndNormalized = [][]string{ + {"gh:", "gh:"}, + {"GH:", "gh:"}, + {"gitHub.com/", "https://github.com/"}, + {"github.com:", "https://github.com/"}, + {"http://github.com/", "https://github.com/"}, + {"https://github.com/", "https://github.com/"}, + {"hTTps://github.com/", "https://github.com/"}, + {"https://git-codecommit.us-east-2.amazonaws.com/", "https://git-codecommit.us-east-2.amazonaws.com/"}, + {"https://fabrikops2.visualstudio.com/", "https://fabrikops2.visualstudio.com/"}, + {"ssh://git.example.com:7999/", "ssh://git.example.com:7999/"}, + {"git::https://gitlab.com/", "https://gitlab.com/"}, + {"git::http://git.example.com/", "http://git.example.com/"}, + {"git::https://git.example.com/", "https://git.example.com/"}, + {"git@github.com:", "git@github.com:"}, + {"git@github.com/", "git@github.com:"}, + {"git@gitlab2.sqtools.ru:10022/", "git@gitlab2.sqtools.ru:10022/"}, +} + +func makeUrlOld(hostFmt, orgRepo, path, href string) string { + if len(path) > 0 { + orgRepo = filepath.Join(orgRepo, path) + } + url := fmt.Sprintf(hostFmt, orgRepo) + if href != "" { + url += refQuery + href + } + return url +} + +func makeUrl(hostFmt, orgRepo, path, href string) string { + if len(path) > 0 { + orgRepo = filepath.Join(orgRepo, path) + } + url := hostFmt + orgRepo + if href != "" { + url += refQuery + href + } + return url +} + +func TestNewRepoSpecFromUrl(t *testing.T) { + var bad [][]string + for _, tuple := range hostNamesRawAndNormalized { + hostRaw := tuple[0] + hostSpec := tuple[1] + for _, orgRepo := range orgRepos { + for _, pathName := range pathNames { for _, hrefArg := range hrefArgs { - spec := repoName - if len(pathName) > 0 { - spec = filepath.Join(spec, pathName) - } - input := fmt.Sprintf(extractFmt, spec) - if hrefArg != "" { - input = input + refQuery + hrefArg - } - if !IsRepoUrl(input) { - t.Errorf("Should smell like github arg: %s\n", input) - continue - } - host, repo, path, gitRef, err := parseGithubUrl(input) + uri := makeUrl(hostRaw, orgRepo, pathName, hrefArg) + rs, err := NewRepoSpecFromUrl(uri) if err != nil { t.Errorf("problem %v", err) } + if rs.host != hostSpec { + bad = append(bad, []string{"host", uri, rs.host, hostSpec}) + } + if rs.orgRepo != orgRepo { + bad = append(bad, []string{"orgRepo", uri, rs.orgRepo, orgRepo}) + } + if rs.path != pathName { + bad = append(bad, []string{"path", uri, rs.path, pathName}) + } + if rs.ref != hrefArg { + bad = append(bad, []string{"ref", uri, rs.ref, hrefArg}) + } + } + } + } + } + if len(bad) > 0 { + for _, tuple := range bad { + fmt.Printf("\n"+ + " from uri: %s\n"+ + " actual %4s: %s\n"+ + "expected %4s: %s\n", + tuple[1], tuple[0], tuple[2], tuple[0], tuple[3]) + } + t.Fail() + } +} + +var badData = [][]string{ + {"/tmp", "uri looks like abs path"}, + {"iauhsdiuashduas", "url lacks orgRepo"}, + {"htxxxtp://github.com/", "url lacks host"}, + {"ssh://git.example.com", "url lacks orgRepo"}, + {"git::___", "url lacks orgRepo"}, +} + +func TestNewRepoSpecFromUrlErrors(t *testing.T) { + for _, tuple := range badData { + _, err := NewRepoSpecFromUrl(tuple[0]) + if err == nil { + t.Error("expected error") + } + if !strings.Contains(err.Error(), tuple[1]) { + t.Errorf("unexpected error: %s", err) + } + } +} + +func TestParseGithubUrl(t *testing.T) { + for hostRawFmt, hostSpec := range hostNamesRawAndNormalizedOld { + for _, orgRepo := range orgRepos { + for _, pathName := range pathNames { + for _, hrefArg := range hrefArgs { + input := makeUrlOld(hostRawFmt, orgRepo, pathName, hrefArg) + if !IsRepoUrl(input) { + t.Errorf("Should smell like github arg: %s\n", input) + } + host, repo, path, gitRef := parseGithubUrl(input) if host != hostSpec { t.Errorf("\n"+ " from %s\n"+ " actual host %s\n"+ "expected host %s\n", input, host, hostSpec) } - if repo != repoName { + if repo != orgRepo { t.Errorf("\n"+ " from %s\n"+ " actual Repo %s\n"+ - "expected Repo %s\n", input, repo, repoName) + "expected Repo %s\n", input, repo, orgRepo) } if path != pathName { t.Errorf("\n"+ @@ -171,7 +265,7 @@ func TestParseGithubUrl(t *testing.T) { } } -func TestNewRepoSpecFromUrl(t *testing.T) { +func TestNewRepoSpecFromUrlOld(t *testing.T) { testcases := []struct { input string repo string @@ -220,9 +314,9 @@ func TestNewRepoSpecFromUrl(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %v", err) } - if rs.repo != testcase.repo { - t.Errorf("repo expected to be %v, but got %v on %s", - testcase.repo, rs.repo, testcase.input) + if rs.CloneSpec() != testcase.repo { + t.Errorf("CloneSpec expected to be %v, but got %v on %s", + testcase.repo, rs.CloneSpec(), testcase.input) } if rs.path != testcase.path { t.Errorf("path expected to be %v, but got %v on %s",