diff --git a/api/internal/plugins/compiler/compiler.go b/api/internal/plugins/compiler/compiler.go index 809fada48..41ef8022a 100644 --- a/api/internal/plugins/compiler/compiler.go +++ b/api/internal/plugins/compiler/compiler.go @@ -119,7 +119,7 @@ func (b *Compiler) Compile(g, v, k string) error { lowK := strings.ToLower(k) objDir := filepath.Join(b.objRoot, g, v, lowK) objFile := filepath.Join(objDir, k) + ".so" - if RecentFileExists(objFile) { + if FileYoungerThan(objFile, time.Minute) { // Skip rebuilding it. return nil } @@ -134,7 +134,6 @@ func (b *Compiler) Compile(g, v, k string) error { if !FileExists(s) { return fmt.Errorf( "cannot find source at '%s' or '%s'", srcFile, s) - } srcFile = s } @@ -160,17 +159,15 @@ func (b *Compiler) Compile(g, v, k string) error { return nil } -// True if file less than 3 minutes old, i.e. not -// accidentally left over from some earlier build. -func RecentFileExists(path string) bool { +// FileYoungerThan returns true if the file age is <= the Duration argument. +func FileYoungerThan(path string, d time.Duration) bool { fi, err := os.Stat(path) if err != nil { if os.IsNotExist(err) { return false } } - age := time.Since(fi.ModTime()) - return age.Minutes() < 3 + return time.Since(fi.ModTime()) <= d } func FileExists(name string) bool { diff --git a/api/internal/plugins/compiler/compiler_test.go b/api/internal/plugins/compiler/compiler_test.go index 28eee0ede..e30c1ca14 100644 --- a/api/internal/plugins/compiler/compiler_test.go +++ b/api/internal/plugins/compiler/compiler_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "testing" + "time" "sigs.k8s.io/kustomize/api/filesys" . "sigs.k8s.io/kustomize/api/internal/plugins/compiler" @@ -38,7 +39,7 @@ func TestCompiler(t *testing.T) { if err != nil { t.Error(err) } - if !RecentFileExists(expectObj) { + if !FileYoungerThan(expectObj, time.Second) { t.Errorf("didn't find expected obj file %s", expectObj) } @@ -52,7 +53,7 @@ func TestCompiler(t *testing.T) { if err != nil { t.Error(err) } - if !RecentFileExists(expectObj) { + if !FileYoungerThan(expectObj, time.Second) { t.Errorf("didn't find expected obj file %s", expectObj) } diff --git a/api/internal/plugins/loader/loader.go b/api/internal/plugins/loader/loader.go index fc18d6a75..76e8ac611 100644 --- a/api/internal/plugins/loader/loader.go +++ b/api/internal/plugins/loader/loader.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/kustomize/api/types" ) +// Loader loads plugins using a file loader (a different loader). type Loader struct { pc *types.PluginConfig rf *resmap.Factory @@ -107,17 +108,35 @@ func isBuiltinPlugin(res *resource.Resource) bool { } func (l *Loader) loadAndConfigurePlugin( - ldr ifc.Loader, v ifc.Validator, res *resource.Resource) (c resmap.Configurable, err error) { + ldr ifc.Loader, + v ifc.Validator, + res *resource.Resource) (c resmap.Configurable, err error) { if isBuiltinPlugin(res) { - // Instead of looking for and loading a .so file, just - // instantiate the plugin from a generated factory - // function (see "pluginator"). Being able to do this - // is what makes a plugin "builtin". - c, err = l.makeBuiltinPlugin(res.GetGvk()) - } else if l.pc.PluginRestrictions == types.PluginRestrictionsNone { - c, err = l.loadPlugin(res.OrgId()) + switch l.pc.BpLoadingOptions { + case types.BploLoadFromFileSys: + c, err = l.loadPlugin(res.OrgId()) + case types.BploUseStaticallyLinked: + // Instead of looking for and loading a .so file, + // instantiate the plugin from a generated factory + // function (see "pluginator"). Being able to do this + // is what makes a plugin "builtin". + c, err = l.makeBuiltinPlugin(res.GetGvk()) + default: + err = fmt.Errorf( + "unknown plugin loader behavior specified: %v", + l.pc.BpLoadingOptions) + } } else { - err = types.NewErrOnlyBuiltinPluginsAllowed(res.OrgId().Kind) + switch l.pc.PluginRestrictions { + case types.PluginRestrictionsNone: + c, err = l.loadPlugin(res.OrgId()) + case types.PluginRestrictionsBuiltinsOnly: + err = types.NewErrOnlyBuiltinPluginsAllowed(res.OrgId().Kind) + default: + err = fmt.Errorf( + "unknown plugin restriction specified: %v", + l.pc.PluginRestrictions) + } } if err != nil { return nil, err diff --git a/api/internal/plugins/loader/loader_test.go b/api/internal/plugins/loader/loader_test.go index 22c66ba57..4e644ac4e 100644 --- a/api/internal/plugins/loader/loader_test.go +++ b/api/internal/plugins/loader/loader_test.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/kustomize/api/resource" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest" + "sigs.k8s.io/kustomize/api/types" ) const ( @@ -58,22 +59,26 @@ func TestLoader(t *testing.T) { if err != nil { t.Fatal(err) } - c, err := konfig.EnabledPluginConfig() - if err != nil { - t.Fatal(err) - } - pLdr := NewLoader(c, rmF) - if pLdr == nil { - t.Fatal("expect non-nil loader") - } - m, err := rmF.NewResMapFromBytes([]byte( + generatorConfigs, err := rmF.NewResMapFromBytes([]byte( someServiceGenerator + "---\n" + secretGenerator)) if err != nil { t.Fatal(err) } - _, err = pLdr.LoadGenerators( - fLdr, valtest_test.MakeFakeValidator(), m) - if err != nil { - t.Fatal(err) + for _, behavior := range []types.BuiltinPluginLoadingOptions{ + types.BploUseStaticallyLinked, + types.BploLoadFromFileSys} { + c, err := konfig.EnabledPluginConfig(behavior) + if err != nil { + t.Fatal(err) + } + pLdr := NewLoader(c, rmF) + if pLdr == nil { + t.Fatal("expect non-nil loader") + } + _, err = pLdr.LoadGenerators( + fLdr, valtest_test.MakeFakeValidator(), generatorConfigs) + if err != nil { + t.Fatal(err) + } } } diff --git a/api/internal/target/kusttarget_test.go b/api/internal/target/kusttarget_test.go index 8e7060248..add7fc155 100644 --- a/api/internal/target/kusttarget_test.go +++ b/api/internal/target/kusttarget_test.go @@ -7,7 +7,6 @@ import ( "encoding/base64" "testing" - "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/k8sdeps/kunstruct" "sigs.k8s.io/kustomize/api/resmap" @@ -19,8 +18,7 @@ import ( // high level tests. func TestMakeCustomizedResMap(t *testing.T) { - fSys := filesys.MakeFsInMemory() - th := kusttest_test.MakeHarnessWithFs(t, fSys) + th := kusttest_test.MakeHarness(t) th.WriteK("/whatever", ` apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization @@ -168,7 +166,7 @@ metadata: } actual, err := makeKustTargetWithRf( - t, fSys, "/whatever", resFactory).MakeCustomizedResMap() + t, th.GetFSys(), "/whatever", resFactory).MakeCustomizedResMap() if err != nil { t.Fatalf("unexpected Resources error %v", err) } diff --git a/api/konfig/plugins.go b/api/konfig/plugins.go index 1fd571f87..a42e5e296 100644 --- a/api/konfig/plugins.go +++ b/api/konfig/plugins.go @@ -35,37 +35,46 @@ const ( // Domain from which kustomize code is imported, for locating // plugin source code under $GOPATH when GOPATH is defined. DomainName = "sigs.k8s.io" + + // Injected into plugin paths when plugins are disabled. + // Provides a clue in flows that shouldn't happen. + NoPluginHomeSentinal = "/No/non-builtin/plugins!" ) -func EnabledPluginConfig() (*types.PluginConfig, error) { +func EnabledPluginConfig(b types.BuiltinPluginLoadingOptions) (*types.PluginConfig, error) { dir, err := DefaultAbsPluginHome(filesys.MakeFsOnDisk()) if err != nil { return nil, err } - return MakePluginConfig(types.PluginRestrictionsNone, dir), nil + return MakePluginConfig(types.PluginRestrictionsNone, b, dir), nil } func DisabledPluginConfig() *types.PluginConfig { return MakePluginConfig( - types.PluginRestrictionsBuiltinsOnly, NoPluginHomeSentinal) + types.PluginRestrictionsBuiltinsOnly, + types.BploUseStaticallyLinked, + NoPluginHomeSentinal) } func MakePluginConfig( - pr types.PluginRestrictions, home string) *types.PluginConfig { + pr types.PluginRestrictions, + b types.BuiltinPluginLoadingOptions, + home string) *types.PluginConfig { return &types.PluginConfig{ PluginRestrictions: pr, AbsPluginHome: home, + BpLoadingOptions: b, } } -// Use an obviously erroneous path, in case it's accidentally used. -const NoPluginHomeSentinal = "/no/non-builtin/plugins!" - type NotedFunc struct { Note string F func() string } +// DefaultAbsPluginHome returns the absolute path in the given file +// system to first directory that looks like a good candidate for +// the home of kustomize plugins. func DefaultAbsPluginHome(fSys filesys.FileSystem) (string, error) { return FirstDirThatExistsElseError( "plugin home directory", fSys, []NotedFunc{ diff --git a/api/testutils/kusttest/harness.go b/api/testutils/kusttest/harness.go index 3fd19305a..ef059b03a 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -70,7 +70,8 @@ func (th Harness) MakeOptionsPluginsDisabled() krusty.Options { // Enables use of non-builtin plugins. func (th Harness) MakeOptionsPluginsEnabled() krusty.Options { - c, err := konfig.EnabledPluginConfig() + // TODO: Change to types.BploLoadFromFileSys to enable debugging. + c, err := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) if err != nil { if strings.Contains(err.Error(), "unable to find plugin root") { th.t.Log( diff --git a/api/testutils/kusttest/harnessenhanced.go b/api/testutils/kusttest/harnessenhanced.go index 4fb3ac6fa..40b2d46f0 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -19,38 +19,51 @@ import ( "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest" + "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/yaml" ) // HarnessEnhanced manages a full plugin environment for tests. type HarnessEnhanced struct { + // An instance of *testing.T, and a filesystem (likely in-memory) + // for loading test data - plugin config, resources to transform, etc. Harness + + // plugintestEnv holds the plugin compiler and data needed to + // create compilation sub-processes. pte *pluginTestEnv - rf *resmap.Factory + + // rf creates Resources from byte streams. + rf *resmap.Factory + + // A file loader using the Harness.fSys to read test data. ldr ifc.Loader - pl *pLdr.Loader + + // A plugin loader that loads plugins from a (real) file system. + pl *pLdr.Loader } func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced { pte := newPluginTestEnv(t).set() - pc, err := konfig.EnabledPluginConfig() + // TODO: Change to types.BploLoadFromFileSys to enable debugging. + pc, err := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) if err != nil { t.Fatal(err) } - fSys := filesys.MakeFsInMemory() - rf := resmap.NewFactory( resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()), transformer.NewFactoryImpl()) result := &HarnessEnhanced{ - Harness: Harness{t: t, fSys: fSys}, + Harness: MakeHarness(t), pte: pte, rf: rf, pl: pLdr.NewLoader(pc, rf)} + + // Point the file loader to the root ('/') of the in-memory file system. result.ResetLoaderRoot(filesys.Separator) return result @@ -60,21 +73,23 @@ func (th *HarnessEnhanced) Reset() { th.pte.reset() } +func (th *HarnessEnhanced) PrepBuiltin(k string) *HarnessEnhanced { + return th.BuildGoPlugin(konfig.BuiltinPluginPackage, "", k) +} + func (th *HarnessEnhanced) BuildGoPlugin(g, v, k string) *HarnessEnhanced { - th.pte.buildGoPlugin(g, v, k) + th.pte.prepareGoPlugin(g, v, k) return th } func (th *HarnessEnhanced) PrepExecPlugin(g, v, k string) *HarnessEnhanced { - th.pte.prepExecPlugin(g, v, k) - return th -} - -func (th *HarnessEnhanced) PrepBuiltin(k string) *HarnessEnhanced { - th.pte.buildGoPlugin(konfig.BuiltinPluginPackage, "", k) + th.pte.prepareExecPlugin(g, v, k) return th } +// ResetLoaderRoot interprets its argument as an absolute directory path. +// It creates the directory, and creates the harness's file loader +// rooted in that directory. func (th *HarnessEnhanced) ResetLoaderRoot(root string) { if err := th.fSys.Mkdir(root); err != nil { th.t.Fatal(err) @@ -137,7 +152,6 @@ func toggleYamlSupportField(config string, yamlSupport bool) (string, error) { Reader: bytes.NewBufferString(config), Writer: &out, } - err := kio.Pipeline{ Inputs: []kio.Reader{&rw}, Filters: []kio.Filter{ diff --git a/api/testutils/kusttest/plugintestenv.go b/api/testutils/kusttest/plugintestenv.go index fbc558c94..69adc23ec 100644 --- a/api/testutils/kusttest/plugintestenv.go +++ b/api/testutils/kusttest/plugintestenv.go @@ -16,10 +16,10 @@ import ( "sigs.k8s.io/kustomize/api/konfig" ) -// pluginTestEnv manages plugins for tests. +// pluginTestEnv manages compiling plugins for tests. // It manages a Go plugin compiler, -// makes and removes a temporary working directory, -// and sets/resets shell env vars as needed. +// maybe makes and removes a temporary working directory, +// maybe sets/resets shell env vars as needed. type pluginTestEnv struct { t *testing.T compiler *compiler.Compiler @@ -56,19 +56,19 @@ func (x *pluginTestEnv) reset() { x.removeWorkDir() } -// buildGoPlugin compiles a Go plugin, leaving the newly +// prepareGoPlugin compiles a Go plugin, leaving the newly // created object code in the right place - a temporary -// working directory pointed to by KustomizePluginHomeEnv. +// working directory pointed to by KustomizePluginHomeEnv. // This avoids overwriting anything the user/developer has // otherwise created. -func (x *pluginTestEnv) buildGoPlugin(g, v, k string) { +func (x *pluginTestEnv) prepareGoPlugin(g, v, k string) { err := x.compiler.Compile(g, v, k) if err != nil { x.t.Errorf("compile failed: %v", err) } } -// prepExecPlugin copies an exec plugin from it's +// prepareExecPlugin copies an exec plugin from it's // home in the discovered srcRoot to the same temp // directory where Go plugin object code is placed. // Kustomize (and its tests) expect to find plugins @@ -76,7 +76,7 @@ func (x *pluginTestEnv) buildGoPlugin(g, v, k string) { // framework is compiling Go plugins to a temp dir, // it must likewise copy Exec plugins to that same // temp dir. -func (x *pluginTestEnv) prepExecPlugin(g, v, k string) { +func (x *pluginTestEnv) prepareExecPlugin(g, v, k string) { lowK := strings.ToLower(k) src := filepath.Join(x.srcRoot, g, v, lowK, k) tmp := filepath.Join(x.workDir, g, v, lowK, k) diff --git a/api/types/pluginconfig.go b/api/types/pluginconfig.go index 3d3aaa73a..88c0ade77 100644 --- a/api/types/pluginconfig.go +++ b/api/types/pluginconfig.go @@ -10,22 +10,23 @@ type PluginConfig struct { // containing the fields 'apiVersion' and 'kind', e.g. // apiVersion: apps/v1 // kind: Deployment - // When kustomize reads a plugin configuration file (as as result - // of seeing the file name in the 'generators:' or 'transformers:' - // field in a kustomization file), it must then locate the plugin - // code (Go plugin or exec plugin). - // Every kustomize plugin (its code, its tests, supporting data + // 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, but might - // default to $XDG_CONFIG_HOME/kustomize/plugin. + // The value of AbsPluginHome can be any absolute path. AbsPluginHome string - // PluginRestrictions defines the plugin restriction state. - // See type for more information. + // PluginRestrictions distinguishes plugin restrictions. PluginRestrictions PluginRestrictions + + // BpLoadingOptions distinguishes builtin plugin behaviors. + BpLoadingOptions BuiltinPluginLoadingOptions } diff --git a/api/types/pluginrestrictions.go b/api/types/pluginrestrictions.go index 166086757..a9953c00f 100644 --- a/api/types/pluginrestrictions.go +++ b/api/types/pluginrestrictions.go @@ -26,3 +26,18 @@ const ( // No restrictions, do whatever you want. PluginRestrictionsNone ) + +// BuiltinPluginLoadingOptions distinguish ways in which builtin plugins are used. +//go:generate stringer -type=BuiltinPluginLoadingOptions +type BuiltinPluginLoadingOptions int + +const ( + BploUndefined BuiltinPluginLoadingOptions = iota + + // Desired in production use for performance. + BploUseStaticallyLinked + + // Desired in testing and development cycles where it's undesirable + // to generate static code. + BploLoadFromFileSys +) diff --git a/hack/buildExternalGoPlugins.sh b/hack/buildExternalGoPlugins.sh index ad4aec1f7..65bd4bbc9 100755 --- a/hack/buildExternalGoPlugins.sh +++ b/hack/buildExternalGoPlugins.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env sh +#!/bin/bash set -e # Builds or removes Go plugin object code. @@ -40,8 +40,8 @@ function removePlugin { } goPlugins=$( - find $root -name "*.go" | - grep -v builtin/ | + find $root -name "*.go" | + grep -v builtin/ | xargs grep -l "var KustomizePlugin") for p in $goPlugins; do diff --git a/kustomize/internal/commands/build/build.go b/kustomize/internal/commands/build/build.go index 21af80347..0c356f536 100644 --- a/kustomize/internal/commands/build/build.go +++ b/kustomize/internal/commands/build/build.go @@ -16,6 +16,7 @@ import ( "sigs.k8s.io/kustomize/api/krusty" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" + "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/yaml" ) @@ -107,7 +108,7 @@ func (o *Options) makeOptions() *krusty.Options { DoPrune: false, } if isFlagEnablePluginsSet() { - c, err := konfig.EnabledPluginConfig() + c, err := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) if err != nil { log.Fatal(err) }