Merge pull request #730 from monopole/initializeGitPackage

Move git code to its own pkg.

pkg/git/cloner.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package loader
+package git
 
 import (
 	"bytes"
@@ -27,8 +27,8 @@ import (
 	"github.com/pkg/errors"
 )
 
-// gitCloner is a function that can clone a git repo.
-type gitCloner func(url string) (
+// Cloner is a function that can clone a git repo.
+type Cloner func(url string) (
 	// Directory where the repo is cloned to.
 	checkoutDir string,
 	// Relative path in the checkoutDir to location
@@ -37,8 +37,8 @@ type gitCloner func(url string) (
 	// Any error encountered when cloning.
 	err error)
 
-// isRepoUrl checks if a string is likely a github repo Url.
-func isRepoUrl(arg string) bool {
+// IsRepoUrl checks if a string is likely a github repo Url.
+func IsRepoUrl(arg string) bool {
 	arg = strings.ToLower(arg)
 	return !filepath.IsAbs(arg) &&
 		(strings.HasPrefix(arg, "git::") ||
@@ -54,7 +54,7 @@ func makeTmpDir() (string, error) {
 	return ioutil.TempDir("", "kustomize-")
 }
 
-func simpleGitCloner(spec string) (
+func ClonerUsingGitExec(spec string) (
 	checkoutDir string, pathInCoDir string, err error) {
 	gitProgram, err := exec.LookPath("git")
 	if err != nil {

pkg/git/cloner_test.go

@@ -14,16 +14,12 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package loader
+package git
 
 import (
 	"fmt"
 	"path/filepath"
-	"strings"
 	"testing"
-
-	"sigs.k8s.io/kustomize/pkg/constants"
-	"sigs.k8s.io/kustomize/pkg/fs"
 )
 
 func TestIsRepoURL(t *testing.T) {
@@ -98,107 +94,13 @@ func TestIsRepoURL(t *testing.T) {
 		},
 	}
 	for _, tc := range testcases {
-		actual := isRepoUrl(tc.input)
+		actual := IsRepoUrl(tc.input)
 		if actual != tc.expected {
 			t.Errorf("unexpected error: unexpected result %t for input %s", actual, tc.input)
 		}
 	}
 }
 
-func splitOnNthSlash(v string, n int) (string, string) {
-	left := ""
-	for i := 0; i < n; i++ {
-		k := strings.Index(v, "/")
-		if k < 0 {
-			break
-		}
-		left = left + v[:k+1]
-		v = v[k+1:]
-	}
-	return left[:len(left)-1], v
-}
-
-func TestSplit(t *testing.T) {
-	path := "a/b/c/d/e/f/g"
-	if left, right := splitOnNthSlash(path, 2); left != "a/b" || right != "c/d/e/f/g" {
-		t.Fatalf("got left='%s', right='%s'", left, right)
-	}
-	if left, right := splitOnNthSlash(path, 3); left != "a/b/c" || right != "d/e/f/g" {
-		t.Fatalf("got left='%s', right='%s'", left, right)
-	}
-	if left, right := splitOnNthSlash(path, 6); left != "a/b/c/d/e/f" || right != "g" {
-		t.Fatalf("got left='%s', right='%s'", left, right)
-	}
-}
-
-// makeFakeGitCloner returns a cloner that ignores the
-// URL argument and returns a path in a fake file system
-// that should already hold the 'repo' contents.
-func makeFakeGitCloner(t *testing.T, fSys fs.FileSystem, coRoot string) gitCloner {
-	if !fSys.IsDir(coRoot) {
-		t.Fatalf("expecting a directory at '%s'", coRoot)
-	}
-	return func(url string) (
-		checkoutDir string, pathInCoDir string, err error) {
-		_, path := splitOnNthSlash(url, 3)
-		if !fSys.IsDir(coRoot + "/" + path) {
-			t.Fatalf("expecting a directory at '%s'/'%s'",
-				coRoot, path)
-		}
-		return coRoot, path, nil
-	}
-}
-
-func TestGitLoader(t *testing.T) {
-	rootUrl := "github.com/someOrg/someRepo"
-	pathInRepo := "foo/base"
-	url := rootUrl + "/" + pathInRepo
-	if !isRepoUrl(url) {
-		t.Fatalf("'%s' should be accepted as a repo url", url)
-	}
-
-	coRoot := "/tmp"
-	fSys := fs.MakeFakeFS()
-	fSys.MkdirAll(coRoot)
-	fSys.MkdirAll(coRoot + "/" + pathInRepo)
-	fSys.WriteFile(
-		coRoot+"/"+pathInRepo+"/"+constants.KustomizationFileNames[0],
-		[]byte(`
-whatever
-`))
-	l, err := newGitLoader(
-		url, fSys, nil,
-		makeFakeGitCloner(t, fSys, coRoot))
-	if err != nil {
-		t.Fatalf("unexpected err: %v\n", err)
-	}
-	if coRoot+"/"+pathInRepo != l.Root() {
-		t.Fatalf("expected root '%s', got '%s'\n",
-			coRoot+"/"+pathInRepo, l.Root())
-	}
-	if _, err = l.New(url); err == nil {
-		t.Fatalf("expected cycle error 1")
-	}
-	if _, err = l.New(rootUrl + "/" + "foo"); err == nil {
-		t.Fatalf("expected cycle error 2")
-	}
-
-	pathInRepo = "foo/overlay"
-	fSys.MkdirAll(coRoot + "/" + pathInRepo)
-	url = rootUrl + "/" + pathInRepo
-	if !isRepoUrl(url) {
-		t.Fatalf("'%s' should be accepted as a repo url", url)
-	}
-	l2, err := l.New(url)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	if coRoot+"/"+pathInRepo != l2.Root() {
-		t.Fatalf("expected root '%s', got '%s'\n",
-			coRoot+"/"+pathInRepo, l2.Root())
-	}
-}
-
 var repoNames = []string{"someOrg/someRepo", "kubernetes/website"}
 
 var paths = []string{"README.md", "foo/krusty.txt", ""}
@@ -231,7 +133,7 @@ func TestParseGithubUrl(t *testing.T) {
 					if hrefArg != "" {
 						input = input + refQuery + hrefArg
 					}
-					if !isRepoUrl(input) {
+					if !IsRepoUrl(input) {
 						t.Errorf("Should smell like github arg: %s\n", input)
 						continue
 					}

pkg/loader/fileloader.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	"sigs.k8s.io/kustomize/pkg/fs"
+	"sigs.k8s.io/kustomize/pkg/git"
 	"sigs.k8s.io/kustomize/pkg/ifc"
 )
 
@@ -91,7 +92,7 @@ type fileLoader struct {
 	// File system utilities.
 	fSys fs.FileSystem
 	// Used to clone repositories.
-	cloner gitCloner
+	cloner git.Cloner
 	// Used to clean up, as needed.
 	cleaner func() error
 }
@@ -113,18 +114,18 @@ func (l *fileLoader) Root() string {
 }
 
 func newLoaderOrDie(fSys fs.FileSystem, path string) *fileLoader {
-	l, err := newFileLoaderAt(
-		path, fSys, nil, simpleGitCloner)
+	l, err := newLoaderAtConfirmedDir(
+		path, fSys, nil, git.ClonerUsingGitExec)
 	if err != nil {
 		log.Fatalf("unable to make loader at '%s'; %v", path, err)
 	}
 	return l
 }
 
-// newFileLoaderAt returns a new fileLoader with given root.
-func newFileLoaderAt(
+// newLoaderAtConfirmedDir returns a new fileLoader with given root.
+func newLoaderAtConfirmedDir(
 	possibleRoot string, fSys fs.FileSystem,
-	referrer *fileLoader, cloner gitCloner) (*fileLoader, error) {
+	referrer *fileLoader, cloner git.Cloner) (*fileLoader, error) {
 	if possibleRoot == "" {
 		return nil, fmt.Errorf(
 			"loader root cannot be empty")
@@ -159,25 +160,25 @@ func (l *fileLoader) New(path string) (ifc.Loader, error) {
 	if path == "" {
 		return nil, fmt.Errorf("new root cannot be empty")
 	}
-	if isRepoUrl(path) {
+	if git.IsRepoUrl(path) {
 		// Avoid cycles.
 		if err := l.errIfPreviouslySeenUri(path); err != nil {
 			return nil, err
 		}
-		return newGitLoader(path, l.fSys, l.referrer, l.cloner)
+		return newLoaderAtGitClone(path, l.fSys, l.referrer, l.cloner)
 	}
 	if filepath.IsAbs(path) {
 		return nil, fmt.Errorf("new root '%s' cannot be absolute", path)
 	}
-	return newFileLoaderAt(
+	return newLoaderAtConfirmedDir(
 		l.root.Join(path), l.fSys, l, l.cloner)
 }
 
-// newGitLoader returns a new Loader pinned to a temporary
+// newLoaderAtGitClone returns a new Loader pinned to a temporary
 // directory holding a cloned git repo.
-func newGitLoader(
+func newLoaderAtGitClone(
 	uri string, fSys fs.FileSystem,
-	referrer *fileLoader, cloner gitCloner) (ifc.Loader, error) {
+	referrer *fileLoader, cloner git.Cloner) (ifc.Loader, error) {
 	tmpDirForRepo, pathInRepo, err := cloner(uri)
 	if err != nil {
 		return nil, err

pkg/loader/fileloader_test.go

@@ -25,6 +25,9 @@ import (
 	"strings"
 	"testing"
 
+	"sigs.k8s.io/kustomize/pkg/constants"
+	"sigs.k8s.io/kustomize/pkg/git"
+
 	"sigs.k8s.io/kustomize/pkg/fs"
 	"sigs.k8s.io/kustomize/pkg/ifc"
 )
@@ -61,17 +64,17 @@ func MakeFakeFs(td []testData) fs.FileSystem {
 	return fSys
 }
 
-func TestNewFileLoaderAt_DemandsDirectory(t *testing.T) {
+func TestNewLoaderAtConfirmedDir_DemandsDirectory(t *testing.T) {
 	fSys := MakeFakeFs(testCases)
-	_, err := newFileLoaderAt("/foo", fSys, nil, nil)
+	_, err := newLoaderAtConfirmedDir("/foo", fSys, nil, nil)
 	if err != nil {
 		t.Fatalf("Unexpected error - a directory should work.")
 	}
-	_, err = newFileLoaderAt("/foo/project", fSys, nil, nil)
+	_, err = newLoaderAtConfirmedDir("/foo/project", fSys, nil, nil)
 	if err != nil {
 		t.Fatalf("Unexpected error - a directory should work.")
 	}
-	_, err = newFileLoaderAt("/foo/project/fileA.yaml", fSys, nil, nil)
+	_, err = newLoaderAtConfirmedDir("/foo/project/fileA.yaml", fSys, nil, nil)
 	if err == nil {
 		t.Fatalf("Expected error - a file should not work.")
 	}
@@ -321,3 +324,97 @@ func TestRestrictedLoadingInRealLoader(t *testing.T) {
 		t.Fatalf("unexpected err: %v", err)
 	}
 }
+
+func splitOnNthSlash(v string, n int) (string, string) {
+	left := ""
+	for i := 0; i < n; i++ {
+		k := strings.Index(v, "/")
+		if k < 0 {
+			break
+		}
+		left = left + v[:k+1]
+		v = v[k+1:]
+	}
+	return left[:len(left)-1], v
+}
+
+func TestSplit(t *testing.T) {
+	path := "a/b/c/d/e/f/g"
+	if left, right := splitOnNthSlash(path, 2); left != "a/b" || right != "c/d/e/f/g" {
+		t.Fatalf("got left='%s', right='%s'", left, right)
+	}
+	if left, right := splitOnNthSlash(path, 3); left != "a/b/c" || right != "d/e/f/g" {
+		t.Fatalf("got left='%s', right='%s'", left, right)
+	}
+	if left, right := splitOnNthSlash(path, 6); left != "a/b/c/d/e/f" || right != "g" {
+		t.Fatalf("got left='%s', right='%s'", left, right)
+	}
+}
+
+// makeFakeGitCloner returns a cloner that ignores the
+// URL argument and returns a path in a fake file system
+// that should already hold the 'repo' contents.
+func makeFakeGitCloner(t *testing.T, fSys fs.FileSystem, coRoot string) git.Cloner {
+	if !fSys.IsDir(coRoot) {
+		t.Fatalf("expecting a directory at '%s'", coRoot)
+	}
+	return func(url string) (
+		checkoutDir string, pathInCoDir string, err error) {
+		_, path := splitOnNthSlash(url, 3)
+		if !fSys.IsDir(coRoot + "/" + path) {
+			t.Fatalf("expecting a directory at '%s'/'%s'",
+				coRoot, path)
+		}
+		return coRoot, path, nil
+	}
+}
+
+func TestNewLoaderAtGitClone(t *testing.T) {
+	rootUrl := "github.com/someOrg/someRepo"
+	pathInRepo := "foo/base"
+	url := rootUrl + "/" + pathInRepo
+	if !git.IsRepoUrl(url) {
+		t.Fatalf("'%s' should be accepted as a repo url", url)
+	}
+
+	coRoot := "/tmp"
+	fSys := fs.MakeFakeFS()
+	fSys.MkdirAll(coRoot)
+	fSys.MkdirAll(coRoot + "/" + pathInRepo)
+	fSys.WriteFile(
+		coRoot+"/"+pathInRepo+"/"+constants.KustomizationFileNames[0],
+		[]byte(`
+whatever
+`))
+	l, err := newLoaderAtGitClone(
+		url, fSys, nil,
+		makeFakeGitCloner(t, fSys, coRoot))
+	if err != nil {
+		t.Fatalf("unexpected err: %v\n", err)
+	}
+	if coRoot+"/"+pathInRepo != l.Root() {
+		t.Fatalf("expected root '%s', got '%s'\n",
+			coRoot+"/"+pathInRepo, l.Root())
+	}
+	if _, err = l.New(url); err == nil {
+		t.Fatalf("expected cycle error 1")
+	}
+	if _, err = l.New(rootUrl + "/" + "foo"); err == nil {
+		t.Fatalf("expected cycle error 2")
+	}
+
+	pathInRepo = "foo/overlay"
+	fSys.MkdirAll(coRoot + "/" + pathInRepo)
+	url = rootUrl + "/" + pathInRepo
+	if !git.IsRepoUrl(url) {
+		t.Fatalf("'%s' should be accepted as a repo url", url)
+	}
+	l2, err := l.New(url)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if coRoot+"/"+pathInRepo != l2.Root() {
+		t.Fatalf("expected root '%s', got '%s'\n",
+			coRoot+"/"+pathInRepo, l2.Root())
+	}
+}

pkg/loader/loader.go

@@ -19,15 +19,16 @@ package loader
 
 import (
 	"sigs.k8s.io/kustomize/pkg/fs"
+	"sigs.k8s.io/kustomize/pkg/git"
 	"sigs.k8s.io/kustomize/pkg/ifc"
 )
 
 // NewLoader returns a Loader.
 func NewLoader(root string, fSys fs.FileSystem) (ifc.Loader, error) {
-	if isRepoUrl(root) {
-		return newGitLoader(
-			root, fSys, nil, simpleGitCloner)
+	if git.IsRepoUrl(root) {
+		return newLoaderAtGitClone(
+			root, fSys, nil, git.ClonerUsingGitExec)
 	}
-	return newFileLoaderAt(
-		root, fSys, nil, simpleGitCloner)
+	return newLoaderAtConfirmedDir(
+		root, fSys, nil, git.ClonerUsingGitExec)
 }