diff --git a/api/krusty/kustomizer.go b/api/krusty/kustomizer.go index c8b69aaa6..8dd4c17a6 100644 --- a/api/krusty/kustomizer.go +++ b/api/krusty/kustomizer.go @@ -28,11 +28,6 @@ type Kustomizer struct { options *Options } -// MakeDefaultKustomizer returns a Kustomizer with default configuration. -func MakeDefaultKustomizer() *Kustomizer { - return MakeKustomizer(filesys.MakeFsOnDisk(), MakeDefaultOptions()) -} - // MakeKustomizer returns an instance of Kustomizer. func MakeKustomizer(fSys filesys.FileSystem, o *Options) *Kustomizer { return &Kustomizer{fSys: fSys, options: o} diff --git a/api/krusty/options.go b/api/krusty/options.go index d6db3d7b4..101772400 100644 --- a/api/krusty/options.go +++ b/api/krusty/options.go @@ -35,6 +35,6 @@ func MakeDefaultOptions() *Options { DoLegacyResourceSort: true, LoadRestrictions: types.LoadRestrictionsRootOnly, DoPrune: false, - PluginConfig: pgmconfig.DefaultPluginConfig(), + PluginConfig: pgmconfig.DisabledPluginConfig(), } } diff --git a/api/pgmconfig/pluginconfig.go b/api/pgmconfig/pluginconfig.go index cd1e47ea4..18667c191 100644 --- a/api/pgmconfig/pluginconfig.go +++ b/api/pgmconfig/pluginconfig.go @@ -36,19 +36,27 @@ const ( DomainName = "sigs.k8s.io" ) -func ActivePluginConfig() *types.PluginConfig { - pc := DefaultPluginConfig() - pc.PluginRestrictions = types.PluginRestrictionsNone - return pc +func EnabledPluginConfig() *types.PluginConfig { + return MakePluginConfig( + types.PluginRestrictionsNone, DefaultAbsPluginHome()) } -func DefaultPluginConfig() *types.PluginConfig { +func DisabledPluginConfig() *types.PluginConfig { + return MakePluginConfig( + types.PluginRestrictionsBuiltinsOnly, NoPluginHomeSentinal) +} + +func MakePluginConfig( + pr types.PluginRestrictions, home string) *types.PluginConfig { return &types.PluginConfig{ - PluginRestrictions: types.PluginRestrictionsBuiltinsOnly, - AbsPluginHome: DefaultAbsPluginHome(), + PluginRestrictions: pr, + AbsPluginHome: home, } } +// Use an obviously erroneous path, in case it's accidentally used. +const NoPluginHomeSentinal = "/no/non-builtin/plugins!" + func DefaultAbsPluginHome() string { return filepath.Join(configRoot(), RelPluginHome) } diff --git a/api/plugins/compiler/compiler.go b/api/plugins/compiler/compiler.go index 03a166ce9..4f1df36b6 100644 --- a/api/plugins/compiler/compiler.go +++ b/api/plugins/compiler/compiler.go @@ -41,7 +41,7 @@ func DefaultSrcRoot() (string, error) { } nope = append(nope, root) - root = pgmconfig.DefaultPluginConfig().AbsPluginHome + root = pgmconfig.DefaultAbsPluginHome() if FileExists(root) { return root, nil } diff --git a/api/plugins/execplugin/execplugin.go b/api/plugins/execplugin/execplugin.go index fa5754042..4cd9ee802 100644 --- a/api/plugins/execplugin/execplugin.go +++ b/api/plugins/execplugin/execplugin.go @@ -44,15 +44,19 @@ type ExecPlugin struct { h *resmap.PluginHelpers } -func NewExecPlugin(p string) (*ExecPlugin, error) { - f, err := os.Stat(p) +func NewExecPlugin(p string) *ExecPlugin { + return &ExecPlugin{path: p} +} + +func (p *ExecPlugin) ErrIfNotExecutable() error { + f, err := os.Stat(p.path) if err != nil { - return nil, err + return err } if f.Mode()&0111 == 0000 { - return nil, fmt.Errorf("unable to execute plugin on path: %s", p) + return fmt.Errorf("unexecutable plugin at: %s", p.path) } - return &ExecPlugin{path: p}, nil + return nil } func (p *ExecPlugin) Path() string { diff --git a/api/plugins/execplugin/execplugin_test.go b/api/plugins/execplugin/execplugin_test.go index 1efd78923..64b21f1b7 100644 --- a/api/plugins/execplugin/execplugin_test.go +++ b/api/plugins/execplugin/execplugin_test.go @@ -5,7 +5,6 @@ package execplugin_test import ( "fmt" - "os" "strings" "testing" @@ -44,13 +43,15 @@ s/$BAR/bar/g \ \ \ `)) - p, err := NewExecPlugin( + p := NewExecPlugin( loader.AbsolutePluginPath( - pgmconfig.DefaultPluginConfig(), + pgmconfig.DisabledPluginConfig(), pluginConfig.OrgId())) - if err != nil { - t.Fatalf("unexpected error: %v", err.Error()) - } + // Not checking to see if the plugin is executable, + // because this test does not run it. + // This tests only covers sending configuration + // to the plugin wrapper object and confirming + // that it's properly prepared for execution. yaml, err := pluginConfig.AsYAML() if err != nil { @@ -58,7 +59,7 @@ s/$BAR/bar/g } p.Config(resmap.NewPluginHelpers(ldr, v, rf), yaml) - expected := "/kustomize/plugin/someteam.example.com/v1/sedtransformer/SedTransformer" + expected := "someteam.example.com/v1/sedtransformer/SedTransformer" if !strings.HasSuffix(p.Path(), expected) { t.Fatalf("expected suffix '%s', got '%s'", expected, p.Path()) } @@ -118,9 +119,9 @@ func strptr(s string) *string { } func TestUpdateResourceOptions(t *testing.T) { - p, err := NewExecPlugin("") - if !os.IsNotExist(err) { - t.Fatalf("unexpected error: %v", err.Error()) + p := NewExecPlugin("") + if err := p.ErrIfNotExecutable(); err == nil { + t.Fatalf("expected unexecutable error") } rf := resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()) in := resmap.New() @@ -166,9 +167,9 @@ func TestUpdateResourceOptions(t *testing.T) { } func TestUpdateResourceOptionsWithInvalidHashAnnotationValues(t *testing.T) { - p, err := NewExecPlugin("") - if !os.IsNotExist(err) { - t.Fatalf("unexpected error: %v", err.Error()) + p := NewExecPlugin("") + if err := p.ErrIfNotExecutable(); err == nil { + t.Fatalf("expected unexecutable error") } rf := resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()) cases := []string{ diff --git a/api/plugins/loader/loader.go b/api/plugins/loader/loader.go index 3b5ec52db..bdd1c30c2 100644 --- a/api/plugins/loader/loader.go +++ b/api/plugins/loader/loader.go @@ -146,13 +146,23 @@ func (l *Loader) makeBuiltinPlugin(r resid.Gvk) (resmap.Configurable, error) { } func (l *Loader) loadPlugin(resId resid.ResId) (resmap.Configurable, error) { - p, err := execplugin.NewExecPlugin(l.absolutePluginPath(resId)) + // First try to load the plugin as an executable. + p := execplugin.NewExecPlugin(l.absolutePluginPath(resId)) + err := p.ErrIfNotExecutable() if err == nil { return p, nil } if !os.IsNotExist(err) { + // The file exists, but something else is wrong, + // likely it's not executable. + // Assume the user forgot to set the exec bit, + // and return an error, rather than adding ".so" + // to the name and attempting to load it as a Go + // plugin, which will likely fail and result + // in an obscure message. return nil, err } + // Failing the above, try loading it as a Go plugin. c, err := l.loadGoPlugin(resId) if err != nil { return nil, err @@ -186,7 +196,7 @@ func (l *Loader) loadGoPlugin(id resid.ResId) (resmap.Configurable, error) { } c, ok := symbol.(resmap.Configurable) if !ok { - return nil, fmt.Errorf("plugin %s not configurable", regId) + return nil, fmt.Errorf("plugin '%s' not configurable", regId) } registry[regId] = c return copyPlugin(c), nil diff --git a/api/plugins/loader/loader_test.go b/api/plugins/loader/loader_test.go index e44a82135..3a4373030 100644 --- a/api/plugins/loader/loader_test.go +++ b/api/plugins/loader/loader_test.go @@ -57,7 +57,7 @@ func TestLoader(t *testing.T) { ldr := loadertest.NewFakeLoader("/foo") - pLdr := NewLoader(pgmconfig.ActivePluginConfig(), rmF) + pLdr := NewLoader(pgmconfig.EnabledPluginConfig(), rmF) if pLdr == nil { t.Fatal("expect non-nil loader") } diff --git a/api/target/baseandoverlaysmall_test.go b/api/target/baseandoverlaysmall_test.go index 25c2e89e7..d967c344f 100644 --- a/api/target/baseandoverlaysmall_test.go +++ b/api/target/baseandoverlaysmall_test.go @@ -300,7 +300,7 @@ spec: func TestSharedPatchDisAllowed(t *testing.T) { th := kusttest_test.NewKustTestHarnessFull( t, "/app/overlay", - loader.RestrictionRootOnly, pgmconfig.DefaultPluginConfig()) + loader.RestrictionRootOnly, pgmconfig.DisabledPluginConfig()) writeSmallBase(th) th.WriteK("/app/overlay", ` commonLabels: @@ -332,7 +332,7 @@ spec: func TestSharedPatchAllowed(t *testing.T) { th := kusttest_test.NewKustTestHarnessFull( t, "/app/overlay", - loader.RestrictionNone, pgmconfig.DefaultPluginConfig()) + loader.RestrictionNone, pgmconfig.DisabledPluginConfig()) writeSmallBase(th) th.WriteK("/app/overlay", ` commonLabels: diff --git a/api/target/plugindir_test.go b/api/target/plugindir_test.go index dc49e7af3..5c41c440a 100644 --- a/api/target/plugindir_test.go +++ b/api/target/plugindir_test.go @@ -65,7 +65,7 @@ metadata: rf := resmap.NewFactory(resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()), nil) - pl := pLdr.NewLoader(pgmconfig.ActivePluginConfig(), rf) + pl := pLdr.NewLoader(pgmconfig.EnabledPluginConfig(), rf) tg, err := target.NewKustTarget( ldr, valtest_test.MakeFakeValidator(), rf, transformer.NewFactoryImpl(), pl) if err != nil { diff --git a/api/testutils/kusttest/kusttestharness.go b/api/testutils/kusttest/kusttestharness.go index 27aba3f53..c106ef660 100644 --- a/api/testutils/kusttest/kusttestharness.go +++ b/api/testutils/kusttest/kusttestharness.go @@ -36,17 +36,17 @@ type KustTestHarness struct { func NewKustTestHarness(t *testing.T, path string) *KustTestHarness { return NewKustTestHarnessFull( - t, path, fLdr.RestrictionRootOnly, pgmconfig.DefaultPluginConfig()) + t, path, fLdr.RestrictionRootOnly, pgmconfig.DisabledPluginConfig()) } func NewKustTestHarnessAllowPlugins(t *testing.T, path string) *KustTestHarness { return NewKustTestHarnessFull( - t, path, fLdr.RestrictionRootOnly, pgmconfig.ActivePluginConfig()) + t, path, fLdr.RestrictionRootOnly, pgmconfig.EnabledPluginConfig()) } func NewKustTestHarnessNoLoadRestrictor(t *testing.T, path string) *KustTestHarness { return NewKustTestHarnessFull( - t, path, fLdr.RestrictionNone, pgmconfig.DefaultPluginConfig()) + t, path, fLdr.RestrictionNone, pgmconfig.DisabledPluginConfig()) } func NewKustTestHarnessFull( diff --git a/kustomize/internal/commands/build/build.go b/kustomize/internal/commands/build/build.go index 538527852..ac1d462c6 100644 --- a/kustomize/internal/commands/build/build.go +++ b/kustomize/internal/commands/build/build.go @@ -15,7 +15,6 @@ import ( "sigs.k8s.io/kustomize/api/pgmconfig" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" - "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/yaml" ) @@ -103,12 +102,15 @@ func (o *Options) Validate(args []string) (err error) { } func (o *Options) makeOptions() *krusty.Options { - opts := krusty.MakeDefaultOptions() - opts.LoadRestrictions = getFlagLoadRestrictorValue() - opts.DoLegacyResourceSort = o.outOrder == legacy - opts.PluginConfig.PluginRestrictions = types.PluginRestrictionsBuiltinsOnly + opts := &krusty.Options{ + DoLegacyResourceSort: o.outOrder == legacy, + LoadRestrictions: getFlagLoadRestrictorValue(), + DoPrune: false, + } if isFlagEnablePluginsSet() { - opts.PluginConfig.PluginRestrictions = types.PluginRestrictionsNone + opts.PluginConfig = pgmconfig.EnabledPluginConfig() + } else { + opts.PluginConfig = pgmconfig.DisabledPluginConfig() } return opts }