From b8611cf0a916a06d41577d16661f9f0a1ab698b7 Mon Sep 17 00:00:00 2001 From: Anna Song Date: Thu, 1 Dec 2022 09:53:46 -0800 Subject: [PATCH] Refactor localizer code * encapsulate kusttarget params * change dst to string * ConfirmDir --- api/internal/localizer/localizer.go | 82 +++++++++++++-------- api/internal/localizer/localizer_test.go | 92 ++++++++++-------------- api/internal/localizer/util.go | 10 +-- 3 files changed, 97 insertions(+), 87 deletions(-) diff --git a/api/internal/localizer/localizer.go b/api/internal/localizer/localizer.go index 4fe7943ec..67728c1b2 100644 --- a/api/internal/localizer/localizer.go +++ b/api/internal/localizer/localizer.go @@ -12,6 +12,7 @@ import ( "sigs.k8s.io/kustomize/api/internal/target" "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/loader" + "sigs.k8s.io/kustomize/api/provider" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/errors" @@ -20,8 +21,8 @@ import ( "sigs.k8s.io/yaml" ) -// Localizer encapsulates all state needed to localize the root at ldr. -type Localizer struct { +// localizer encapsulates all state needed to localize the root at ldr. +type localizer struct { fSys filesys.FileSystem // kusttarget fields @@ -32,32 +33,56 @@ type Localizer struct { // underlying type is Loader ldr ifc.Loader - // destination directory in newDir that mirrors ldr's current root. - dst filesys.ConfirmedDir + // root is at ldr.Root() + root filesys.ConfirmedDir + + // destination directory in newDir that mirrors root + dst string } -// NewLocalizer is the factory method for Localizer -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()) +// Run attempts to localize the kustomization root at target with the given localize arguments +func Run(target string, scope string, newDir string, fSys filesys.FileSystem) error { + ldr, args, err := NewLoader(target, scope, newDir, fSys) if err != nil { - log.Fatalf("cannot find path from %q to child directory %q: %s", ldr.args.Scope, ldr.Root(), err) + return errors.Wrap(err) } - dst := ldr.args.NewDir.Join(toDst) - if err = ldr.fSys.MkdirAll(dst); err != nil { - return nil, errors.WrapPrefixf(err, "unable to create directory in localize destination") + defer func() { _ = ldr.Cleanup() }() + + toDst, err := filepath.Rel(args.Scope.String(), args.Target.String()) + if err != nil { + log.Panicf("cannot find path from %q to child directory %q: %s", args.Scope, args.Target, err) } - return &Localizer{ - fSys: ldr.fSys, - validator: validator, + dst := args.NewDir.Join(toDst) + if err = fSys.MkdirAll(dst); err != nil { + return errors.WrapPrefixf(err, "unable to create directory in localize destination") + } + + depProvider := provider.NewDepProvider() + rFactory := resmap.NewFactory(depProvider.GetResourceFactory()) + // As of alpha, only built-in plugins, using kustomize's built-in definitions of them, + // are potentially localized. + plgnsLdr := pLdr.NewLoader(types.DisabledPluginConfig(), rFactory, filesys.MakeFsOnDisk()) + err = (&localizer{ + fSys: fSys, + validator: depProvider.GetFieldValidator(), rFactory: rFactory, - pLdr: pLdr, + pLdr: plgnsLdr, ldr: ldr, - dst: filesys.ConfirmedDir(dst), - }, nil + root: args.Target, + dst: dst, + }).localize() + if err != nil { + errCleanup := fSys.RemoveAll(args.NewDir.String()) + if errCleanup != nil { + log.Printf("unable to clean localize destination: %s", errCleanup) + } + return errors.WrapPrefixf(err, "unable to localize target %q", target) + } + return nil } -// Localize localizes the root that lc is at -func (lc *Localizer) Localize() error { +// localize localizes the root that lc is at +func (lc *localizer) localize() error { kt := target.NewKustTarget(lc.ldr, lc.validator, lc.rFactory, lc.pLdr) err := kt.Load() if err != nil { @@ -78,7 +103,7 @@ func (lc *Localizer) Localize() error { if err != nil { return errors.WrapPrefixf(err, "unable to serialize localized kustomization file") } - if err = lc.fSys.WriteFile(lc.dst.Join(konfig.DefaultKustomizationFileName()), content); err != nil { + if err = lc.fSys.WriteFile(filepath.Join(lc.dst, konfig.DefaultKustomizationFileName()), content); err != nil { return errors.WrapPrefixf(err, "unable to write localized kustomization file") } return nil @@ -86,7 +111,7 @@ func (lc *Localizer) Localize() error { // localizeNativeFields localizes paths on kustomize-native fields, like configMapGenerator, that kustomize has a // built-in understanding of. This excludes helm-related fields, such as `helmGlobals` and `helmCharts`. -func (lc *Localizer) localizeNativeFields(kust *types.Kustomization) error { +func (lc *localizer) localizeNativeFields(kust *types.Kustomization) error { for i := range kust.Patches { if kust.Patches[i].Path != "" { newPath, err := lc.localizeFile(kust.Patches[i].Path) @@ -98,12 +123,11 @@ func (lc *Localizer) localizeNativeFields(kust *types.Kustomization) error { } // TODO(annasong): localize all other kustomization fields: resources, bases, components, crds, configurations, // openapi, patchesJson6902, patchesStrategicMerge, replacements, configMapGenerators, secretGenerators - // TODO(annasong): localize built-in plugins under generators, transformers, and validators fields return nil } // localizeFile localizes file path and returns the localized path -func (lc *Localizer) localizeFile(path string) (string, error) { +func (lc *localizer) localizeFile(path string) (string, error) { content, err := lc.ldr.Load(path) if err != nil { return "", errors.Wrap(err) @@ -111,7 +135,8 @@ func (lc *Localizer) localizeFile(path string) (string, error) { var locPath string if loader.IsRemoteFile(path) { - // TODO(annasong): check if able to add localize directory + // TODO(annasong): You need to check if you can add a localize directory here to store + // the remote file content. There may be a directory that shares the localize directory name. locPath = locFilePath(path) } else { // ldr has checked that path must be relative; this is subject to change in beta. @@ -123,10 +148,9 @@ func (lc *Localizer) localizeFile(path string) (string, error) { // 2. avoid paths that temporarily traverse outside the current root, // i.e. ../../../scope/target/current-root. The localized file will be surrounded by // different directories than its source, and so an uncleaned path may no longer be valid. - locPath = cleanFilePath(lc.fSys, filesys.ConfirmedDir(lc.ldr.Root()), path) - // TODO(annasong): check if hits localize directory + locPath = cleanFilePath(lc.fSys, lc.root, path) } - absPath := lc.dst.Join(locPath) + absPath := filepath.Join(lc.dst, locPath) if err = lc.fSys.MkdirAll(filepath.Dir(absPath)); err != nil { return "", errors.WrapPrefixf(err, "unable to create directories to localize file %q", path) } @@ -140,7 +164,7 @@ func (lc *Localizer) localizeFile(path string) (string, error) { // can be inline or in a file. This excludes the HelmChartInflationGenerator. // // Note that the localization in this function has not been implemented yet. -func (lc *Localizer) localizeBuiltinPlugins(kust *types.Kustomization) error { +func (lc *localizer) localizeBuiltinPlugins(kust *types.Kustomization) error { for fieldName, plugins := range map[string]struct { entries []string localizer kio.Filter @@ -186,7 +210,7 @@ func (lc *Localizer) localizeBuiltinPlugins(kust *types.Kustomization) error { // loadResource tries to load resourceEntry as a file path or inline. // On success, loadResource returns the loaded resource map and whether resourceEntry is a file path. -func (lc *Localizer) loadResource(resourceEntry string) (resmap.ResMap, bool, error) { +func (lc *localizer) loadResource(resourceEntry string) (resmap.ResMap, bool, error) { var fileErr error rm, inlineErr := lc.rFactory.NewResMapFromBytes([]byte(resourceEntry)) if inlineErr != nil { diff --git a/api/internal/localizer/localizer_test.go b/api/internal/localizer/localizer_test.go index f022b0561..f050d536e 100644 --- a/api/internal/localizer/localizer_test.go +++ b/api/internal/localizer/localizer_test.go @@ -11,13 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "sigs.k8s.io/kustomize/api/hasher" . "sigs.k8s.io/kustomize/api/internal/localizer" - "sigs.k8s.io/kustomize/api/internal/plugins/loader" - "sigs.k8s.io/kustomize/api/internal/validate" - "sigs.k8s.io/kustomize/api/resmap" - "sigs.k8s.io/kustomize/api/resource" - "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/filesys" ) @@ -57,23 +51,6 @@ 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 Loader - ldr, _, err := NewLoader(target, scope, newDir, fSys) - require.NoError(t, err) - rmFactory := resmap.NewFactory(resource.NewFactory(&hasher.Hasher{})) - lc, err := NewLocalizer( - ldr, - validate.NewFieldValidator(), - rmFactory, - // file system can be in memory, as plugin configuration will prevent the use of file system anyway - loader.NewLoader(types.DisabledPluginConfig(), rmFactory, fSys)) - require.NoError(t, err) - return lc -} - func checkFSys(t *testing.T, fSysExpected filesys.FileSystem, fSysActual filesys.FileSystem) { t.Helper() @@ -97,7 +74,7 @@ func reportFSysDiff(t *testing.T, fSysExpected filesys.FileSystem, fSysActual fi actualContent, readErr := fSysActual.ReadFile(path) require.NoError(t, readErr) expectedContent, findErr := fSysExpected.ReadFile(path) - assert.NoError(t, findErr) + assert.NoErrorf(t, findErr, "unexpected file %q", path) if findErr == nil { assert.Equal(t, string(expectedContent), string(actualContent)) } @@ -118,7 +95,7 @@ func reportFSysDiff(t *testing.T, fSysExpected filesys.FileSystem, fSysActual fi require.NoError(t, err) } -func TestNewLocalizerTargetIsScope(t *testing.T) { +func TestTargetIsScope(t *testing.T) { fSys := makeMemoryFs(t) kustomization := map[string]string{ "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 @@ -127,8 +104,8 @@ namePrefix: my- `, } addFiles(t, fSys, "/a", kustomization) - lclzr := createLocalizer(t, fSys, "/a", "", "/a/b/dst") - require.NoError(t, lclzr.Localize()) + err := Run("/a", "", "/a/b/dst", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/a", kustomization) @@ -136,7 +113,7 @@ namePrefix: my- checkFSys(t, fSysExpected, fSys) } -func TestNewLocalizerTargetNestedInScope(t *testing.T) { +func TestTargetNestedInScope(t *testing.T) { fSys := makeMemoryFs(t) kustomization := map[string]string{ "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 @@ -152,8 +129,8 @@ patches: `, } addFiles(t, fSys, "/a/b", kustomization) - lclzr := createLocalizer(t, fSys, "/a/b", "/", "/a/b/dst") - require.NoError(t, lclzr.Localize()) + err := Run("/a/b", "/", "/a/b/dst", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/a/b", kustomization) @@ -173,8 +150,8 @@ kind: Kustomization } addFiles(t, fSys, "/a", kustomization) - lclzr := createLocalizer(t, fSys, "/a", "/", "/dst") - require.NoError(t, lclzr.Localize()) + err := Run("/a", "/", "/dst", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/a", kustomization) @@ -186,11 +163,11 @@ kind: Kustomization func TestLocalizeFileName(t *testing.T) { for name, path := range map[string]string{ - "nested_directories": "a/b/c/d/patch.yaml", - "localize_dir_name_when_absent": LocalizeDir, - "in_localize_dir_name_when_absent": fmt.Sprintf("%s/patch.yaml", LocalizeDir), - "no_file_extension": "patch", - "kustomization_name": "a/kustomization.yaml", + "nested_directories": "a/b/c/d/patch.yaml", + "localize_dir_name_when_no_remote": LocalizeDir, + "in_localize_dir_name_when_no_remote": fmt.Sprintf("%s/patch.yaml", LocalizeDir), + "no_file_extension": "patch", + "kustomization_name": "a/kustomization.yaml", } { t.Run(name, func(t *testing.T) { fSys := makeMemoryFs(t) @@ -204,8 +181,8 @@ patches: } addFiles(t, fSys, "/a", kustAndPatch) - lclzr := createLocalizer(t, fSys, "/a", "/", "/a/dst") - require.NoError(t, lclzr.Localize()) + err := Run("/a", "/", "/a/dst", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/a", kustAndPatch) @@ -227,8 +204,8 @@ patches: } addFiles(t, fSys, "/alpha/beta/gamma", kustAndPatch) - lclzr := createLocalizer(t, fSys, "/alpha/beta/gamma", "/", "") - require.NoError(t, lclzr.Localize()) + err := Run("/alpha/beta/gamma", "/", "", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/alpha/beta/gamma", kustAndPatch) @@ -276,8 +253,8 @@ spec: } addFiles(t, fSys, "/", kustAndPatch) - lclzr := createLocalizer(t, fSys, "/", "", "") - require.NoError(t, lclzr.Localize()) + err := Run("/", "", "", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/", kustAndPatch) @@ -296,8 +273,12 @@ patches: } addFiles(t, fSys, "/a/b", kustAndPatch) - lclzr := createLocalizer(t, fSys, "/a/b", "", "/dst") - require.Error(t, lclzr.Localize()) + err := Run("/a/b", "", "/dst", fSys) + require.Error(t, err) + + fSysExpected := makeMemoryFs(t) + addFiles(t, fSysExpected, "/a/b", kustAndPatch) + checkFSys(t, fSysExpected, fSys) } func TestLocalizeGenerators(t *testing.T) { @@ -333,8 +314,8 @@ metadata: } addFiles(t, fSys, "/a", kustAndPlugins) - lclzr := createLocalizer(t, fSys, "/a", "", "/alpha/dst") - require.NoError(t, lclzr.Localize()) + err := Run("/a", "", "/alpha/dst", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/a", kustAndPlugins) @@ -384,8 +365,8 @@ paths: } addFiles(t, fSys, "/a", kustAndPlugins) - lclzr := createLocalizer(t, fSys, "/a", "", "/dst") - require.NoError(t, lclzr.Localize()) + err := Run("/a", "", "/dst", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/a", kustAndPlugins) @@ -433,8 +414,8 @@ replacements: `, } addFiles(t, fSys, "/", kustAndPlugin) - lclzr := createLocalizer(t, fSys, "/", "", "/dst") - require.NoError(t, lclzr.Localize()) + err := Run("/", "", "/dst", fSys) + require.NoError(t, err) fSysExpected := makeMemoryFs(t) addFiles(t, fSysExpected, "/", kustAndPlugin) @@ -491,16 +472,19 @@ metadata: t.Run(test.name, func(t *testing.T) { fSys := makeMemoryFs(t) addFiles(t, fSys, "/", test.files) - lclzr := createLocalizer(t, fSys, "/", "", "/dst") - err := lclzr.Localize() + err := Run("/", "", "/dst", fSys) var actualErr ResourceLoadError require.ErrorAs(t, err, &actualErr) require.EqualError(t, actualErr.InlineError, test.inlineErrMsg) require.EqualError(t, actualErr.FileError, test.fileErrMsg) - require.EqualError(t, err, fmt.Sprintf(`%s: when parsing as inline received error: %s + require.EqualError(t, err, fmt.Sprintf(`unable to localize target "/": %s: when parsing as inline received error: %s when parsing as filepath received error: %s`, test.errPrefix, test.inlineErrMsg, test.fileErrMsg)) + + fSysExpected := makeMemoryFs(t) + addFiles(t, fSysExpected, "/", test.files) + checkFSys(t, fSysExpected, fSys) }) } } diff --git a/api/internal/localizer/util.go b/api/internal/localizer/util.go index cf076e129..58820796a 100644 --- a/api/internal/localizer/util.go +++ b/api/internal/localizer/util.go @@ -15,11 +15,13 @@ import ( "sigs.k8s.io/kustomize/kyaml/filesys" ) -// DstPrefix prefixes the target and ref, if target is remote, in the default localize destination directory name -const DstPrefix = "localized" +const ( + // DstPrefix prefixes the target and ref, if target is remote, in the default localize destination directory name + DstPrefix = "localized" -// LocalizeDir is the name of the localize directories used to store remote content in the localize destination -const LocalizeDir = "localized-files" + // LocalizeDir is the name of the localize directories used to store remote content in the localize destination + LocalizeDir = "localized-files" +) // establishScope returns the effective scope given localize arguments and targetLdr at rawTarget. For remote rawTarget, // the effective scope is the downloaded repo.