Merge pull request #2385 from monopole/pluginLoader

Plugin flow doc improvements and a new option for the plugin loader

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
+// FileYoungerThan returns true if the file age is <= the Duration argument.
-// accidentally left over from some earlier build.
+func FileYoungerThan(path string, d time.Duration) bool {
-func RecentFileExists(path string) bool {
 	fi, err := os.Stat(path)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return false
 		}
 	}
-	age := time.Since(fi.ModTime())
+	return time.Since(fi.ModTime()) <= d
-	return age.Minutes() < 3
 }
 
 func FileExists(name string) bool {

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)
 	}
 

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
+		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())
-	} else if l.pc.PluginRestrictions == types.PluginRestrictionsNone {
+		default:
-		c, err = l.loadPlugin(res.OrgId())
+			err = fmt.Errorf(
+				"unknown plugin loader behavior specified: %v",
+				l.pc.BpLoadingOptions)
+		}
 	} else {
+		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

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,7 +59,15 @@ func TestLoader(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	c, err := konfig.EnabledPluginConfig()
+	generatorConfigs, err := rmF.NewResMapFromBytes([]byte(
+		someServiceGenerator + "---\n" + secretGenerator))
+	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)
 		}
@@ -66,14 +75,10 @@ func TestLoader(t *testing.T) {
 		if pLdr == nil {
 			t.Fatal("expect non-nil loader")
 		}
-	m, err := rmF.NewResMapFromBytes([]byte(
+		_, err = pLdr.LoadGenerators(
-		someServiceGenerator + "---\n" + secretGenerator))
+			fLdr, valtest_test.MakeFakeValidator(), generatorConfigs)
 		if err != nil {
 			t.Fatal(err)
 		}
-	_, err = pLdr.LoadGenerators(
-		fLdr, valtest_test.MakeFakeValidator(), m)
-	if err != nil {
-		t.Fatal(err)
 	}
 }

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.MakeHarness(t)
-	th := kusttest_test.MakeHarnessWithFs(t, fSys)
 	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)
 	}

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{

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(

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 creates Resources from byte streams.
 	rf *resmap.Factory
+
+	// A file loader using the Harness.fSys to read test data.
 	ldr ifc.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)
+	th.pte.prepareExecPlugin(g, v, k)
-	return th
-}
-
-func (th *HarnessEnhanced) PrepBuiltin(k string) *HarnessEnhanced {
-	th.pte.buildGoPlugin(konfig.BuiltinPluginPackage, "", 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{

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,
+// maybe makes and removes a temporary working directory,
-// and sets/resets shell env vars as needed.
+// 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.
 // 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)

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
+	// kustomize reads plugin configuration data from a file path
-	// of seeing the file name in the 'generators:' or 'transformers:'
+	// specified in the 'generators:' or 'transformers:' field of a
-	// field in a kustomization file), it must then locate the plugin
+	// kustomization file.  kustomize must then use this data to both
-	// code (Go plugin or exec plugin).
+	// locate the plugin and configure it.
-	// Every kustomize plugin (its code, its tests, supporting data
+	// 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
+	// The value of AbsPluginHome can be any absolute path.
-	// default to $XDG_CONFIG_HOME/kustomize/plugin.
 	AbsPluginHome string
 
-	// PluginRestrictions defines the plugin restriction state.
+	// PluginRestrictions distinguishes plugin restrictions.
-	// See type for more information.
 	PluginRestrictions PluginRestrictions
+
+	// BpLoadingOptions distinguishes builtin plugin behaviors.
+	BpLoadingOptions BuiltinPluginLoadingOptions
 }

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
+)

hack/buildExternalGoPlugins.sh

@@ -1,4 +1,4 @@
-#!/usr/bin/env sh
+#!/bin/bash
 set -e
 
 # Builds or removes Go plugin object code.

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)
 		}