From 3255c73c71d88ea4dff5b9446c139e7f9004e016 Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Wed, 24 Mar 2021 17:40:59 -0700 Subject: [PATCH] Loader FS and empty env fix --- .../plugins/execplugin/execplugin_test.go | 4 +- api/internal/plugins/loader/loader.go | 7 +-- api/internal/plugins/loader/loader_test.go | 8 +-- api/internal/target/maker_test.go | 6 +-- api/konfig/plugins.go | 37 +++++--------- api/konfig/plugins_test.go | 49 +++++++++++++++++++ api/krusty/kustomizer.go | 3 +- api/krusty/options.go | 3 +- api/testutils/kusttest/harness.go | 2 +- api/testutils/kusttest/harnessenhanced.go | 5 +- api/types/pluginconfig.go | 18 +++++++ kustomize/commands/build/build.go | 2 +- 12 files changed, 100 insertions(+), 44 deletions(-) diff --git a/api/internal/plugins/execplugin/execplugin_test.go b/api/internal/plugins/execplugin/execplugin_test.go index 2f7e42bcc..434aa4b52 100644 --- a/api/internal/plugins/execplugin/execplugin_test.go +++ b/api/internal/plugins/execplugin/execplugin_test.go @@ -13,10 +13,10 @@ import ( . "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" - "sigs.k8s.io/kustomize/api/konfig" 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" ) func TestExecPluginConfig(t *testing.T) { @@ -45,7 +45,7 @@ s/$BAR/bar baz/g }) pluginConfig.RemoveBuildAnnotations() - loader := pLdr.NewLoader(konfig.DisabledPluginConfig(), rf) + loader := pLdr.NewLoader(types.DisabledPluginConfig(), rf, fSys) pluginPath, err := loader.AbsolutePluginPath(pluginConfig.OrgId()) if err != nil { t.Fatalf("unexpected err: %v", err) diff --git a/api/internal/plugins/loader/loader.go b/api/internal/plugins/loader/loader.go index 34b9731af..6a39c305f 100644 --- a/api/internal/plugins/loader/loader.go +++ b/api/internal/plugins/loader/loader.go @@ -30,6 +30,7 @@ import ( type Loader struct { pc *types.PluginConfig rf *resmap.Factory + fs filesys.FileSystem // absolutePluginHome caches the location of a valid plugin root directory. // It should only be set once the directory's existence has been confirmed. @@ -37,8 +38,8 @@ type Loader struct { } func NewLoader( - pc *types.PluginConfig, rf *resmap.Factory) *Loader { - return &Loader{pc: pc, rf: rf} + pc *types.PluginConfig, rf *resmap.Factory, fs filesys.FileSystem) *Loader { + return &Loader{pc: pc, rf: rf, fs: fs} } func (l *Loader) LoadGenerators( @@ -135,7 +136,7 @@ func (l *Loader) absPluginHome() (string, error) { } // Check default locations for a valid plugin root, and cache it if found. - dir, err := konfig.DefaultAbsPluginHome(filesys.MakeFsOnDisk()) + dir, err := konfig.DefaultAbsPluginHome(l.fs) if err != nil { return "", err } diff --git a/api/internal/plugins/loader/loader_test.go b/api/internal/plugins/loader/loader_test.go index 3930eaa72..b31f4245c 100644 --- a/api/internal/plugins/loader/loader_test.go +++ b/api/internal/plugins/loader/loader_test.go @@ -8,7 +8,6 @@ import ( "sigs.k8s.io/kustomize/api/filesys" . "sigs.k8s.io/kustomize/api/internal/plugins/loader" - "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/loader" "sigs.k8s.io/kustomize/api/provider" "sigs.k8s.io/kustomize/api/resmap" @@ -52,9 +51,10 @@ func TestLoader(t *testing.T) { defer th.Reset() p := provider.NewDefaultDepProvider() rmF := resmap.NewFactory(p.GetResourceFactory()) + fsys := filesys.MakeFsInMemory() fLdr, err := loader.NewLoader( loader.RestrictionRootOnly, - filesys.Separator, filesys.MakeFsInMemory()) + filesys.Separator, fsys) if err != nil { t.Fatal(err) } @@ -66,8 +66,8 @@ func TestLoader(t *testing.T) { for _, behavior := range []types.BuiltinPluginLoadingOptions{ /* types.BploUseStaticallyLinked, types.BploLoadFromFileSys */} { - c := konfig.EnabledPluginConfig(behavior) - pLdr := NewLoader(c, rmF) + c := types.EnabledPluginConfig(behavior) + pLdr := NewLoader(c, rmF, fsys) if pLdr == nil { t.Fatal("expect non-nil loader") } diff --git a/api/internal/target/maker_test.go b/api/internal/target/maker_test.go index a0d0e6512..e59c4cce7 100644 --- a/api/internal/target/maker_test.go +++ b/api/internal/target/maker_test.go @@ -9,11 +9,11 @@ import ( "sigs.k8s.io/kustomize/api/filesys" pLdr "sigs.k8s.io/kustomize/api/internal/plugins/loader" "sigs.k8s.io/kustomize/api/internal/target" - "sigs.k8s.io/kustomize/api/konfig" fLdr "sigs.k8s.io/kustomize/api/loader" "sigs.k8s.io/kustomize/api/provider" "sigs.k8s.io/kustomize/api/resmap" valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest" + "sigs.k8s.io/kustomize/api/types" ) func makeAndLoadKustTarget( @@ -37,10 +37,10 @@ func makeKustTargetWithRf( t.Fatal(err) } rf := resmap.NewFactory(pvd.GetResourceFactory()) - pc := konfig.DisabledPluginConfig() + pc := types.DisabledPluginConfig() return target.NewKustTarget( ldr, valtest_test.MakeFakeValidator(), rf, - pLdr.NewLoader(pc, rf)) + pLdr.NewLoader(pc, rf, fSys)) } diff --git a/api/konfig/plugins.go b/api/konfig/plugins.go index 30f0b05f2..0615f1768 100644 --- a/api/konfig/plugins.go +++ b/api/konfig/plugins.go @@ -41,24 +41,6 @@ const ( NoPluginHomeSentinal = "/No/non-builtin/plugins!" ) -func EnabledPluginConfig(b types.BuiltinPluginLoadingOptions) *types.PluginConfig { - return MakePluginConfig(types.PluginRestrictionsNone, b) -} - -func DisabledPluginConfig() *types.PluginConfig { - return MakePluginConfig( - types.PluginRestrictionsBuiltinsOnly, - types.BploUseStaticallyLinked) -} - -func MakePluginConfig(pr types.PluginRestrictions, - b types.BuiltinPluginLoadingOptions) *types.PluginConfig { - return &types.PluginConfig{ - PluginRestrictions: pr, - BpLoadingOptions: b, - } -} - type NotedFunc struct { Note string F func() string @@ -79,9 +61,11 @@ func DefaultAbsPluginHome(fSys filesys.FileSystem) (string, error) { { Note: "homed in $" + XdgConfigHomeEnv, F: func() string { - return filepath.Join( - os.Getenv(XdgConfigHomeEnv), - ProgramName, RelPluginHome) + if root := os.Getenv(XdgConfigHomeEnv); root != "" { + return filepath.Join(root, ProgramName, RelPluginHome) + } + // do not look in "kustomize/plugin" if XdgConfigHomeEnv is unset + return "" }, }, { @@ -110,11 +94,14 @@ func FirstDirThatExistsElseError( pathFuncs []NotedFunc) (string, error) { var nope []types.Pair for _, dt := range pathFuncs { - dir := dt.F() - if fSys.Exists(dir) { - return dir, nil + if dir := dt.F(); dir != "" { + if fSys.Exists(dir) { + return dir, nil + } + nope = append(nope, types.Pair{Key: dt.Note, Value: dir}) + } else { + nope = append(nope, types.Pair{Key: dt.Note, Value: ""}) } - nope = append(nope, types.Pair{Key: dt.Note, Value: dir}) } return "", types.NewErrUnableToFind(what, nope) } diff --git a/api/konfig/plugins_test.go b/api/konfig/plugins_test.go index 5ef6b28a3..953047d86 100644 --- a/api/konfig/plugins_test.go +++ b/api/konfig/plugins_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/types" ) @@ -28,6 +30,34 @@ func TestDefaultAbsPluginHome_NoKustomizePluginHomeEnv(t *testing.T) { if !types.IsErrUnableToFind(err) { t.Fatalf("unexpected err: %v", err) } + for _, expectedMsg := range []string{ + "unable to find plugin root - tried:", + "(''; homed in $KUSTOMIZE_PLUGIN_HOME)", + "; homed in $XDG_CONFIG_HOME)", + "/.config/kustomize/plugin'; homed in default value of $XDG_CONFIG_HOME)", + "/kustomize/plugin'; homed in home directory)", + } { + assert.Contains(t, err.Error(), expectedMsg) + } +} + +func TestDefaultAbsPluginHome_EmptyKustomizePluginHomeEnv(t *testing.T) { + keep, isSet := os.LookupEnv(KustomizePluginHomeEnv) + os.Setenv(KustomizePluginHomeEnv, "") + + _, err := DefaultAbsPluginHome(filesys.MakeFsInMemory()) + if !isSet { + _ = os.Unsetenv(KustomizePluginHomeEnv) + } else { + _ = os.Setenv(KustomizePluginHomeEnv, keep) + } + if err == nil { + t.Fatalf("expected err") + } + if !types.IsErrUnableToFind(err) { + t.Fatalf("unexpected err: %v", err) + } + assert.Contains(t, err.Error(), "(''; homed in $KUSTOMIZE_PLUGIN_HOME)") } func TestDefaultAbsPluginHome_WithKustomizePluginHomeEnv(t *testing.T) { @@ -89,6 +119,25 @@ func TestDefaultAbsPluginHomeNoConfig(t *testing.T) { } } +func TestDefaultAbsPluginHomeEmptyXdgConfig(t *testing.T) { + keep, isSet := os.LookupEnv(XdgConfigHomeEnv) + os.Setenv(XdgConfigHomeEnv, "") + if isSet { + _ = os.Unsetenv(XdgConfigHomeEnv) + } + _, err := DefaultAbsPluginHome(filesys.MakeFsInMemory()) + if isSet { + os.Setenv(XdgConfigHomeEnv, keep) + } + if err == nil { + t.Fatalf("expected err") + } + if !types.IsErrUnableToFind(err) { + t.Fatalf("unexpected err: %v", err) + } + assert.Contains(t, err.Error(), "(''; homed in $XDG_CONFIG_HOME)") +} + func TestDefaultAbsPluginHomeNoXdgWithDotConfig(t *testing.T) { fSys := filesys.MakeFsInMemory() configDir := filepath.Join( diff --git a/api/krusty/kustomizer.go b/api/krusty/kustomizer.go index 94cd651bd..08af7ee75 100644 --- a/api/krusty/kustomizer.go +++ b/api/krusty/kustomizer.go @@ -66,7 +66,8 @@ func (b *Kustomizer) Run( ldr, b.depProvider.GetFieldValidator(), resmapFactory, - pLdr.NewLoader(b.options.PluginConfig, resmapFactory), + // The plugin configs are always located on disk, regardless of the fSys passed in + pLdr.NewLoader(b.options.PluginConfig, resmapFactory, filesys.MakeFsOnDisk()), ) err = kt.Load() if err != nil { diff --git a/api/krusty/options.go b/api/krusty/options.go index 91fd707a9..438f6c102 100644 --- a/api/krusty/options.go +++ b/api/krusty/options.go @@ -5,7 +5,6 @@ package krusty import ( "sigs.k8s.io/kustomize/api/internal/plugins/builtinhelpers" - "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/types" ) @@ -42,7 +41,7 @@ func MakeDefaultOptions() *Options { AddManagedbyLabel: false, LoadRestrictions: types.LoadRestrictionsRootOnly, DoPrune: false, - PluginConfig: konfig.DisabledPluginConfig(), + PluginConfig: types.DisabledPluginConfig(), } } diff --git a/api/testutils/kusttest/harness.go b/api/testutils/kusttest/harness.go index 19db91ddc..8fefbb628 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -76,7 +76,7 @@ func (th Harness) MakeOptionsPluginsDisabled() krusty.Options { // Enables use of non-builtin plugins. func (th Harness) MakeOptionsPluginsEnabled() krusty.Options { - pc := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) + pc := types.EnabledPluginConfig(types.BploLoadFromFileSys) o := *krusty.MakeDefaultOptions() o.PluginConfig = pc return o diff --git a/api/testutils/kusttest/harnessenhanced.go b/api/testutils/kusttest/harnessenhanced.go index 955abd6d2..fbe485840 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -40,7 +40,7 @@ type HarnessEnhanced struct { func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced { pte := newPluginTestEnv(t).set() - pc := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) + pc := types.EnabledPluginConfig(types.BploLoadFromFileSys) pc.FnpLoadingOptions.EnableStar = true p := provider.NewDefaultDepProvider() resourceFactory := p.GetResourceFactory() @@ -50,7 +50,8 @@ func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced { Harness: MakeHarness(t), pte: pte, rf: resmapFactory, - pl: pLdr.NewLoader(pc, resmapFactory)} + // The plugin configs are always located on disk, regardless of the test harness's FS + pl: pLdr.NewLoader(pc, resmapFactory, filesys.MakeFsOnDisk())} // Point the file loader to the root ('/') of the in-memory file system. result.ResetLoaderRoot(filesys.Separator) diff --git a/api/types/pluginconfig.go b/api/types/pluginconfig.go index d058ef30b..b9fccfee2 100644 --- a/api/types/pluginconfig.go +++ b/api/types/pluginconfig.go @@ -14,3 +14,21 @@ type PluginConfig struct { // FnpLoadingOptions sets the way function-based plugin behaviors. FnpLoadingOptions FnPluginLoadingOptions } + +func EnabledPluginConfig(b BuiltinPluginLoadingOptions) *PluginConfig { + return MakePluginConfig(PluginRestrictionsNone, b) +} + +func DisabledPluginConfig() *PluginConfig { + return MakePluginConfig( + PluginRestrictionsBuiltinsOnly, + BploUseStaticallyLinked) +} + +func MakePluginConfig(pr PluginRestrictions, + b BuiltinPluginLoadingOptions) *PluginConfig { + return &PluginConfig{ + PluginRestrictions: pr, + BpLoadingOptions: b, + } +} diff --git a/kustomize/commands/build/build.go b/kustomize/commands/build/build.go index 6527ca6a6..37c35e0b0 100644 --- a/kustomize/commands/build/build.go +++ b/kustomize/commands/build/build.go @@ -129,7 +129,7 @@ func HonorKustomizeFlags(kOpts *krusty.Options) *krusty.Options { kOpts.DoLegacyResourceSort = getFlagReorderOutput() == legacy kOpts.LoadRestrictions = getFlagLoadRestrictorValue() if theFlags.enable.plugins { - c := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) + c := types.EnabledPluginConfig(types.BploUseStaticallyLinked) c.FnpLoadingOptions = theFlags.fnOptions kOpts.PluginConfig = c }