git: accept explicit commit hash for git context (#1765)

* git: accept explicit commit hash for git context

When checking out code from non-github repositories, the typical
assumptions may not be valid, e.g. that the only interesting
non-branch commits have ref names starting with refs/pull. A specific
example is fetching an un-merged commit from a gerrit repository by
commit hash.

This change just looks at the second part of the git context path and
checks if it's a SHA commit hash, and if so, will fetch and check out
this commit after cloning the repository.

Sample context argument:

    https://github.repo/project#e1772f228e06d15facdf175e5385e265b57068c0

* ci: fix test script to recognize any non-zero exit as an error

hack/linter.sh didn't properly install golangci-lint in hack/bin as I
already have another version of golangci-lint on my PATH, but then it
failed to execute because it was looking for it specifically in
hack/bin.

When the executable is not found, the exit code is 127 instead of 1,
and so test.sh ignored the error.

Two fixes:

1. `test.sh`:
  - Use `if (script) ...` instead of assigning / checking a result
    variable to determine if each validation script passed or failed.

2. `hack/linter.sh`:
  - Instead of checking for golangci-lint on the path, just
    specifically check for an executable file (`test -x`) in the
    expected location.

Co-authored-by: Wade Carpenter <wwade@users.noreply.github.com>

hack/linter.sh

@@ -16,12 +16,12 @@
 
 set -e -o pipefail
 
-DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )
 BIN=${DIR}/bin
 
-if ! [ -x "$(command -v golangci-lint)" ]; then
+if [ ! -x "${BIN}/golangci-lint" ]; then
 	echo "Installing GolangCI-Lint"
-	${DIR}/install_golint.sh -b ${BIN} v1.23.7
+	"${DIR}/install_golint.sh" -b "${BIN}" v1.23.7
 fi
 
-${BIN}/golangci-lint run
+"${BIN}/golangci-lint" run

integration/integration_test.go

@@ -30,6 +30,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/go-git/go-git/v5"
+	gitConfig "github.com/go-git/go-git/v5/config"
+	"github.com/go-git/go-git/v5/plumbing"
+	"github.com/go-git/go-git/v5/storage/memory"
 	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/google/go-containerregistry/pkg/v1/daemon"
 	"github.com/pkg/errors"
@@ -205,22 +209,59 @@ func TestRun(t *testing.T) {
 	}
 }
 
-func getGitRepo() string {
+func findSHA(ref plumbing.ReferenceName, refs []*plumbing.Reference) (string, error) {
-	var branch, repoSlug string
+	for _, ref2 := range refs {
-	if os.Getenv("TRAVIS_PULL_REQUEST") != "" {
+		if ref.String() == ref2.Name().String() {
+			return ref2.Hash().String(), nil
+		}
+	}
+	return "", errors.New("no ref found")
+}
+
+// getBranchSHA get a SHA commit hash for the given repo url and branch ref name.
+func getBranchSHA(t *testing.T, url, branch string) string {
+	repo := "https://" + url
+	c := &gitConfig.RemoteConfig{URLs: []string{repo}}
+	remote := git.NewRemote(memory.NewStorage(), c)
+	refs, err := remote.List(&git.ListOptions{})
+	if err != nil {
+		t.Fatalf("list remote %s#%s: %s", repo, branch, err)
+	}
+	commit, err := findSHA(plumbing.NewBranchReferenceName(branch), refs)
+	if err != nil {
+		t.Fatalf("findSHA %s#%s: %s", repo, branch, err)
+	}
+	return commit
+}
+
+func getBranchAndURL() (branch, url string) {
+	var repoSlug string
+	if _, ok := os.LookupEnv("TRAVIS_PULL_REQUEST"); ok {
 		branch = "master"
 		repoSlug = os.Getenv("TRAVIS_REPO_SLUG")
 		log.Printf("Travis CI Pull request source repo: %s branch: %s\n", repoSlug, branch)
-	} else {
+	} else if _, ok := os.LookupEnv("TRAVIS_BRANCH"); ok {
 		branch = os.Getenv("TRAVIS_BRANCH")
 		repoSlug = os.Getenv("TRAVIS_REPO_SLUG")
 		log.Printf("Travis CI repo: %s branch: %s\n", repoSlug, branch)
+	} else {
+		branch = "master"
+		repoSlug = "GoogleContainerTools/kaniko"
 	}
-	return "github.com/" + repoSlug + "#refs/heads/" + branch
+	url = "github.com/" + repoSlug
+	return
 }
 
-func TestGitBuildcontext(t *testing.T) {
+func getGitRepo(t *testing.T, explicit bool) string {
-	repo := getGitRepo()
+	branch, url := getBranchAndURL()
+	if explicit {
+		return url + "#" + getBranchSHA(t, url, branch)
+	}
+	return url + "#refs/heads/" + branch
+}
+
+func testGitBuildcontextHelper(t *testing.T, repo string) {
+	t.Log("testGitBuildcontextHelper repo", repo)
 	dockerfile := fmt.Sprintf("%s/%s/Dockerfile_test_run_2", integrationPath, dockerfilesPath)
 
 	// Build with docker
@@ -257,8 +298,32 @@ func TestGitBuildcontext(t *testing.T) {
 	checkContainerDiffOutput(t, diff, expected)
 }
 
+// TestGitBuildcontext explicitly names the master branch
+// Example:
+//   git://github.com/myuser/repo#refs/heads/master
+func TestGitBuildcontext(t *testing.T) {
+	repo := getGitRepo(t, false)
+	testGitBuildcontextHelper(t, repo)
+}
+
+// TestGitBuildcontextNoRef builds without any commit / branch reference
+// Example:
+//   git://github.com/myuser/repo
+func TestGitBuildcontextNoRef(t *testing.T) {
+	_, repo := getBranchAndURL()
+	testGitBuildcontextHelper(t, repo)
+}
+
+// TestGitBuildcontextExplicitCommit uses an explicit commit hash instead of named reference
+// Example:
+//   git://github.com/myuser/repo#b873088c4a7b60bb7e216289c58da945d0d771b6
+func TestGitBuildcontextExplicitCommit(t *testing.T) {
+	repo := getGitRepo(t, true)
+	testGitBuildcontextHelper(t, repo)
+}
+
 func TestGitBuildcontextSubPath(t *testing.T) {
-	repo := getGitRepo()
+	repo := getGitRepo(t, false)
 	dockerfile := "Dockerfile_test_run_2"
 
 	// Build with docker
@@ -267,8 +332,8 @@ func TestGitBuildcontextSubPath(t *testing.T) {
 		append([]string{
 			"build",
 			"-t", dockerImage,
-			"-f", dockerfile,
+			"-f", filepath.Join(integrationPath, dockerfilesPath, dockerfile),
-			repo + ":" + filepath.Join(integrationPath, dockerfilesPath),
+			repo,
 		})...)
 	out, err := RunCommandWithoutTest(dockerCmd)
 	if err != nil {
@@ -302,7 +367,7 @@ func TestGitBuildcontextSubPath(t *testing.T) {
 }
 
 func TestBuildViaRegistryMirrors(t *testing.T) {
-	repo := getGitRepo()
+	repo := getGitRepo(t, false)
 	dockerfile := fmt.Sprintf("%s/%s/Dockerfile_registry_mirror", integrationPath, dockerfilesPath)
 
 	// Build with docker
@@ -342,7 +407,7 @@ func TestBuildViaRegistryMirrors(t *testing.T) {
 }
 
 func TestBuildWithLabels(t *testing.T) {
-	repo := getGitRepo()
+	repo := getGitRepo(t, false)
 	dockerfile := fmt.Sprintf("%s/%s/Dockerfile_test_label", integrationPath, dockerfilesPath)
 
 	testLabel := "mylabel=myvalue"
@@ -385,7 +450,7 @@ func TestBuildWithLabels(t *testing.T) {
 }
 
 func TestBuildWithHTTPError(t *testing.T) {
-	repo := getGitRepo()
+	repo := getGitRepo(t, false)
 	dockerfile := fmt.Sprintf("%s/%s/Dockerfile_test_add_404", integrationPath, dockerfilesPath)
 
 	// Build with docker

pkg/buildcontext/git.go

@@ -17,6 +17,7 @@ limitations under the License.
 package buildcontext
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -66,8 +67,14 @@ func (g *Git) UnpackTarFromBuildContext() (string, error) {
 		SingleBranch:      g.opts.GitSingleBranch,
 		RecurseSubmodules: getRecurseSubmodules(g.opts.GitRecurseSubmodules),
 	}
+	var fetchRef string
 	if len(parts) > 1 {
-		if !strings.HasPrefix(parts[1], "refs/pull/") {
+		if plumbing.IsHash(parts[1]) || !strings.HasPrefix(parts[1], "refs/pull/") {
+			// Handle any non-branch refs separately. First, clone the repo HEAD, and
+			// then fetch and check out the fetchRef.
+			fetchRef = parts[1]
+		} else {
+			// Branches will be cloned directly.
 			options.ReferenceName = plumbing.ReferenceName(parts[1])
 		}
 	}
@@ -82,23 +89,25 @@ func (g *Git) UnpackTarFromBuildContext() (string, error) {
 
 	logrus.Debugf("Getting source from reference %s", options.ReferenceName)
 	r, err := git.PlainClone(directory, false, &options)
-
 	if err != nil {
 		return directory, err
 	}
 
-	if len(parts) > 1 && strings.HasPrefix(parts[1], "refs/pull/") {
+	if fetchRef != "" {
-
 		err = r.Fetch(&git.FetchOptions{
 			RemoteName: "origin",
-			RefSpecs:   []config.RefSpec{config.RefSpec(parts[1] + ":" + parts[1])},
+			RefSpecs:   []config.RefSpec{config.RefSpec(fetchRef + ":" + fetchRef)},
 		})
-		if err != nil {
+		if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) {
 			return directory, err
 		}
 	}
 
+	checkoutRef := fetchRef
 	if len(parts) > 2 {
+		checkoutRef = parts[2]
+	}
+	if checkoutRef != "" {
 		// ... retrieving the commit being pointed by HEAD
 		_, err := r.Head()
 		if err != nil {
@@ -112,13 +121,13 @@ func (g *Git) UnpackTarFromBuildContext() (string, error) {
 
 		// ... checking out to desired commit
 		err = w.Checkout(&git.CheckoutOptions{
-			Hash: plumbing.NewHash(parts[2]),
+			Hash: plumbing.NewHash(checkoutRef),
 		})
 		if err != nil {
 			return directory, err
 		}
 	}
-	return directory, err
+	return directory, nil
 }
 
 func getGitReferenceName(directory string, url string, branch string) (plumbing.ReferenceName, error) {

scripts/test.sh

@@ -37,15 +37,11 @@ fail=0
 for s in "${scripts[@]}"
 do
     echo "RUN ${s}"
-    set +e
+    if "./${s}"; then
-    ./$s
+        echo -e "${GREEN}PASSED${RESET} ${s}"
-    result=$?
+    else
-    set -e
-    if [[ $result  -eq 1 ]]; then
         echo -e "${RED}FAILED${RESET} ${s}"
         fail=1
-    else
-        echo -e "${GREEN}PASSED${RESET} ${s}"
     fi
 done
 exit $fail