diff --git a/api/internal/plugins/execplugin/execplugin_test.go b/api/internal/plugins/execplugin/execplugin_test.go index f943a2464..2f7e42bcc 100644 --- a/api/internal/plugins/execplugin/execplugin_test.go +++ b/api/internal/plugins/execplugin/execplugin_test.go @@ -45,10 +45,12 @@ s/$BAR/bar baz/g }) pluginConfig.RemoveBuildAnnotations() - p := NewExecPlugin( - pLdr.AbsolutePluginPath( - konfig.DisabledPluginConfig(), - pluginConfig.OrgId())) + loader := pLdr.NewLoader(konfig.DisabledPluginConfig(), rf) + pluginPath, err := loader.AbsolutePluginPath(pluginConfig.OrgId()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + p := NewExecPlugin(pluginPath) // Not checking to see if the plugin is executable, // because this test does not run it. // This tests only covers sending configuration diff --git a/api/internal/plugins/loader/loader.go b/api/internal/plugins/loader/loader.go index aaa7c3600..34b9731af 100644 --- a/api/internal/plugins/loader/loader.go +++ b/api/internal/plugins/loader/loader.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/pkg/errors" + "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/plugins/builtinhelpers" "sigs.k8s.io/kustomize/api/internal/plugins/execplugin" @@ -29,6 +30,10 @@ import ( type Loader struct { pc *types.PluginConfig rf *resmap.Factory + + // absolutePluginHome caches the location of a valid plugin root directory. + // It should only be set once the directory's existence has been confirmed. + absolutePluginHome string } func NewLoader( @@ -95,13 +100,47 @@ func relativePluginPath(id resid.ResId) string { strings.ToLower(id.Kind)) } -func AbsolutePluginPath(pc *types.PluginConfig, id resid.ResId) string { - return filepath.Join( - pc.AbsPluginHome, relativePluginPath(id), id.Kind) +func (l *Loader) AbsolutePluginPath(id resid.ResId) (string, error) { + pluginHome, err := l.absPluginHome() + if err != nil { + return "", err + } + return filepath.Join(pluginHome, relativePluginPath(id), id.Kind), nil } -func (l *Loader) absolutePluginPath(id resid.ResId) string { - return AbsolutePluginPath(l.pc, id) +// absPluginHome is the home of kustomize Exec and Go plugins. +// Kustomize plugin configuration files are k8s-style objects +// containing the fields 'apiVersion' and 'kind', e.g. +// apiVersion: apps/v1 +// kind: Deployment +// kustomize reads plugin configuration data from a file path +// specified in the 'generators:' or 'transformers:' field of a +// kustomization file. For Exec and Go plugins, kustomize +// uses this data to both locate the plugin and configure it. +// Each Exec or Go plugin (its code, its tests, its supporting data +// files, etc.) must be housed in its own directory at +// ${absPluginHome}/${pluginApiVersion}/LOWERCASE(${pluginKind}) +// where +// - ${absPluginHome} is an absolute path, defined below. +// - ${pluginApiVersion} is taken from the plugin config file. +// - ${pluginKind} is taken from the plugin config file. +func (l *Loader) absPluginHome() (string, error) { + // External plugins are disabled--return the dummy plugin root. + if l.pc.PluginRestrictions != types.PluginRestrictionsNone { + return konfig.NoPluginHomeSentinal, nil + } + // We've already determined plugin home--use the cached value. + if l.absolutePluginHome != "" { + return l.absolutePluginHome, nil + } + + // Check default locations for a valid plugin root, and cache it if found. + dir, err := konfig.DefaultAbsPluginHome(filesys.MakeFsOnDisk()) + if err != nil { + return "", err + } + l.absolutePluginHome = dir + return l.absolutePluginHome, nil } func isBuiltinPlugin(res *resource.Resource) bool { @@ -176,10 +215,13 @@ func (l *Loader) loadPlugin(res *resource.Resource) (resmap.Configurable, error) } func (l *Loader) loadExecOrGoPlugin(resId resid.ResId) (resmap.Configurable, error) { + absPluginPath, err := l.AbsolutePluginPath(resId) + if err != nil { + return nil, err + } // First try to load the plugin as an executable. - p := execplugin.NewExecPlugin(l.absolutePluginPath(resId)) - err := p.ErrIfNotExecutable() - if err == nil { + p := execplugin.NewExecPlugin(absPluginPath) + if err = p.ErrIfNotExecutable(); err == nil { return p, nil } if !os.IsNotExist(err) { @@ -193,7 +235,7 @@ func (l *Loader) loadExecOrGoPlugin(resId resid.ResId) (resmap.Configurable, err return nil, err } // Failing the above, try loading it as a Go plugin. - c, err := l.loadGoPlugin(resId) + c, err := l.loadGoPlugin(resId, absPluginPath+".so") if err != nil { return nil, err } @@ -208,12 +250,11 @@ func (l *Loader) loadExecOrGoPlugin(resId resid.ResId) (resmap.Configurable, err // as a Loader instance variable. So make it a package variable. var registry = make(map[string]resmap.Configurable) -func (l *Loader) loadGoPlugin(id resid.ResId) (resmap.Configurable, error) { +func (l *Loader) loadGoPlugin(id resid.ResId, absPath string) (resmap.Configurable, error) { regId := relativePluginPath(id) if c, ok := registry[regId]; ok { return copyPlugin(c), nil } - absPath := l.absolutePluginPath(id) + ".so" if !utils.FileExists(absPath) { return nil, fmt.Errorf( "expected file with Go object code at: %s", absPath) diff --git a/api/internal/plugins/loader/loader_test.go b/api/internal/plugins/loader/loader_test.go index 451617965..3930eaa72 100644 --- a/api/internal/plugins/loader/loader_test.go +++ b/api/internal/plugins/loader/loader_test.go @@ -66,10 +66,7 @@ func TestLoader(t *testing.T) { for _, behavior := range []types.BuiltinPluginLoadingOptions{ /* types.BploUseStaticallyLinked, types.BploLoadFromFileSys */} { - c, err := konfig.EnabledPluginConfig(behavior) - if err != nil { - t.Fatal(err) - } + c := konfig.EnabledPluginConfig(behavior) pLdr := NewLoader(c, rmF) if pLdr == nil { t.Fatal("expect non-nil loader") diff --git a/api/internal/plugins/utils/utils.go b/api/internal/plugins/utils/utils.go index 12dd6e89f..7fa461191 100644 --- a/api/internal/plugins/utils/utils.go +++ b/api/internal/plugins/utils/utils.go @@ -34,7 +34,7 @@ func GoBin() string { // has her ${g}/${v}/$lower(${k})/${k}.go files. func DeterminePluginSrcRoot(fSys filesys.FileSystem) (string, error) { return konfig.FirstDirThatExistsElseError( - "source directory", fSys, []konfig.NotedFunc{ + "plugin src root", fSys, []konfig.NotedFunc{ { Note: "relative to unit test", F: func() string { diff --git a/api/konfig/plugins.go b/api/konfig/plugins.go index a42e5e296..30f0b05f2 100644 --- a/api/konfig/plugins.go +++ b/api/konfig/plugins.go @@ -41,28 +41,20 @@ const ( NoPluginHomeSentinal = "/No/non-builtin/plugins!" ) -func EnabledPluginConfig(b types.BuiltinPluginLoadingOptions) (*types.PluginConfig, error) { - dir, err := DefaultAbsPluginHome(filesys.MakeFsOnDisk()) - if err != nil { - return nil, err - } - return MakePluginConfig(types.PluginRestrictionsNone, b, dir), nil +func EnabledPluginConfig(b types.BuiltinPluginLoadingOptions) *types.PluginConfig { + return MakePluginConfig(types.PluginRestrictionsNone, b) } func DisabledPluginConfig() *types.PluginConfig { return MakePluginConfig( types.PluginRestrictionsBuiltinsOnly, - types.BploUseStaticallyLinked, - NoPluginHomeSentinal) + types.BploUseStaticallyLinked) } -func MakePluginConfig( - pr types.PluginRestrictions, - b types.BuiltinPluginLoadingOptions, - home string) *types.PluginConfig { +func MakePluginConfig(pr types.PluginRestrictions, + b types.BuiltinPluginLoadingOptions) *types.PluginConfig { return &types.PluginConfig{ PluginRestrictions: pr, - AbsPluginHome: home, BpLoadingOptions: b, } } @@ -77,7 +69,7 @@ type NotedFunc struct { // the home of kustomize plugins. func DefaultAbsPluginHome(fSys filesys.FileSystem) (string, error) { return FirstDirThatExistsElseError( - "plugin home directory", fSys, []NotedFunc{ + "plugin root", fSys, []NotedFunc{ { Note: "homed in $" + KustomizePluginHomeEnv, F: func() string { diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index 4894f36c4..cef7e6fb2 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -8,8 +8,8 @@ import ( ) func TestFnExecGenerator(t *testing.T) { - th := kusttest_test.MakeEnhancedHarness(t) - defer th.Reset() + // Function plugins should not need the env setup done by MakeEnhancedHarness + th := kusttest_test.MakeHarness(t) th.WriteK(".", ` resources: @@ -92,8 +92,8 @@ func skipIfNoDocker(t *testing.T) { func TestFnContainerGenerator(t *testing.T) { skipIfNoDocker(t) - th := kusttest_test.MakeEnhancedHarness(t) - defer th.Reset() + // Function plugins should not need the env setup done by MakeEnhancedHarness + th := kusttest_test.MakeHarness(t) th.WriteK(".", ` resources: @@ -308,8 +308,8 @@ spec: func TestFnContainerTransformer(t *testing.T) { skipIfNoDocker(t) - th := kusttest_test.MakeEnhancedHarness(t) - defer th.Reset() + // Function plugins should not need the env setup done by MakeEnhancedHarness + th := kusttest_test.MakeHarness(t) th.WriteK(".", ` resources: @@ -408,8 +408,8 @@ spec: func TestFnContainerTransformerWithConfig(t *testing.T) { skipIfNoDocker(t) - th := kusttest_test.MakeEnhancedHarness(t) - defer th.Reset() + // Function plugins should not need the env setup done by MakeEnhancedHarness + th := kusttest_test.MakeHarness(t) th.WriteK(".", ` resources: @@ -468,8 +468,8 @@ metadata: func TestFnContainerEnvVars(t *testing.T) { skipIfNoDocker(t) - th := kusttest_test.MakeEnhancedHarness(t) - defer th.Reset() + // Function plugins should not need the env setup done by MakeEnhancedHarness + th := kusttest_test.MakeHarness(t) th.WriteK(".", ` generators: diff --git a/api/testutils/kusttest/harness.go b/api/testutils/kusttest/harness.go index a3adbe6dd..19db91ddc 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -5,7 +5,6 @@ package kusttest_test import ( "path/filepath" - "strings" "testing" "sigs.k8s.io/kustomize/api/filesys" @@ -77,15 +76,7 @@ func (th Harness) MakeOptionsPluginsDisabled() krusty.Options { // Enables use of non-builtin plugins. func (th Harness) MakeOptionsPluginsEnabled() krusty.Options { - pc, err := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) - if err != nil { - if strings.Contains(err.Error(), "unable to find plugin root") { - th.t.Log( - "Tests that want to run with plugins enabled must be " + - "bookended by calls to MakeEnhancedHarness(), Reset().") - } - th.t.Fatal(err) - } + pc := konfig.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 ba4a613c5..955abd6d2 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -40,11 +40,8 @@ type HarnessEnhanced struct { func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced { pte := newPluginTestEnv(t).set() - pc, err := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) + pc := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) pc.FnpLoadingOptions.EnableStar = true - if err != nil { - t.Fatal(err) - } p := provider.NewDefaultDepProvider() resourceFactory := p.GetResourceFactory() resmapFactory := resmap.NewFactory(resourceFactory) diff --git a/api/types/errunabletofind.go b/api/types/errunabletofind.go index a39e9a385..f95b8edd5 100644 --- a/api/types/errunabletofind.go +++ b/api/types/errunabletofind.go @@ -23,7 +23,7 @@ func (e *errUnableToFind) Error() string { m = append(m, "('"+p.Value+"'; "+p.Key+")") } return fmt.Sprintf( - "unable to find plugin root - tried: %s", strings.Join(m, ", ")) + "unable to find %s - tried: %s", e.what, strings.Join(m, ", ")) } func NewErrUnableToFind(w string, a []Pair) *errUnableToFind { diff --git a/api/types/pluginconfig.go b/api/types/pluginconfig.go index 2756f9826..d058ef30b 100644 --- a/api/types/pluginconfig.go +++ b/api/types/pluginconfig.go @@ -5,25 +5,6 @@ package types // PluginConfig holds plugin configuration. type PluginConfig struct { - // AbsPluginHome is the home of kustomize plugins. - // Kustomize plugin configuration files are k8s-style objects - // containing the fields 'apiVersion' and 'kind', e.g. - // apiVersion: apps/v1 - // kind: Deployment - // kustomize reads plugin configuration data from a file path - // specified in the 'generators:' or 'transformers:' field of a - // kustomization file. kustomize must then use this data to both - // locate the plugin and configure it. - // Every kustomize plugin (its code, its tests, its supporting data - // files, etc.) must be housed in its own directory at - // ${AbsPluginHome}/${pluginApiVersion}/LOWERCASE(${pluginKind}) - // where - // - ${AbsPluginHome} is an absolute path, defined below. - // - ${pluginApiVersion} is taken from the plugin config file. - // - ${pluginKind} is taken from the plugin config file. - // The value of AbsPluginHome can be any absolute path. - AbsPluginHome string - // PluginRestrictions distinguishes plugin restrictions. PluginRestrictions PluginRestrictions diff --git a/kustomize/commands/build/build.go b/kustomize/commands/build/build.go index c7069bfe2..6527ca6a6 100644 --- a/kustomize/commands/build/build.go +++ b/kustomize/commands/build/build.go @@ -6,7 +6,6 @@ package build import ( "fmt" "io" - "log" "github.com/spf13/cobra" "sigs.k8s.io/kustomize/api/filesys" @@ -130,10 +129,7 @@ func HonorKustomizeFlags(kOpts *krusty.Options) *krusty.Options { kOpts.DoLegacyResourceSort = getFlagReorderOutput() == legacy kOpts.LoadRestrictions = getFlagLoadRestrictorValue() if theFlags.enable.plugins { - c, err := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) - if err != nil { - log.Fatal(err) - } + c := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) c.FnpLoadingOptions = theFlags.fnOptions kOpts.PluginConfig = c }