From 02d2d38c216c8d175bb5ff6c7af3affdcbb2dbd9 Mon Sep 17 00:00:00 2001
From: jregan <jregan@google.com>
Date: Thu, 22 Nov 2018 10:53:14 -0800
Subject: [PATCH] Deal with branch spec in simpleGitCloner.

---
 pkg/loader/gitcloner.go      | 57 ++++++++++++++++++++++++-------
 pkg/loader/gitcloner_test.go | 66 +++++++++++++++++++++---------------
 2 files changed, 84 insertions(+), 39 deletions(-)

diff --git a/pkg/loader/gitcloner.go b/pkg/loader/gitcloner.go
index 1306e9f4a..9d4ef4381 100644
--- a/pkg/loader/gitcloner.go
+++ b/pkg/loader/gitcloner.go
@@ -63,20 +63,42 @@ func simpleGitCloner(spec string) (
 	if err != nil {
 		return
 	}
-	url, pathInCoDir, err := extractGithubRepoName(spec)
-	cmd := exec.Command(gitProgram, "clone", url, checkoutDir)
+	repo, pathInCoDir, gitRef, err := parseGithubUrl(spec)
+	if err != nil {
+		return
+	}
+	cmd := exec.Command(
+		gitProgram,
+		"clone",
+		"https://github.com/"+repo+".git",
+		checkoutDir)
 	var out bytes.Buffer
 	cmd.Stdout = &out
 	err = cmd.Run()
 	if err != nil {
-		return "", "", errors.Wrapf(err, "trouble cloning %s", spec)
+		return "", "",
+			errors.Wrapf(err, "trouble cloning %s", spec)
 	}
-	return
+	if gitRef == "" {
+		return
+	}
+	cmd = exec.Command(gitProgram, "checkout", gitRef)
+	cmd.Dir = checkoutDir
+	err = cmd.Run()
+	if err != nil {
+		return "", "",
+			errors.Wrapf(err, "trouble checking out href %s", gitRef)
+	}
+	return checkoutDir, pathInCoDir, nil
 }
 
+const refQuery = "?ref="
+
 // From strings like git@github.com:someOrg/someRepo.git or
-// https://github.com/someOrg/someRepo, extract path.
-func extractGithubRepoName(n string) (string, string, error) {
+// https://github.com/someOrg/someRepo?ref=someHash, extract
+// the parts.
+func parseGithubUrl(n string) (
+	repo string, path string, gitRef string, err error) {
 	for _, p := range []string{
 		// Order matters here.
 		"git::", "gh:", "https://", "http://",
@@ -90,15 +112,26 @@ func extractGithubRepoName(n string) (string, string, error) {
 	}
 	i := strings.Index(n, string(filepath.Separator))
 	if i < 1 {
-		return "", "", errors.New("no separator")
+		return "", "", "", errors.New("no separator")
 	}
 	j := strings.Index(n[i+1:], string(filepath.Separator))
-	if j < 0 {
-		// No path, so show entire repo.
-		return n, "", nil
+	if j >= 0 {
+		j += i + 1
+		repo = n[:j]
+		path, gitRef = peelQuery(n[j+1:])
+	} else {
+		path = ""
+		repo, gitRef = peelQuery(n)
 	}
-	j += i + 1
-	return n[:j], n[j+1:], nil
+	return
+}
+
+func peelQuery(arg string) (string, string) {
+	j := strings.Index(arg, refQuery)
+	if j >= 0 {
+		return arg[:j], arg[j+len(refQuery):]
+	}
+	return arg, ""
 }
 
 func hashicorpGitCloner(repoUrl string) (
diff --git a/pkg/loader/gitcloner_test.go b/pkg/loader/gitcloner_test.go
index fdd5c3194..984c25682 100644
--- a/pkg/loader/gitcloner_test.go
+++ b/pkg/loader/gitcloner_test.go
@@ -181,7 +181,9 @@ whatever
 
 var repoNames = []string{"someOrg/someRepo", "kubernetes/website"}
 
-var paths = []string{"", "README.md", "foo/index.md"}
+var paths = []string{"README.md", "foo/krusty.txt", ""}
+
+var hrefArgs = []string{"someBranch", ""}
 
 var extractFmts = []string{
 	"gh:%s",
@@ -190,38 +192,48 @@ var extractFmts = []string{
 	"https://github.com/%s",
 	"hTTps://github.com/%s",
 	"git::https://gitlab.com/%s",
-	"git@gitHUB.com:%s.git",
 	"github.com:%s",
 }
 
-func TestExtractGithubRepoName(t *testing.T) {
+func TestParseGithubUrl(t *testing.T) {
 	for _, repoName := range repoNames {
 		for _, pathName := range paths {
 			for _, extractFmt := range extractFmts {
-				spec := repoName
-				if len(pathName) > 0 {
-					spec = filepath.Join(spec, pathName)
-				}
-				input := fmt.Sprintf(extractFmt, spec)
-				if !isRepoUrl(input) {
-					t.Errorf("Should smell like github arg: %s\n", input)
-					continue
-				}
-				repo, path, err := extractGithubRepoName(input)
-				if err != nil {
-					t.Errorf("problem %v", err)
-				}
-				if repo != repoName {
-					t.Errorf("\n"+
-						"       from %s\n"+
-						"    gotRepo %s\n"+
-						"desiredRepo %s\n", input, repo, repoName)
-				}
-				if path != pathName {
-					t.Errorf("\n"+
-						"       from %s\n"+
-						"    gotPath %s\n"+
-						"desiredPath %s\n", input, path, pathName)
+				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
+					}
+					repo, path, gitRef, err := parseGithubUrl(input)
+					if err != nil {
+						t.Errorf("problem %v", err)
+					}
+					if repo != repoName {
+						t.Errorf("\n"+
+							"         from %s\n"+
+							"  actual Repo %s\n"+
+							"expected Repo %s\n", input, repo, repoName)
+					}
+					if path != pathName {
+						t.Errorf("\n"+
+							"         from %s\n"+
+							"  actual Path %s\n"+
+							"expected Path %s\n", input, path, pathName)
+					}
+					if gitRef != hrefArg {
+						t.Errorf("\n"+
+							"         from %s\n"+
+							"  actual Href %s\n"+
+							"expected Href %s\n", input, gitRef, hrefArg)
+					}
 				}
 			}
 		}