diff --git a/api/internal/localizer/localizer.go b/api/internal/localizer/localizer.go index 545c44753..49a0093fe 100644 --- a/api/internal/localizer/localizer.go +++ b/api/internal/localizer/localizer.go @@ -27,7 +27,7 @@ type Localizer struct { rFactory *resmap.Factory pLdr *pLdr.Loader - // underlying type is LocLoader + // underlying type is Loader ldr ifc.Loader // destination directory in newDir that mirrors ldr's current root. @@ -35,7 +35,7 @@ type Localizer struct { } // NewLocalizer is the factory method for Localizer -func NewLocalizer(ldr *LocLoader, validator ifc.Validator, rFactory *resmap.Factory, pLdr *pLdr.Loader) (*Localizer, error) { +func NewLocalizer(ldr *Loader, validator ifc.Validator, rFactory *resmap.Factory, pLdr *pLdr.Loader) (*Localizer, error) { toDst, err := filepath.Rel(ldr.args.Scope.String(), ldr.Root()) if err != nil { log.Fatalf("cannot find path from directory %q to %q inside directory: %s", ldr.args.Scope.String(), diff --git a/api/internal/localizer/localizer_test.go b/api/internal/localizer/localizer_test.go index d2a2171a1..596881cca 100644 --- a/api/internal/localizer/localizer_test.go +++ b/api/internal/localizer/localizer_test.go @@ -57,8 +57,8 @@ func addFiles(t *testing.T, fSys filesys.FileSystem, parentDir string, files map func createLocalizer(t *testing.T, fSys filesys.FileSystem, target string, scope string, newDir string) *Localizer { t.Helper() - // no need to re-test LocLoader - ldr, _, err := NewLocLoader(target, scope, newDir, fSys) + // no need to re-test Loader + ldr, _, err := NewLoader(target, scope, newDir, fSys) require.NoError(t, err) rmFactory := resmap.NewFactory(resource.NewFactory(&hasher.Hasher{})) lc, err := NewLocalizer( diff --git a/api/internal/localizer/locloader.go b/api/internal/localizer/locloader.go index d754cb0c1..c29d355ed 100644 --- a/api/internal/localizer/locloader.go +++ b/api/internal/localizer/locloader.go @@ -16,8 +16,8 @@ import ( const DstPrefix = "localized" -// LocArgs holds localize arguments -type LocArgs struct { +// Args holds localize arguments +type Args struct { // target; local copy if remote Target filesys.ConfirmedDir @@ -28,55 +28,54 @@ type LocArgs struct { NewDir filesys.ConfirmedDir } -// LocLoader is the Loader for kustomize localize. It is an ifc.Loader that enforces localize constraints. -type LocLoader struct { +// Loader is an ifc.Loader that enforces additional constraints specific to kustomize localize. +type Loader struct { fSys filesys.FileSystem - args *LocArgs + args *Args - // loader at locLoader's current directory + // loader at Loader's current directory ifc.Loader - // whether locLoader and all its ancestors are the result of local references + // whether Loader and all its ancestors are the result of local references local bool } -var _ ifc.Loader = &LocLoader{} +var _ ifc.Loader = &Loader{} -// NewLocLoader is the factory method for LocLoader, under localize constraints, at targetArg. For invalid localize arguments, -// NewLocLoader returns an error. -func NewLocLoader(targetArg string, scopeArg string, newDirArg string, fSys filesys.FileSystem) (*LocLoader, LocArgs, error) { +// NewLoader is the factory method for Loader, under localize constraints, at rawTarget. For invalid localize arguments, +// NewLoader returns an error. +func NewLoader(rawTarget string, rawScope string, rawNewDir string, fSys filesys.FileSystem) (*Loader, Args, error) { // check earlier to avoid cleanup - repoSpec, err := git.NewRepoSpecFromURL(targetArg) + repoSpec, err := git.NewRepoSpecFromURL(rawTarget) if err == nil && repoSpec.Ref == "" { - return nil, LocArgs{}, - errors.Errorf("localize remote root '%s' missing ref query string parameter", targetArg) + return nil, Args{}, errors.Errorf("localize remote root %q missing ref query string parameter", rawTarget) } // for security, should enforce load restrictions - ldr, err := loader.NewLoader(loader.RestrictionRootOnly, targetArg, fSys) + ldr, err := loader.NewLoader(loader.RestrictionRootOnly, rawTarget, fSys) if err != nil { - return nil, LocArgs{}, errors.WrapPrefixf(err, "unable to establish localize target '%s'", targetArg) + return nil, Args{}, errors.WrapPrefixf(err, "unable to establish localize target %q", rawTarget) } - scope, err := establishScope(scopeArg, targetArg, ldr, fSys) + scope, err := establishScope(rawScope, rawTarget, ldr, fSys) if err != nil { _ = ldr.Cleanup() - return nil, LocArgs{}, errors.WrapPrefixf(err, "invalid localize scope '%s'", scopeArg) + return nil, Args{}, errors.WrapPrefixf(err, "invalid localize scope %q", rawScope) } - newDir, err := createNewDir(newDirArg, ldr, repoSpec, fSys) + newDir, err := createNewDir(rawNewDir, ldr, repoSpec, fSys) if err != nil { _ = ldr.Cleanup() - return nil, LocArgs{}, errors.WrapPrefixf(err, "invalid localize destination '%s'", newDirArg) + return nil, Args{}, errors.WrapPrefixf(err, "invalid localize destination %q", rawNewDir) } - args := LocArgs{ + args := Args{ Target: filesys.ConfirmedDir(ldr.Root()), Scope: scope, NewDir: newDir, } - return &LocLoader{ + return &Loader{ fSys: fSys, args: &args, Loader: ldr, @@ -86,28 +85,28 @@ func NewLocLoader(targetArg string, scopeArg string, newDirArg string, fSys file // 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) { +func (ll *Loader) 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) + return nil, errors.Errorf("absolute paths not yet supported in alpha: file path %q 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()) + log.Fatalf(errors.WrapPrefixf(err, "cannot clean validated file path %q", 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) + "file path %q references into localize destination %q", dir.Join(f), ll.args.NewDir) } } return content, nil @@ -115,7 +114,7 @@ func (ll *LocLoader) Load(path string) ([]byte, error) { // 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) { +func (ll *Loader) New(path string) (ifc.Loader, error) { ldr, err := ll.Loader.New(path) if err != nil { return nil, errors.WrapPrefixf(err, "invalid root reference") @@ -123,17 +122,17 @@ func (ll *LocLoader) New(path string) (ifc.Loader, error) { if repo := ldr.Repo(); repo == "" { 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) + return nil, errors.Errorf("root %q outside localize scope %q", 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) + "root %q references into localize destination %q", ldr.Root(), ll.args.NewDir) } } else if !hasRef(path) { return nil, errors.Errorf("localize remote root %q missing ref query string parameter", path) } - return &LocLoader{ + return &Loader{ fSys: ll.fSys, args: ll.args, Loader: ldr, diff --git a/api/internal/localizer/locloader_test.go b/api/internal/localizer/locloader_test.go index 8c0a7c1ad..b7e150338 100644 --- a/api/internal/localizer/locloader_test.go +++ b/api/internal/localizer/locloader_test.go @@ -14,9 +14,9 @@ import ( "sigs.k8s.io/kustomize/kyaml/filesys" ) -func checkNewLocLoader(req *require.Assertions, ldr *LocLoader, args *LocArgs, target string, scope string, newDir string, fSys filesys.FileSystem) { +func checkNewLoader(req *require.Assertions, ldr *Loader, args *Args, target string, scope string, newDir string, fSys filesys.FileSystem) { checkLoader(req, ldr, target) - checkLocArgs(req, args, target, scope, newDir, fSys) + checkArgs(req, args, target, scope, newDir, fSys) } func checkLoader(req *require.Assertions, ldr ifc.Loader, root string) { @@ -24,7 +24,7 @@ func checkLoader(req *require.Assertions, ldr ifc.Loader, root string) { req.Empty(ldr.Repo()) } -func checkLocArgs(req *require.Assertions, args *LocArgs, target string, scope string, newDir string, fSys filesys.FileSystem) { +func checkArgs(req *require.Assertions, args *Args, 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()) @@ -38,9 +38,9 @@ func TestLocalLoadNewAndCleanup(t *testing.T) { var buf bytes.Buffer log.SetOutput(&buf) // typical setup - ldr, args, err := NewLocLoader("a", "/", "/newDir", fSys) + ldr, args, err := NewLoader("a", "/", "/newDir", fSys) req.NoError(err) - checkNewLocLoader(req, ldr, &args, "/a", "/", "/newDir", fSys) + checkNewLoader(req, ldr, &args, "/a", "/", "/newDir", fSys) fSysCopy := makeMemoryFs(t) req.NoError(fSysCopy.Mkdir("/newDir")) @@ -85,9 +85,9 @@ func TestNewLocLoaderDefaultForRootTarget(t *testing.T) { req := require.New(t) fSys := makeMemoryFs(t) - ldr, args, err := NewLocLoader(params.target, params.scope, "", fSys) + ldr, args, err := NewLoader(params.target, params.scope, "", fSys) req.NoError(err) - checkNewLocLoader(req, ldr, &args, "/", "/", "/"+DstPrefix, fSys) + checkNewLoader(req, ldr, &args, "/", "/", "/"+DstPrefix, fSys) // file in root, but nested content, err := ldr.Load("a/pod.yaml") @@ -112,9 +112,9 @@ func TestNewMultiple(t *testing.T) { // default destination for non-file system root target // destination outside of scope - ldr, args, err := NewLocLoader("/alpha/beta", "/alpha", "", fSys) + ldr, args, err := NewLoader("/alpha/beta", "/alpha", "", fSys) req.NoError(err) - checkNewLocLoader(req, ldr, &args, "/alpha/beta", "/alpha", "/"+DstPrefix+"-beta", fSys) + checkNewLoader(req, ldr, &args, "/alpha/beta", "/alpha", "/"+DstPrefix+"-beta", fSys) // nested child root that isn't cleaned descLdr, err := ldr.New("../beta/gamma/delta") @@ -173,7 +173,7 @@ func TestNewLocLoaderCwdNotRoot(t *testing.T) { req := require.New(t) fSys := makeWdFs(t)[test.wd] - ldr, args, err := NewLocLoader(test.target, test.scope, test.newDir, fSys) + ldr, args, err := NewLoader(test.target, test.scope, test.newDir, fSys) req.NoError(err) checkLoader(req, ldr, "a/b/c/d/e") @@ -223,7 +223,8 @@ func TestNewLocLoaderFails(t *testing.T) { t.Run(name, func(t *testing.T) { var buf bytes.Buffer log.SetOutput(&buf) - _, _, err := NewLocLoader(params.target, params.scope, params.dest, makeMemoryFs(t)) + + _, _, err := NewLoader(params.target, params.scope, params.dest, makeMemoryFs(t)) require.Error(t, err) require.Empty(t, buf.String()) }) @@ -234,9 +235,9 @@ func TestNewFails(t *testing.T) { req := require.New(t) fSys := makeMemoryFs(t) - ldr, args, err := NewLocLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys) + ldr, args, err := NewLoader("/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) + checkNewLoader(req, ldr, &args, "/alpha/beta/gamma", "/alpha", "/alpha/beta/gamma/newDir", fSys) cases := map[string]string{ "outside scope": "../../../a", @@ -250,7 +251,7 @@ func TestNewFails(t *testing.T) { t.Run(name, func(t *testing.T) { fSys := makeMemoryFs(t) - ldr, _, err := NewLocLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys) + ldr, _, err := NewLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys) require.NoError(t, err) _, err = ldr.New(root) @@ -263,9 +264,9 @@ func TestLoadFails(t *testing.T) { req := require.New(t) fSys := makeMemoryFs(t) - ldr, args, err := NewLocLoader("./a/../a", "/a/../a", "/a/newDir", fSys) + ldr, args, err := NewLoader("./a/../a", "/a/../a", "/a/newDir", fSys) req.NoError(err) - checkNewLocLoader(req, ldr, &args, "/a", "/a", "/a/newDir", fSys) + checkNewLoader(req, ldr, &args, "/a", "/a", "/a/newDir", fSys) cases := map[string]string{ "absolute path": "/a/pod.yaml", @@ -280,7 +281,7 @@ func TestLoadFails(t *testing.T) { req := require.New(t) fSys := makeMemoryFs(t) - ldr, _, err := NewLocLoader("./a/../a", "/a/../a", "/a/newDir", fSys) + ldr, _, err := NewLoader("./a/../a", "/a/../a", "/a/newDir", fSys) req.NoError(err) req.NoError(fSys.WriteFile("/a/newDir/pod.yaml", []byte(podConfiguration))) diff --git a/api/internal/localizer/util.go b/api/internal/localizer/util.go index c1564d513..dfbd8fc91 100644 --- a/api/internal/localizer/util.go +++ b/api/internal/localizer/util.go @@ -14,44 +14,44 @@ import ( "sigs.k8s.io/kustomize/kyaml/filesys" ) -// establishScope returns the effective scope given localize arguments and targetLdr at targetArg. For remote targetArg, +// establishScope returns the effective scope given localize arguments and targetLdr at rawTarget. For remote targetArg, // the effective scope is the downloaded repo. -func establishScope(scopeArg string, targetArg string, targetLdr ifc.Loader, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) { +func establishScope(rawScope string, rawTarget string, targetLdr ifc.Loader, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) { if repo := targetLdr.Repo(); repo != "" { - if scopeArg != "" { - return "", errors.Errorf("scope '%s' specified for remote localize target '%s'", scopeArg, targetArg) + if rawScope != "" { + return "", errors.Errorf("scope %q specified for remote localize target %q", rawScope, rawTarget) } return filesys.ConfirmedDir(repo), nil } // default scope - if scopeArg == "" { + if rawScope == "" { return filesys.ConfirmedDir(targetLdr.Root()), nil } - scope, err := filesys.ConfirmDir(fSys, scopeArg) + scope, err := filesys.ConfirmDir(fSys, rawScope) 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, errors.Errorf("localize scope %q does not contain target %q at %q", rawScope, rawTarget, + 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) +func createNewDir(rawNewDir string, targetLdr ifc.Loader, spec *git.RepoSpec, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) { + if rawNewDir == "" { + rawNewDir = defaultNewDir(targetLdr, spec) } - if fSys.Exists(newDirArg) { - return "", errors.Errorf("localize destination '%s' already exists", newDirArg) + if fSys.Exists(rawNewDir) { + return "", errors.Errorf("localize destination %q already exists", rawNewDir) } // destination directory must sit in an existing directory - if err := fSys.Mkdir(newDirArg); err != nil { + if err := fSys.Mkdir(rawNewDir); err != nil { return "", errors.WrapPrefixf(err, "unable to create localize destination directory") } - newDir, err := filesys.ConfirmDir(fSys, newDirArg) + newDir, err := filesys.ConfirmDir(fSys, rawNewDir) if err != nil { if errCleanup := fSys.RemoveAll(newDir.String()); errCleanup != nil { log.Printf("%s", errors.WrapPrefixf(errCleanup, "unable to clean localize destination").Error())