Skip to content

Commit

Permalink
Merge pull request #5193 from varshaprasad96/refactor/api
Browse files Browse the repository at this point in the history
[refactor]: Internalize loader api
  • Loading branch information
k8s-ci-robot committed Sep 28, 2023
2 parents e3b9afc + 7911b2c commit 654d795
Show file tree
Hide file tree
Showing 31 changed files with 109 additions and 97 deletions.
2 changes: 1 addition & 1 deletion api/internal/accumulator/loadconfigfromcrds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (

"github.com/stretchr/testify/require"
. "sigs.k8s.io/kustomize/api/internal/accumulator"
"sigs.k8s.io/kustomize/api/internal/loader"
"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filesys"
"sigs.k8s.io/kustomize/kyaml/resid"
Expand Down
2 changes: 1 addition & 1 deletion api/internal/generators/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/stretchr/testify/assert"
. "sigs.k8s.io/kustomize/api/internal/generators"
"sigs.k8s.io/kustomize/api/kv"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/pkg/loader"
valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filesys"
Expand Down
2 changes: 1 addition & 1 deletion api/internal/generators/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/stretchr/testify/assert"
. "sigs.k8s.io/kustomize/api/internal/generators"
"sigs.k8s.io/kustomize/api/kv"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/pkg/loader"
valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filesys"
Expand Down
File renamed without changes.
101 changes: 43 additions & 58 deletions api/loader/fileloader.go → api/internal/loader/fileloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func IsRemoteFile(path string) bool {
return err == nil && (u.Scheme == "http" || u.Scheme == "https")
}

// fileLoader is a kustomization's interface to files.
// FileLoader is a kustomization's interface to files.
//
// The directory in which a kustomization file sits
// is referred to below as the kustomization's _root_.
Expand All @@ -38,49 +38,48 @@ func IsRemoteFile(path string) bool {
//
// * supplemental data paths
//
// `Load` is used to visit these paths.
// `Load` is used to visit these paths.
//
// These paths refer to resources, patches,
// data for ConfigMaps and Secrets, etc.
// These paths refer to resources, patches,
// data for ConfigMaps and Secrets, etc.
//
// The loadRestrictor may disallow certain paths
// or classes of paths.
// The loadRestrictor may disallow certain paths
// or classes of paths.
//
// * bases (other kustomizations)
//
// `New` is used to load bases.
// `New` is used to load bases.
//
// A base can be either a remote git repo URL, or
// a directory specified relative to the current
// root. In the former case, the repo is locally
// cloned, and the new loader is rooted on a path
// in that clone.
// A base can be either a remote git repo URL, or
// a directory specified relative to the current
// root. In the former case, the repo is locally
// cloned, and the new loader is rooted on a path
// in that clone.
//
// As loaders create new loaders, a root history
// is established, and used to disallow:
// As loaders create new loaders, a root history
// is established, and used to disallow:
//
// - A base that is a repository that, in turn,
// specifies a base repository seen previously
// in the loading stack (a cycle).
// - A base that is a repository that, in turn,
// specifies a base repository seen previously
// in the loading stack (a cycle).
//
// - An overlay depending on a base positioned at
// or above it. I.e. '../foo' is OK, but '.',
// '..', '../..', etc. are disallowed. Allowing
// such a base has no advantages and encourages
// cycles, particularly if some future change
// were to introduce globbing to file
// specifications in the kustomization file.
// - An overlay depending on a base positioned at
// or above it. I.e. '../foo' is OK, but '.',
// '..', '../..', etc. are disallowed. Allowing
// such a base has no advantages and encourages
// cycles, particularly if some future change
// were to introduce globbing to file
// specifications in the kustomization file.
//
// These restrictions assure that kustomizations
// are self-contained and relocatable, and impose
// some safety when relying on remote kustomizations,
// e.g. a remotely loaded ConfigMap generator specified
// to read from /etc/passwd will fail.
//
type fileLoader struct {
type FileLoader struct {
// Loader that spawned this loader.
// Used to avoid cycles.
referrer *fileLoader
referrer *FileLoader

// An absolute, cleaned path to a directory.
// The Load function will read non-absolute
Expand All @@ -107,23 +106,9 @@ type fileLoader struct {
cleaner func() error
}

// NewFileLoaderAtCwd returns a loader that loads from PWD.
// A convenience for kustomize edit commands.
func NewFileLoaderAtCwd(fSys filesys.FileSystem) *fileLoader {
return newLoaderOrDie(
RestrictionRootOnly, fSys, filesys.SelfDir)
}

// NewFileLoaderAtRoot returns a loader that loads from "/".
// A convenience for tests.
func NewFileLoaderAtRoot(fSys filesys.FileSystem) *fileLoader {
return newLoaderOrDie(
RestrictionRootOnly, fSys, filesys.Separator)
}

// Repo returns the absolute path to the repo that contains Root if this fileLoader was created from a url
// or the empty string otherwise.
func (fl *fileLoader) Repo() string {
func (fl *FileLoader) Repo() string {
if fl.repoSpec != nil {
return fl.repoSpec.Dir.String()
}
Expand All @@ -132,13 +117,13 @@ func (fl *fileLoader) Repo() string {

// Root returns the absolute path that is prepended to any
// relative paths used in Load.
func (fl *fileLoader) Root() string {
func (fl *FileLoader) Root() string {
return fl.root.String()
}

func newLoaderOrDie(
func NewLoaderOrDie(
lr LoadRestrictorFunc,
fSys filesys.FileSystem, path string) *fileLoader {
fSys filesys.FileSystem, path string) *FileLoader {
root, err := filesys.ConfirmDir(fSys, path)
if err != nil {
log.Fatalf("unable to make loader at '%s'; %v", path, err)
Expand All @@ -147,12 +132,12 @@ func newLoaderOrDie(
lr, root, fSys, nil, git.ClonerUsingGitExec)
}

// newLoaderAtConfirmedDir returns a new fileLoader with given root.
// newLoaderAtConfirmedDir returns a new FileLoader with given root.
func newLoaderAtConfirmedDir(
lr LoadRestrictorFunc,
root filesys.ConfirmedDir, fSys filesys.FileSystem,
referrer *fileLoader, cloner git.Cloner) *fileLoader {
return &fileLoader{
referrer *FileLoader, cloner git.Cloner) *FileLoader {
return &FileLoader{
loadRestrictor: lr,
root: root,
referrer: referrer,
Expand All @@ -164,7 +149,7 @@ func newLoaderAtConfirmedDir(

// New returns a new Loader, rooted relative to current loader,
// or rooted in a temp directory holding a git repo clone.
func (fl *fileLoader) New(path string) (ifc.Loader, error) {
func (fl *FileLoader) New(path string) (ifc.Loader, error) {
if path == "" {
return nil, errors.Errorf("new root cannot be empty")
}
Expand Down Expand Up @@ -200,7 +185,7 @@ func (fl *fileLoader) New(path string) (ifc.Loader, error) {
// directory holding a cloned git repo.
func newLoaderAtGitClone(
repoSpec *git.RepoSpec, fSys filesys.FileSystem,
referrer *fileLoader, cloner git.Cloner) (ifc.Loader, error) {
referrer *FileLoader, cloner git.Cloner) (ifc.Loader, error) {
cleaner := repoSpec.Cleaner(fSys)
err := cloner(repoSpec)
if err != nil {
Expand Down Expand Up @@ -229,7 +214,7 @@ func newLoaderAtGitClone(
return nil, fmt.Errorf("%q refers to directory outside of repo %q", repoSpec.AbsPath(),
repoSpec.CloneDir())
}
return &fileLoader{
return &FileLoader{
// Clones never allowed to escape root.
loadRestrictor: RestrictionRootOnly,
root: root,
Expand All @@ -241,7 +226,7 @@ func newLoaderAtGitClone(
}, nil
}

func (fl *fileLoader) errIfGitContainmentViolation(
func (fl *FileLoader) errIfGitContainmentViolation(
base filesys.ConfirmedDir) error {
containingRepo := fl.containingRepo()
if containingRepo == nil {
Expand All @@ -259,7 +244,7 @@ func (fl *fileLoader) errIfGitContainmentViolation(

// Looks back through referrers for a git repo, returning nil
// if none found.
func (fl *fileLoader) containingRepo() *git.RepoSpec {
func (fl *FileLoader) containingRepo() *git.RepoSpec {
if fl.repoSpec != nil {
return fl.repoSpec
}
Expand All @@ -271,7 +256,7 @@ func (fl *fileLoader) containingRepo() *git.RepoSpec {

// errIfArgEqualOrHigher tests whether the argument,
// is equal to or above the root of any ancestor.
func (fl *fileLoader) errIfArgEqualOrHigher(
func (fl *FileLoader) errIfArgEqualOrHigher(
candidateRoot filesys.ConfirmedDir) error {
if fl.root.HasPrefix(candidateRoot) {
return fmt.Errorf(
Expand All @@ -288,7 +273,7 @@ func (fl *fileLoader) errIfArgEqualOrHigher(
// I.e. Allow a distinction between git URI with
// path foo and tag bar and a git URI with the same
// path but a different tag?
func (fl *fileLoader) errIfRepoCycle(newRepoSpec *git.RepoSpec) error {
func (fl *FileLoader) errIfRepoCycle(newRepoSpec *git.RepoSpec) error {
// TODO(monopole): Use parsed data instead of Raw().
if fl.repoSpec != nil &&
strings.HasPrefix(fl.repoSpec.Raw(), newRepoSpec.Raw()) {
Expand All @@ -305,7 +290,7 @@ func (fl *fileLoader) errIfRepoCycle(newRepoSpec *git.RepoSpec) error {
// Load returns the content of file at the given path,
// else an error. Relative paths are taken relative
// to the root.
func (fl *fileLoader) Load(path string) ([]byte, error) {
func (fl *FileLoader) Load(path string) ([]byte, error) {
if IsRemoteFile(path) {
return fl.httpClientGetContent(path)
}
Expand All @@ -319,7 +304,7 @@ func (fl *fileLoader) Load(path string) ([]byte, error) {
return fl.fSys.ReadFile(path)
}

func (fl *fileLoader) httpClientGetContent(path string) ([]byte, error) {
func (fl *FileLoader) httpClientGetContent(path string) ([]byte, error) {
var hc *http.Client
if fl.http != nil {
hc = fl.http
Expand All @@ -344,6 +329,6 @@ func (fl *fileLoader) httpClientGetContent(path string) ([]byte, error) {
}

// Cleanup runs the cleaner.
func (fl *fileLoader) Cleanup() error {
func (fl *FileLoader) Cleanup() error {
return fl.cleaner()
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,9 @@ func MakeFakeFs(td []testData) filesys.FileSystem {
return fSys
}

func makeLoader() *fileLoader {
return NewFileLoaderAtRoot(MakeFakeFs(testCases))
func makeLoader() *FileLoader {
return NewLoaderOrDie(
RestrictionRootOnly, MakeFakeFs(testCases), filesys.Separator)
}

func TestLoaderLoad(t *testing.T) {
Expand Down Expand Up @@ -226,7 +227,7 @@ func TestLoaderLocalScheme(t *testing.T) {
dir.Join(filepath.Join(parts...)),
[]byte(content),
))
actualContent, err := newLoaderOrDie(RestrictionRootOnly,
actualContent, err := NewLoaderOrDie(RestrictionRootOnly,
fSys,
dir.String(),
).Load(strings.Join(parts, "//"))
Expand All @@ -240,7 +241,7 @@ func TestLoaderLocalScheme(t *testing.T) {
"root",
}
require.NoError(t, fSys.MkdirAll(dir.Join(filepath.Join(parts...))))
ldr, err := newLoaderOrDie(RestrictionRootOnly,
ldr, err := NewLoaderOrDie(RestrictionRootOnly,
fSys,
dir.String(),
).New(strings.Join(parts, "//"))
Expand Down Expand Up @@ -322,7 +323,7 @@ func TestRestrictionRootOnlyInRealLoader(t *testing.T) {

var l ifc.Loader

l = newLoaderOrDie(RestrictionRootOnly, fSys, dir)
l = NewLoaderOrDie(RestrictionRootOnly, fSys, dir)

l = doSanityChecksAndDropIntoBase(t, l)

Expand All @@ -343,7 +344,7 @@ func TestRestrictionNoneInRealLoader(t *testing.T) {

var l ifc.Loader

l = newLoaderOrDie(RestrictionNone, fSys, dir)
l = NewLoaderOrDie(RestrictionNone, fSys, dir)

l = doSanityChecksAndDropIntoBase(t, l)

Expand Down Expand Up @@ -442,7 +443,7 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) {

// Establish that a local overlay can navigate
// to the local bases.
l1 = newLoaderOrDie(
l1 = NewLoaderOrDie(
RestrictionRootOnly, fSys, cloneRoot+"/foo/overlay")
require.Equal(cloneRoot+"/foo/overlay", l1.Root())

Expand Down Expand Up @@ -600,7 +601,8 @@ func TestLoaderHTTP(t *testing.T) {
},
}

l1 := NewFileLoaderAtRoot(MakeFakeFs(testCasesFile))
l1 := NewLoaderOrDie(
RestrictionRootOnly, MakeFakeFs(testCasesFile), filesys.Separator)
require.Equal("/", l1.Root())

for _, x := range testCasesFile {
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion api/internal/localizer/localizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (

"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/generators"
"sigs.k8s.io/kustomize/api/internal/loader"
"sigs.k8s.io/kustomize/api/internal/target"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/provider"
"sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/types"
Expand Down
2 changes: 1 addition & 1 deletion api/internal/localizer/locloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/git"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/internal/loader"
"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/filesys"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"reflect"
"testing"

"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/internal/loader"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filesys"
"sigs.k8s.io/kustomize/kyaml/resid"
Expand Down
2 changes: 1 addition & 1 deletion api/internal/plugins/execplugin/execplugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import (
"testing"

"github.com/stretchr/testify/require"
fLdr "sigs.k8s.io/kustomize/api/internal/loader"
. "sigs.k8s.io/kustomize/api/internal/plugins/execplugin"
pLdr "sigs.k8s.io/kustomize/api/internal/plugins/loader"
"sigs.k8s.io/kustomize/api/internal/plugins/utils"
fLdr "sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/provider"
"sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/types"
Expand Down
2 changes: 1 addition & 1 deletion api/internal/plugins/loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"testing"

"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/api/internal/loader"
. "sigs.k8s.io/kustomize/api/internal/plugins/loader"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/provider"
"sigs.k8s.io/kustomize/api/resmap"
kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
Expand Down
2 changes: 1 addition & 1 deletion api/internal/target/kusttarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import (
"sigs.k8s.io/kustomize/api/internal/accumulator"
"sigs.k8s.io/kustomize/api/internal/builtins"
"sigs.k8s.io/kustomize/api/internal/kusterr"
load "sigs.k8s.io/kustomize/api/internal/loader"
"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
"sigs.k8s.io/kustomize/api/internal/plugins/builtinhelpers"
"sigs.k8s.io/kustomize/api/internal/plugins/loader"
"sigs.k8s.io/kustomize/api/internal/utils"
"sigs.k8s.io/kustomize/api/konfig"
load "sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/resource"
"sigs.k8s.io/kustomize/api/types"
Expand Down
2 changes: 1 addition & 1 deletion api/internal/target/kusttarget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/api/ifc"
. "sigs.k8s.io/kustomize/api/internal/target"
"sigs.k8s.io/kustomize/api/loader"
"sigs.k8s.io/kustomize/api/pkg/loader"
"sigs.k8s.io/kustomize/api/provider"
"sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/resource"
Expand Down

0 comments on commit 654d795

Please sign in to comment.