Merge pull request #4779 from annasong20/locloader

Reinstate #4652 without url-related code

api/ifc/ifc.go

@@ -28,12 +28,20 @@ type KvLoader interface {
 
 // Loader interface exposes methods to read bytes.
 type Loader interface {
+
+	// Repo returns the repo location and true if this Loader
+	// was created from a url; otherwise the empty string and false.
+	Repo() (string, bool)
+
 	// Root returns the root location for this Loader.
 	Root() string
+
 	// New returns Loader located at newRoot.
 	New(newRoot string) (Loader, error)
+
 	// Load returns the bytes read from the location or an error.
 	Load(location string) ([]byte, error)
+
 	// Cleanup cleans the loader
 	Cleanup() error
 }

api/internal/git/repospec.go

@@ -27,7 +27,7 @@ type RepoSpec struct {
 	// TODO(monopole): Drop raw, use processed fields instead.
 	raw string
 
-	// Host, e.g. github.com
+	// Host, e.g. https://github.com/
 	Host string
 
 	// orgRepo name (organization/repoName),

api/internal/localizer/doc.go

@@ -0,0 +1,7 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+// Package localizer contains utilities for the command kustomize localize, which is
+// documented under proposals/localize-command or at
+// https://github.com/kubernetes-sigs/kustomize/blob/master/proposals/22-04-localize-command.md
+package localizer

api/internal/localizer/locloader.go

@@ -0,0 +1,146 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package localizer
+
+import (
+	"log"
+	"path/filepath"
+
+	"sigs.k8s.io/kustomize/api/ifc"
+	"sigs.k8s.io/kustomize/api/internal/git"
+	"sigs.k8s.io/kustomize/api/loader"
+	"sigs.k8s.io/kustomize/kyaml/errors"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
+)
+
+const dstPrefix = "localized"
+
+// LocArgs holds localize arguments
+type LocArgs struct {
+	// target; local copy if remote
+	Target filesys.ConfirmedDir
+
+	// directory that bounds target's local references, empty string if target is remote
+	Scope filesys.ConfirmedDir
+
+	// localize destination
+	NewDir filesys.ConfirmedDir
+}
+
+// locLoader is the Loader for kustomize localize. It is an ifc.Loader that enforces localize constraints.
+type locLoader struct {
+	fSys filesys.FileSystem
+
+	args *LocArgs
+
+	// loader at locLoader's current directory
+	ifc.Loader
+
+	// whether locLoader and all its ancestors are the result of local references
+	local bool
+}
+
+var _ ifc.Loader = &locLoader{}
+
+// NewLocLoader is the factory method for Loader, under localize constraints, at targetArg. For invalid localize arguments,
+// NewLocLoader returns an error.
+func NewLocLoader(targetArg string, scopeArg string, newDirArg string, fSys filesys.FileSystem) (ifc.Loader, LocArgs, error) {
+	// check earlier to avoid cleanup
+	repoSpec, err := git.NewRepoSpecFromURL(targetArg)
+	if err == nil && repoSpec.Ref == "" {
+		return nil, LocArgs{},
+			errors.Errorf("localize remote root '%s' missing ref query string parameter", targetArg)
+	}
+
+	// for security, should enforce load restrictions
+	ldr, err := loader.NewLoader(loader.RestrictionRootOnly, targetArg, fSys)
+	if err != nil {
+		return nil, LocArgs{}, errors.WrapPrefixf(err, "unable to establish localize target '%s'", targetArg)
+	}
+
+	scope, err := establishScope(scopeArg, targetArg, ldr, fSys)
+	if err != nil {
+		_ = ldr.Cleanup()
+		return nil, LocArgs{}, errors.WrapPrefixf(err, "invalid localize scope '%s'", scopeArg)
+	}
+
+	newDir, err := createNewDir(newDirArg, ldr, repoSpec, fSys)
+	if err != nil {
+		_ = ldr.Cleanup()
+		return nil, LocArgs{}, errors.WrapPrefixf(err, "invalid localize destination '%s'", newDirArg)
+	}
+
+	args := LocArgs{
+		Target: filesys.ConfirmedDir(ldr.Root()),
+		Scope:  scope,
+		NewDir: newDir,
+	}
+	return &locLoader{
+		fSys:   fSys,
+		args:   &args,
+		Loader: ldr,
+		local:  scope != "",
+	}, args, nil
+}
+
+// Load returns the contents of path if path is a valid localize file.
+// Otherwise, Load returns an error.
+func (ll *locLoader) Load(path string) ([]byte, error) {
+	// checks in root, and thus in scope
+	content, err := ll.Loader.Load(path)
+	if err != nil {
+		return nil, errors.WrapPrefixf(err, "invalid file reference")
+	}
+	if filepath.IsAbs(path) {
+		return nil, errors.Errorf("absolute paths not yet supported in alpha: file path '%s' is absolute", path)
+	}
+	if ll.local {
+		abs := filepath.Join(ll.Root(), path)
+		dir, f, err := ll.fSys.CleanedAbs(abs)
+		if err != nil {
+			// should never happen
+			log.Fatalf(errors.WrapPrefixf(err, "cannot clean validated file path '%s'", abs).Error())
+		}
+		// target cannot reference newDir, as this load would've failed prior to localize;
+		// not a problem if remote because then reference could only be in newDir if repo copy,
+		// which will be cleaned, is inside newDir
+		if dir.HasPrefix(ll.args.NewDir) {
+			return nil, errors.Errorf(
+				"file path '%s' references into localize destination '%s'", dir.Join(f), ll.args.NewDir)
+		}
+	}
+	return content, nil
+}
+
+// New returns a Loader at path if path is a valid localize root.
+// Otherwise, New returns an error.
+func (ll *locLoader) New(path string) (ifc.Loader, error) {
+	repoSpec, err := git.NewRepoSpecFromURL(path)
+	if err == nil && repoSpec.Ref == "" {
+		return nil, errors.Errorf("localize remote root '%s' missing ref query string parameter", path)
+	}
+
+	ldr, err := ll.Loader.New(path)
+	if err != nil {
+		return nil, errors.WrapPrefixf(err, "invalid root reference")
+	}
+
+	var isRemote bool
+	if _, isRemote = ldr.Repo(); !isRemote {
+		if ll.local && !filesys.ConfirmedDir(ldr.Root()).HasPrefix(ll.args.Scope) {
+			return nil, errors.Errorf("root '%s' outside localize scope '%s'", ldr.Root(), ll.args.Scope)
+		}
+		if ll.local && filesys.ConfirmedDir(ldr.Root()).HasPrefix(ll.args.NewDir) {
+			return nil, errors.Errorf(
+				"root '%s' references into localize destination '%s'", ldr.Root(), ll.args.NewDir)
+		}
+	}
+
+	return &locLoader{
+		fSys:   ll.fSys,
+		args:   ll.args,
+		Loader: ldr,
+		local:  ll.local && !isRemote,
+	}, nil
+}

api/internal/localizer/locloader_test.go

@@ -0,0 +1,311 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package localizer_test
+
+import (
+	"bytes"
+	"log"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/kustomize/api/ifc"
+	lclzr "sigs.k8s.io/kustomize/api/internal/localizer"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
+)
+
+const dstPrefix = "localized"
+
+func makeMemoryFs(t *testing.T) filesys.FileSystem {
+	t.Helper()
+	req := require.New(t)
+
+	fSys := filesys.MakeFsInMemory()
+	req.NoError(fSys.MkdirAll("/a/b"))
+	req.NoError(fSys.WriteFile("/a/kustomization.yaml", []byte("/a")))
+
+	dirChain := "/alpha/beta/gamma/delta"
+	req.NoError(fSys.MkdirAll(dirChain))
+	req.NoError(fSys.WriteFile(dirChain+"/kustomization.yaml", []byte(dirChain)))
+	req.NoError(fSys.Mkdir("/alpha/beta/c"))
+	return fSys
+}
+
+func checkNewLocLoader(req *require.Assertions, ldr ifc.Loader, args *lclzr.LocArgs, target string, scope string, newDir string, fSys filesys.FileSystem) {
+	checkLoader(req, ldr, target)
+	checkLocArgs(req, args, target, scope, newDir, fSys)
+}
+
+func checkLoader(req *require.Assertions, ldr ifc.Loader, root string) {
+	req.Equal(root, ldr.Root())
+	repo, isRemote := ldr.Repo()
+	req.Equal(false, isRemote)
+	req.Equal("", repo)
+}
+
+func checkLocArgs(req *require.Assertions, args *lclzr.LocArgs, target string, scope string, newDir string, fSys filesys.FileSystem) {
+	req.Equal(target, args.Target.String())
+	req.Equal(scope, args.Scope.String())
+	req.Equal(newDir, args.NewDir.String())
+	req.True(fSys.Exists(newDir))
+}
+
+func TestLocalLoadNewAndCleanup(t *testing.T) {
+	req := require.New(t)
+	fSys := makeMemoryFs(t)
+
+	var buf bytes.Buffer
+	log.SetOutput(&buf)
+	// typical setup
+	ldr, args, err := lclzr.NewLocLoader("a", "/", "/newDir", fSys)
+	req.NoError(err)
+	checkNewLocLoader(req, ldr, &args, "/a", "/", "/newDir", fSys)
+
+	fSysCopy := makeMemoryFs(t)
+	req.NoError(fSysCopy.Mkdir("/newDir"))
+	req.Equal(fSysCopy, fSys)
+
+	// easy load directly in root
+	content, err := ldr.Load("kustomization.yaml")
+	req.NoError(err)
+	req.Equal([]byte("/a"), content)
+
+	// typical sibling root reference
+	sibLdr, err := ldr.New("../alpha")
+	req.NoError(err)
+	checkLoader(req, sibLdr, "/alpha")
+
+	// only need to test once, since don't need to call Cleanup() on local target
+	req.NoError(sibLdr.Cleanup())
+	req.NoError(ldr.Cleanup())
+
+	// file system and buffer checks are also one-time
+	req.Equal(fSysCopy, fSys)
+	req.Empty(buf.String())
+}
+
+func TestNewLocLoaderDefaultForRootTarget(t *testing.T) {
+	cases := map[string]struct {
+		target string
+		scope  string
+	}{
+		"explicit": {
+			"/",
+			".",
+		},
+		"implicit": {
+			".",
+			"",
+		},
+	}
+	for name, params := range cases {
+		params := params
+		t.Run(name, func(t *testing.T) {
+			req := require.New(t)
+			fSys := makeMemoryFs(t)
+
+			ldr, args, err := lclzr.NewLocLoader(params.target, params.scope, "", fSys)
+			req.NoError(err)
+			checkNewLocLoader(req, ldr, &args, "/", "/", "/"+dstPrefix, fSys)
+
+			// file in root, but nested
+			content, err := ldr.Load("a/kustomization.yaml")
+			req.NoError(err)
+			req.Equal([]byte("/a"), content)
+
+			childLdr, err := ldr.New("a")
+			req.NoError(err)
+			checkLoader(req, childLdr, "/a")
+
+			// messy, uncleaned path
+			content, err = childLdr.Load("./../a/kustomization.yaml")
+			req.NoError(err)
+			req.Equal([]byte("/a"), content)
+		})
+	}
+}
+
+func TestNewMultiple(t *testing.T) {
+	req := require.New(t)
+	fSys := makeMemoryFs(t)
+
+	// default destination for non-file system root target
+	// destination outside of scope
+	ldr, args, err := lclzr.NewLocLoader("/alpha/beta", "/alpha", "", fSys)
+	req.NoError(err)
+	checkNewLocLoader(req, ldr, &args, "/alpha/beta", "/alpha", "/"+dstPrefix+"-beta", fSys)
+
+	// nested child root that isn't cleaned
+	descLdr, err := ldr.New("../beta/gamma/delta")
+	req.NoError(err)
+	checkLoader(req, descLdr, "/alpha/beta/gamma/delta")
+
+	// upwards traversal
+	higherLdr, err := descLdr.New("../../c")
+	req.NoError(err)
+	checkLoader(req, higherLdr, "/alpha/beta/c")
+}
+
+func makeWdFs(t *testing.T) map[string]filesys.FileSystem {
+	t.Helper()
+	req := require.New(t)
+
+	root := filesys.MakeEmptyDirInMemory()
+	req.NoError(root.MkdirAll("a/b/c/d/e"))
+
+	outer, err := root.Find("a")
+	req.NoError(err)
+	middle, err := root.Find("a/b/c")
+	req.NoError(err)
+
+	return map[string]filesys.FileSystem{
+		"a":     outer,
+		"a/b/c": middle,
+	}
+}
+
+func TestNewLocLoaderCwdNotRoot(t *testing.T) {
+	cases := map[string]struct {
+		wd     string
+		target string
+		scope  string
+		newDir string
+	}{
+		// target not immediate child of scope
+		"outer dir": {
+			"a",
+			"b/c/d/e",
+			"b/c",
+			"b/newDir",
+		},
+		"scope": {
+			"a/b/c",
+			"d/e",
+			".",
+			"d/e/newDir",
+		},
+	}
+
+	for name, test := range cases {
+		test := test
+		t.Run(name, func(t *testing.T) {
+			req := require.New(t)
+			fSys := makeWdFs(t)[test.wd]
+
+			ldr, args, err := lclzr.NewLocLoader(test.target, test.scope, test.newDir, fSys)
+			req.NoError(err)
+			checkLoader(req, ldr, "a/b/c/d/e")
+
+			req.Equal("a/b/c/d/e", args.Target.String())
+			req.Equal("a/b/c", args.Scope.String())
+			req.Equal(test.wd+"/"+test.newDir, args.NewDir.String())
+			// memory file system can only find paths rooted at current node
+			req.True(fSys.Exists(test.newDir))
+		})
+	}
+}
+
+func TestNewLocLoaderFails(t *testing.T) {
+	cases := map[string]struct {
+		target string
+		scope  string
+		dest   string
+	}{
+		"non-existent target": {
+			"/b",
+			"/",
+			"/newDir",
+		},
+		"file target": {
+			"/a/kustomization.yaml",
+			"/",
+			"/newDir",
+		},
+		"inner scope": {
+			"/alpha",
+			"/alpha/beta",
+			"/newDir",
+		},
+		"side scope": {
+			"/alpha",
+			"/a",
+			"/newDir",
+		},
+		"existing dst": {
+			"/alpha",
+			"/",
+			"/a",
+		},
+	}
+	for name, params := range cases {
+		params := params
+		t.Run(name, func(t *testing.T) {
+			var buf bytes.Buffer
+			log.SetOutput(&buf)
+			_, _, err := lclzr.NewLocLoader(params.target, params.scope, params.dest, makeMemoryFs(t))
+			require.Error(t, err)
+			require.Empty(t, buf.String())
+		})
+	}
+}
+
+func TestNewFails(t *testing.T) {
+	req := require.New(t)
+	fSys := makeMemoryFs(t)
+
+	ldr, args, err := lclzr.NewLocLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys)
+	req.NoError(err)
+	checkNewLocLoader(req, ldr, &args, "/alpha/beta/gamma", "/alpha", "/alpha/beta/gamma/newDir", fSys)
+
+	cases := map[string]string{
+		"outside scope":     "../../../a",
+		"at dst":            "newDir",
+		"ancestor":          "../../beta",
+		"non-existent root": "delt",
+		"file":              "delta/kustomization.yaml",
+	}
+	for name, root := range cases {
+		root := root
+		t.Run(name, func(t *testing.T) {
+			fSys := makeMemoryFs(t)
+
+			ldr, _, err := lclzr.NewLocLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys)
+			require.NoError(t, err)
+
+			_, err = ldr.New(root)
+			require.Error(t, err)
+		})
+	}
+}
+
+func TestLoadFails(t *testing.T) {
+	req := require.New(t)
+	fSys := makeMemoryFs(t)
+
+	ldr, args, err := lclzr.NewLocLoader("./a/../a", "/a/../a", "/a/newDir", fSys)
+	req.NoError(err)
+	checkNewLocLoader(req, ldr, &args, "/a", "/a", "/a/newDir", fSys)
+
+	cases := map[string]string{
+		"absolute path":     "/a/kustomization.yaml",
+		"directory":         "b",
+		"non-existent file": "kubectl.yaml",
+		"file outside root": "../alpha/beta/gamma/delta/kustomization.yaml",
+		"inside dst":        "newDir/kustomization.yaml",
+	}
+	for name, file := range cases {
+		file := file
+		t.Run(name, func(t *testing.T) {
+			req := require.New(t)
+			fSys := makeMemoryFs(t)
+
+			ldr, _, err := lclzr.NewLocLoader("./a/../a", "/a/../a", "/a/newDir", fSys)
+			req.NoError(err)
+
+			req.NoError(fSys.WriteFile("/a/newDir/kustomization.yaml", []byte("/a/newDir")))
+
+			_, err = ldr.Load(file)
+			req.Error(err)
+		})
+	}
+}

api/internal/localizer/util.go

@@ -0,0 +1,91 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package localizer
+
+import (
+	"log"
+	"path/filepath"
+	"strings"
+
+	"sigs.k8s.io/kustomize/api/ifc"
+	"sigs.k8s.io/kustomize/api/internal/git"
+	"sigs.k8s.io/kustomize/kyaml/errors"
+	"sigs.k8s.io/kustomize/kyaml/filesys"
+)
+
+// establishScope returns the scope given localize arguments and targetLdr at targetArg
+func establishScope(scopeArg string, targetArg string, targetLdr ifc.Loader, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) {
+	if _, isRemote := targetLdr.Repo(); isRemote {
+		if scopeArg != "" {
+			return "", errors.Errorf("scope '%s' specified for remote localize target '%s'", scopeArg, targetArg)
+		}
+		return "", nil
+	}
+	// default scope
+	if scopeArg == "" {
+		return filesys.ConfirmedDir(targetLdr.Root()), nil
+	}
+	scope, err := filesys.ConfirmDir(fSys, scopeArg)
+	if err != nil {
+		return "", errors.WrapPrefixf(err, "unable to establish localize scope")
+	}
+	if !filesys.ConfirmedDir(targetLdr.Root()).HasPrefix(scope) {
+		return scope, errors.Errorf("localize scope '%s' does not contain target '%s' at '%s'",
+			scopeArg, targetArg, targetLdr.Root())
+	}
+	return scope, nil
+}
+
+// createNewDir returns the localize destination directory or error. Note that spec is nil if targetLdr is at local
+// target.
+func createNewDir(newDirArg string, targetLdr ifc.Loader, spec *git.RepoSpec, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) {
+	if newDirArg == "" {
+		newDirArg = defaultNewDir(targetLdr, spec)
+	}
+	if fSys.Exists(newDirArg) {
+		return "", errors.Errorf("localize destination '%s' already exists", newDirArg)
+	}
+	// destination directory must sit in an existing directory
+	if err := fSys.Mkdir(newDirArg); err != nil {
+		return "", errors.WrapPrefixf(err, "unable to create localize destination directory")
+	}
+	newDir, err := filesys.ConfirmDir(fSys, newDirArg)
+	if err != nil {
+		if errCleanup := fSys.RemoveAll(newDir.String()); errCleanup != nil {
+			log.Printf("%s", errors.WrapPrefixf(errCleanup, "unable to clean localize destination").Error())
+		}
+		return "", errors.WrapPrefixf(err, "unable to establish localize destination")
+	}
+
+	return newDir, nil
+}
+
+// defaultNewDir calculates the default localize destination directory name from targetLdr at the localize target
+// and spec of target, which is nil if target is local
+func defaultNewDir(targetLdr ifc.Loader, spec *git.RepoSpec) string {
+	targetDir := filepath.Base(targetLdr.Root())
+	if repo, isRemote := targetLdr.Repo(); isRemote {
+		// kustomize doesn't download repo into repo-named folder
+		// must find repo folder name from url
+		if repo == targetLdr.Root() {
+			targetDir = urlBase(spec.OrgRepo)
+		}
+		return strings.Join([]string{dstPrefix, targetDir, strings.ReplaceAll(spec.Ref, "/", "-")}, "-")
+	}
+	// special case for local target directory since destination directory cannot have "/" in name
+	if targetDir == string(filepath.Separator) {
+		return dstPrefix
+	}
+	return strings.Join([]string{dstPrefix, targetDir}, "-")
+}
+
+// urlBase is the url equivalent of filepath.Base
+func urlBase(url string) string {
+	cleaned := strings.TrimRight(url, "/")
+	i := strings.LastIndex(cleaned, "/")
+	if i < 0 {
+		return cleaned
+	}
+	return cleaned[i+1:]
+}

api/internal/localizer/util_test.go

@@ -0,0 +1,18 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package localizer //nolint:testpackage
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUrlBase(t *testing.T) {
+	require.Equal(t, "repo", urlBase("https://github.com/org/repo"))
+}
+
+func TestUrlBaseTrailingSlash(t *testing.T) {
+	require.Equal(t, "repo", urlBase("github.com/org/repo//"))
+}

api/loader/fileloader.go

@@ -18,6 +18,13 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
+// HasRemoteFileScheme returns whether path has a url scheme that kustomize allows for
+// remote files. See https://github.com/kubernetes-sigs/kustomize/blob/master/examples/remoteBuild.md
+func HasRemoteFileScheme(path string) bool {
+	u, err := url.Parse(path)
+	return err == nil && (u.Scheme == "http" || u.Scheme == "https")
+}
+
 // fileLoader is a kustomization's interface to files.
 //
 // The directory in which a kustomization file sits
@@ -114,6 +121,15 @@ func NewFileLoaderAtRoot(fSys filesys.FileSystem) *fileLoader {
 		RestrictionRootOnly, fSys, filesys.Separator)
 }
 
+// Repo returns the absolute path to the repo that contains Root and true
+// if this fileLoader was created from a url; otherwise, the empty string and false
+func (fl *fileLoader) Repo() (string, bool) {
+	if fl.repoSpec != nil {
+		return fl.repoSpec.Dir.String(), true
+	}
+	return "", false
+}
+
 // Root returns the absolute path that is prepended to any
 // relative paths used in Load.
 func (fl *fileLoader) Root() string {
@@ -283,6 +299,7 @@ func (fl *fileLoader) errIfRepoCycle(newRepoSpec *git.RepoSpec) error {
 // else an error. Relative paths are taken relative
 // to the root.
 func (fl *fileLoader) Load(path string) ([]byte, error) {
+	// TODO(annasong): replace check with HasRemoteFileScheme
 	if u, err := url.Parse(path); err == nil && (u.Scheme == "http" || u.Scheme == "https") {
 		var hc *http.Client
 		if fl.http != nil {

api/loader/fileloader_test.go

@@ -21,6 +21,40 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
+func TestHasRemoteFileScheme(t *testing.T) {
+	cases := map[string]struct {
+		url   string
+		valid bool
+	}{
+		"https file": {
+			"https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/examples/helloWorld/configMap.yaml",
+			true,
+		},
+		"https dir": {
+			"https://github.com/kubernetes-sigs/kustomize//examples/helloWorld/",
+			true,
+		},
+		"no scheme": {
+			"github.com/kubernetes-sigs/kustomize//examples/helloWorld/",
+			false,
+		},
+		"ssh": {
+			"ssh://git@github.com/kubernetes-sigs/kustomize.git",
+			false,
+		},
+		"local": {
+			"pod.yaml",
+			false,
+		},
+	}
+	for name, test := range cases {
+		test := test
+		t.Run(name, func(t *testing.T) {
+			require.Equal(t, test.valid, HasRemoteFileScheme(test.url))
+		})
+	}
+}
+
 type testData struct {
 	path            string
 	expectedContent string
@@ -61,6 +95,8 @@ func TestLoaderLoad(t *testing.T) {
 	require := require.New(t)
 
 	l1 := makeLoader()
+	_, remote := l1.Repo()
+	require.False(remote)
 	require.Equal("/", l1.Root())
 
 	for _, x := range testCases {
@@ -73,6 +109,9 @@ func TestLoaderLoad(t *testing.T) {
 	}
 	l2, err := l1.New("foo/project")
 	require.NoError(err)
+
+	_, remote = l2.Repo()
+	require.False(remote)
 	require.Equal("/foo/project", l2.Root())
 
 	for _, x := range testCases {
@@ -322,6 +361,9 @@ whatever
 		repoSpec, fSys, nil,
 		git.DoNothingCloner(filesys.ConfirmedDir(coRoot)))
 	require.NoError(err)
+	repo, remote := l.Repo()
+	require.True(remote)
+	require.Equal(coRoot, repo)
 	require.Equal(coRoot+"/"+pathInRepo, l.Root())
 
 	_, err = l.New(url)
@@ -335,6 +377,10 @@ whatever
 	url = rootURL + "/" + pathInRepo
 	l2, err := l.New(url)
 	require.NoError(err)
+
+	repo, remote = l2.Repo()
+	require.True(remote)
+	require.Equal(coRoot, repo)
 	require.Equal(coRoot+"/"+pathInRepo, l2.Root())
 }
 
@@ -389,6 +435,8 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) {
 	// This is okay.
 	l2, err = l1.New("../base")
 	require.NoError(err)
+	_, remote := l2.Repo()
+	require.False(remote)
 	require.Equal(cloneRoot+"/foo/base", l2.Root())
 
 	// This is not okay.
@@ -414,6 +462,9 @@ func TestLocalLoaderReferencingGitBase(t *testing.T) {
 
 	l2, err := l1.New("github.com/someOrg/someRepo/foo/base")
 	require.NoError(err)
+	repo, remote := l2.Repo()
+	require.True(remote)
+	require.Equal(cloneRoot, repo)
 	require.Equal(cloneRoot+"/foo/base", l2.Root())
 }