Simplify plugin loader code.

* use one place to build plugin file names,
 * use one loader instance,
 * test for plugin enabled flag in just one place to
   avoid errors and reduce if statements,
 * don't return private objects,
 * factor goplugin loading to a method,
 * fix a related test that was commented out.
This commit is contained in:
jregan
2019-04-21 16:12:55 -07:00
committed by Jeffrey Regan
parent c9bf70fd4b
commit 76a3179868
13 changed files with 169 additions and 154 deletions

View File

@@ -29,9 +29,9 @@ import (
"sigs.k8s.io/kustomize/pkg/ifc/transformer"
"sigs.k8s.io/kustomize/pkg/loader"
"sigs.k8s.io/kustomize/pkg/pgmconfig"
"sigs.k8s.io/kustomize/pkg/plugins"
"sigs.k8s.io/kustomize/pkg/resmap"
"sigs.k8s.io/kustomize/pkg/target"
"sigs.k8s.io/kustomize/pkg/types"
)
// Options contain the options for running a build
@@ -70,7 +70,7 @@ func NewCmdBuild(
out io.Writer, fs fs.FileSystem,
rf *resmap.Factory,
ptf transformer.Factory,
pc *types.PluginConfig) *cobra.Command {
pl *plugins.Loader) *cobra.Command {
var o Options
cmd := &cobra.Command{
@@ -83,7 +83,7 @@ func NewCmdBuild(
if err != nil {
return err
}
return o.RunBuild(out, fs, rf, ptf, pc)
return o.RunBuild(out, fs, rf, ptf, pl)
},
}
cmd.Flags().StringVarP(
@@ -92,7 +92,7 @@ func NewCmdBuild(
"If specified, write the build output to this path.")
loader.AddLoadRestrictionsFlag(cmd.Flags())
cmd.AddCommand(NewCmdBuildPrune(out, fs, rf, ptf, pc))
cmd.AddCommand(NewCmdBuildPrune(out, fs, rf, ptf, pl))
return cmd
}
@@ -115,14 +115,14 @@ func (o *Options) Validate(args []string) (err error) {
func (o *Options) RunBuild(
out io.Writer, fSys fs.FileSystem,
rf *resmap.Factory, ptf transformer.Factory,
pc *types.PluginConfig) error {
pl *plugins.Loader) error {
ldr, err := loader.NewLoader(
o.loadRestrictor, o.kustomizationPath, fSys)
if err != nil {
return err
}
defer ldr.Cleanup()
kt, err := target.NewKustTarget(ldr, rf, ptf, pc)
kt, err := target.NewKustTarget(ldr, rf, ptf, pl)
if err != nil {
return err
}
@@ -136,14 +136,14 @@ func (o *Options) RunBuild(
func (o *Options) RunBuildPrune(
out io.Writer, fSys fs.FileSystem,
rf *resmap.Factory, ptf transformer.Factory,
pc *types.PluginConfig) error {
pl *plugins.Loader) error {
ldr, err := loader.NewLoader(
o.loadRestrictor, o.kustomizationPath, fSys)
if err != nil {
return err
}
defer ldr.Cleanup()
kt, err := target.NewKustTarget(ldr, rf, ptf, pc)
kt, err := target.NewKustTarget(ldr, rf, ptf, pl)
if err != nil {
return err
}
@@ -174,7 +174,7 @@ func NewCmdBuildPrune(
out io.Writer, fs fs.FileSystem,
rf *resmap.Factory,
ptf transformer.Factory,
pc *types.PluginConfig) *cobra.Command {
pl *plugins.Loader) *cobra.Command {
var o Options
cmd := &cobra.Command{
@@ -187,7 +187,7 @@ func NewCmdBuildPrune(
if err != nil {
return err
}
return o.RunBuildPrune(out, fs, rf, ptf, pc)
return o.RunBuildPrune(out, fs, rf, ptf, pl)
},
}
return cmd

View File

@@ -31,6 +31,7 @@ import (
"sigs.k8s.io/kustomize/pkg/commands/misc"
"sigs.k8s.io/kustomize/pkg/fs"
"sigs.k8s.io/kustomize/pkg/pgmconfig"
"sigs.k8s.io/kustomize/pkg/plugins"
"sigs.k8s.io/kustomize/pkg/resmap"
"sigs.k8s.io/kustomize/pkg/resource"
"sigs.k8s.io/kustomize/pkg/types"
@@ -64,12 +65,13 @@ See https://sigs.k8s.io/kustomize
PluginConfig: pluginConfig,
}
uf := kunstruct.NewKunstructuredFactoryWithGeneratorArgs(&genMetaArgs)
rf := resmap.NewFactory(resource.NewFactory(uf))
c.AddCommand(
build.NewCmdBuild(
stdOut, fSys,
resmap.NewFactory(resource.NewFactory(uf)),
transformer.NewFactoryImpl(), pluginConfig),
rf,
transformer.NewFactoryImpl(),
plugins.NewLoader(pluginConfig, rf)),
edit.NewCmdEdit(fSys, validator.NewKustValidator(), uf),
misc.NewCmdConfig(fSys),
misc.NewCmdVersion(stdOut),

View File

@@ -62,8 +62,7 @@ func DefaultSrcRoot() (string, error) {
}
nope = append(nope, root)
root = filepath.Join(
pgmconfig.ConfigRoot(), plugin.PluginRoot)
root = plugin.DefaultPluginConfig().DirectoryPath
if FileExists(root) {
return root, nil
}

View File

@@ -27,9 +27,8 @@ import (
"syscall"
"github.com/ghodss/yaml"
"sigs.k8s.io/kustomize/k8sdeps/kv/plugin"
"sigs.k8s.io/kustomize/pkg/ifc"
"sigs.k8s.io/kustomize/pkg/pgmconfig"
"sigs.k8s.io/kustomize/pkg/resid"
"sigs.k8s.io/kustomize/pkg/resmap"
)
@@ -59,11 +58,23 @@ type ExecPlugin struct {
ldr ifc.Loader
}
func NewExecPlugin(root string, id resid.ResId) *ExecPlugin {
return &ExecPlugin{
name: filepath.Join(root, pluginPath(id)),
}
}
// isAvailable checks to see if the plugin is available
func (p *ExecPlugin) isAvailable() bool {
f, err := os.Stat(p.name)
if os.IsNotExist(err) {
return false
}
return f.Mode()&0111 != 0000
}
func (p *ExecPlugin) Config(
ldr ifc.Loader, rf *resmap.Factory, k ifc.Kunstructured) error {
dir := filepath.Join(pgmconfig.ConfigRoot(), plugin.PluginRoot)
id := k.GetGvk()
p.name = filepath.Join(dir, id.Group, id.Version, id.Kind)
p.rf = rf
p.ldr = ldr

View File

@@ -18,6 +18,7 @@ import (
"testing"
"sigs.k8s.io/kustomize/k8sdeps/kunstruct"
"sigs.k8s.io/kustomize/k8sdeps/kv/plugin"
"sigs.k8s.io/kustomize/pkg/internal/loadertest"
"sigs.k8s.io/kustomize/pkg/resmap"
"sigs.k8s.io/kustomize/pkg/resource"
@@ -25,11 +26,11 @@ import (
func TestExecPluginConfig(t *testing.T) {
path := "/app"
kFactory := kunstruct.NewKunstructuredFactoryImpl()
rf := resmap.NewFactory(resource.NewFactory(kFactory))
rf := resmap.NewFactory(
resource.NewFactory(
kunstruct.NewKunstructuredFactoryImpl()))
ldr := loadertest.NewFakeLoader(path)
pluginConfig := kFactory.FromMap(
pluginConfig := rf.RF().FromMap(
map[string]interface{}{
"apiVersion": "someteam.example.com/v1",
"kind": "SedTransformer",
@@ -46,7 +47,9 @@ s/$BAR/bar/g
\ \ \
`))
p := &ExecPlugin{}
p := NewExecPlugin(
plugin.DefaultPluginConfig().DirectoryPath,
pluginConfig.Id())
p.Config(ldr, rf, pluginConfig)

View File

@@ -22,38 +22,19 @@ import (
"sigs.k8s.io/kustomize/pkg/ifc"
"sigs.k8s.io/kustomize/pkg/resmap"
"sigs.k8s.io/kustomize/pkg/transformers"
"sigs.k8s.io/kustomize/pkg/types"
)
type generatorLoader struct {
pc *types.PluginConfig
ldr ifc.Loader
rf *resmap.Factory
}
func NewGeneratorLoader(
pc *types.PluginConfig,
ldr ifc.Loader, rf *resmap.Factory) generatorLoader {
return generatorLoader{pc: pc, ldr: ldr, rf: rf}
}
func (l generatorLoader) Load(
rm resmap.ResMap) ([]transformers.Generator, error) {
if len(rm) == 0 {
return nil, nil
}
if !l.pc.GoEnabled {
return nil, fmt.Errorf("plugins not enabled")
}
func (l *Loader) LoadGenerators(
ldr ifc.Loader, rm resmap.ResMap) ([]transformers.Generator, error) {
var result []transformers.Generator
for id, res := range rm {
c, err := loadAndConfigurePlugin(l.pc.DirectoryPath, id, l.ldr, l.rf, res)
for _, res := range rm {
c, err := l.loadAndConfigurePlugin(ldr, res)
if err != nil {
return nil, err
}
g, ok := c.(transformers.Generator)
if !ok {
return nil, fmt.Errorf("plugin %s not a generator", id.String())
return nil, fmt.Errorf("plugin %s not a generator", res.Id())
}
result = append(result, g)
}

View File

@@ -18,10 +18,10 @@ package plugins
import (
"fmt"
"github.com/pkg/errors"
"os"
"path/filepath"
"plugin"
"github.com/pkg/errors"
kplugin "sigs.k8s.io/kustomize/k8sdeps/kv/plugin"
"sigs.k8s.io/kustomize/pkg/ifc"
"sigs.k8s.io/kustomize/pkg/resid"
@@ -35,91 +35,86 @@ type Configurable interface {
Config(ldr ifc.Loader, rf *resmap.Factory, k ifc.Kunstructured) error
}
type transformerLoader struct {
pc *types.PluginConfig
ldr ifc.Loader
rf *resmap.Factory
type Loader struct {
pc *types.PluginConfig
rf *resmap.Factory
}
func NewTransformerLoader(
pc *types.PluginConfig,
ldr ifc.Loader, rf *resmap.Factory) transformerLoader {
return transformerLoader{pc: pc, ldr: ldr, rf: rf}
func NewLoader(
pc *types.PluginConfig, rf *resmap.Factory) *Loader {
return &Loader{pc: pc, rf: rf}
}
func (l transformerLoader) Load(
rm resmap.ResMap) ([]transformers.Transformer, error) {
if len(rm) == 0 {
return nil, nil
}
if !l.pc.GoEnabled {
return nil, fmt.Errorf("plugins not enabled")
}
func (l *Loader) LoadTransformers(
ldr ifc.Loader, rm resmap.ResMap) ([]transformers.Transformer, error) {
var result []transformers.Transformer
for id, res := range rm {
c, err := loadAndConfigurePlugin(l.pc.DirectoryPath, id, l.ldr, l.rf, res)
for _, res := range rm {
c, err := l.loadAndConfigurePlugin(ldr, res)
if err != nil {
return nil, err
}
t, ok := c.(transformers.Transformer)
if !ok {
return nil, fmt.Errorf("plugin %s not a transformer", id.String())
return nil, fmt.Errorf("plugin %s not a transformer", res.Id())
}
result = append(result, t)
}
return result, nil
}
func goPluginFileName(dir string, id resid.ResId) string {
return execPluginFileName(dir, id) + ".so"
func pluginPath(id resid.ResId) string {
return filepath.Join(id.Gvk().Group, id.Gvk().Version, id.Gvk().Kind)
}
func execPluginFileName(dir string, id resid.ResId) string {
return filepath.Join(
dir,
id.Gvk().Group, id.Gvk().Version, id.Gvk().Kind)
}
// isExecAvailable checks if an executable is available
func isExecAvailable(name string) bool {
f, err := os.Stat(name)
if os.IsNotExist(err) {
return false
func (l *Loader) loadAndConfigurePlugin(
ldr ifc.Loader, res *resource.Resource) (c Configurable, err error) {
if !l.pc.GoEnabled {
return nil, errors.Errorf(
"plugins not enabled, but trying to load %s", res.Id())
}
return f.Mode()&0111 != 0000
}
func loadAndConfigurePlugin(
dir string, id resid.ResId,
ldr ifc.Loader,
rf *resmap.Factory, res *resource.Resource) (Configurable, error) {
var fileName string
var c Configurable
exec := execPluginFileName(dir, id)
if isExecAvailable(exec) {
c = &ExecPlugin{}
if p := NewExecPlugin(l.pc.DirectoryPath, res.Id()); p.isAvailable() {
c = p
} else {
fileName = goPluginFileName(dir, id)
goPlugin, err := plugin.Open(fileName)
c, err = l.loadGoPlugin(res.Id())
if err != nil {
return nil, errors.Wrapf(err, "plugin %s fails to load", fileName)
}
symbol, err := goPlugin.Lookup(kplugin.PluginSymbol)
if err != nil {
return nil, errors.Wrapf(
err, "plugin %s doesn't have symbol %s",
fileName, kplugin.PluginSymbol)
}
var ok bool
c, ok = symbol.(Configurable)
if !ok {
return nil, fmt.Errorf("plugin %s not configurable", fileName)
return nil, err
}
}
err := c.Config(ldr, rf, res)
err = c.Config(ldr, l.rf, res)
if err != nil {
return nil, errors.Wrapf(err, "plugin %s fails configuration", fileName)
return nil, errors.Wrapf(
err, "plugin %s fails configuration", res.Id())
}
return c, nil
}
// Each test makes its own loader, and tries to load its own plugins,
// but the loaded .so files are in shared memory, so one will get
// "this plugin already loaded" errors if the registry is maintained
// as a Loader instance variable. So make it a package variable.
var registry = make(map[string]Configurable)
func (l *Loader) loadGoPlugin(id resid.ResId) (c Configurable, err error) {
var ok bool
path := pluginPath(id)
if c, ok = registry[path]; ok {
return c, nil
}
name := filepath.Join(l.pc.DirectoryPath, path)
p, err := plugin.Open(name + ".so")
if err != nil {
return nil, errors.Wrapf(err, "plugin %s fails to load", name)
}
symbol, err := p.Lookup(kplugin.PluginSymbol)
if err != nil {
return nil, errors.Wrapf(
err, "plugin %s doesn't have symbol %s",
name, kplugin.PluginSymbol)
}
c, ok = symbol.(Configurable)
if !ok {
return nil, fmt.Errorf("plugin %s not configurable", name)
}
registry[path] = c
return
}

View File

@@ -44,7 +44,7 @@ type KustTarget struct {
ldr ifc.Loader
rFactory *resmap.Factory
tFactory transformer.Factory
pluginConfig *types.PluginConfig
pLdr *plugins.Loader
}
// NewKustTarget returns a new instance of KustTarget primed with a Loader.
@@ -52,7 +52,7 @@ func NewKustTarget(
ldr ifc.Loader,
rFactory *resmap.Factory,
tFactory transformer.Factory,
pc *types.PluginConfig) (*KustTarget, error) {
pLdr *plugins.Loader) (*KustTarget, error) {
content, err := loadKustFile(ldr)
if err != nil {
return nil, err
@@ -74,7 +74,7 @@ func NewKustTarget(
ldr: ldr,
rFactory: rFactory,
tFactory: tFactory,
pluginConfig: pc,
pLdr: pLdr,
}, nil
}
@@ -240,11 +240,9 @@ func (kt *KustTarget) AccumulateTarget() (
return nil, errors.Wrap(
err, "merging legacy configMaps and secrets")
}
if kt.pluginConfig.GoEnabled {
err := kt.generateFromPlugins(ra)
if err != nil {
return nil, err
}
err = kt.generateFromPlugins(ra)
if err != nil {
return nil, err
}
patches, err := kt.rFactory.RF().SliceFromPatches(
kt.ldr, kt.kustomization.PatchesStrategicMerge)
@@ -328,7 +326,7 @@ func (kt *KustTarget) accumulateDirectory(
ra *accumulator.ResAccumulator, ldr ifc.Loader, path string) error {
defer ldr.Cleanup()
subKt, err := NewKustTarget(
ldr, kt.rFactory, kt.tFactory, kt.pluginConfig)
ldr, kt.rFactory, kt.tFactory, kt.pLdr)
if err != nil {
return errors.Wrapf(err, "couldn't make target for path '%s'", path)
}
@@ -403,14 +401,11 @@ func (kt *KustTarget) newTransformer(
return nil, err
}
r = append(r, t)
if kt.pluginConfig.GoEnabled {
tp, err := kt.loadTransformerPlugins()
if err != nil {
return nil, err
}
r = append(r, tp...)
tp, err := kt.loadTransformerPlugins()
if err != nil {
return nil, err
}
r = append(r, tp...)
return transformers.NewMultiTransformer(r), nil
}
@@ -420,8 +415,7 @@ func (kt *KustTarget) loadTransformerPlugins() ([]transformers.Transformer, erro
if err != nil {
return nil, err
}
return plugins.NewTransformerLoader(
kt.pluginConfig, kt.ldr, kt.rFactory).Load(ra.ResMap())
return kt.pLdr.LoadTransformers(kt.ldr, ra.ResMap())
}
func (kt *KustTarget) loadGeneratorPlugins() ([]transformers.Generator, error) {
@@ -430,6 +424,5 @@ func (kt *KustTarget) loadGeneratorPlugins() ([]transformers.Generator, error) {
if err != nil {
return nil, err
}
return plugins.NewGeneratorLoader(
kt.pluginConfig, kt.ldr, kt.rFactory).Load(ra.ResMap())
return kt.pLdr.LoadGenerators(kt.ldr, ra.ResMap())
}

View File

@@ -23,7 +23,6 @@ import (
"strings"
"testing"
"sigs.k8s.io/kustomize/k8sdeps/kv/plugin"
"sigs.k8s.io/kustomize/pkg/gvk"
"sigs.k8s.io/kustomize/pkg/ifc"
"sigs.k8s.io/kustomize/pkg/internal/loadertest"
@@ -206,8 +205,7 @@ func TestResources(t *testing.T) {
func TestKustomizationNotFound(t *testing.T) {
_, err := NewKustTarget(
loadertest.NewFakeLoader("/foo"),
nil, nil, plugin.DefaultPluginConfig())
loadertest.NewFakeLoader("/foo"), nil, nil, nil)
if err == nil {
t.Fatalf("expected an error")
}

View File

@@ -30,6 +30,7 @@ import (
"sigs.k8s.io/kustomize/pkg/internal/loadertest"
"sigs.k8s.io/kustomize/pkg/loader"
"sigs.k8s.io/kustomize/pkg/pgmconfig"
"sigs.k8s.io/kustomize/pkg/plugins"
"sigs.k8s.io/kustomize/pkg/resmap"
"sigs.k8s.io/kustomize/pkg/resource"
. "sigs.k8s.io/kustomize/pkg/target"
@@ -41,7 +42,7 @@ type KustTestHarness struct {
t *testing.T
rf *resmap.Factory
ldr loadertest.FakeLoader
pc *types.PluginConfig
pl *plugins.Loader
}
func NewKustTestHarness(t *testing.T, path string) *KustTestHarness {
@@ -58,18 +59,19 @@ func NewKustTestHarnessWithPluginConfig(
func NewKustTestHarnessFull(
t *testing.T, path string,
lr loader.LoadRestrictorFunc, pc *types.PluginConfig) *KustTestHarness {
rf := resmap.NewFactory(resource.NewFactory(
kunstruct.NewKunstructuredFactoryWithGeneratorArgs(
&types.GeneratorMetaArgs{PluginConfig: pc})))
return &KustTestHarness{
t: t,
rf: resmap.NewFactory(resource.NewFactory(
kunstruct.NewKunstructuredFactoryWithGeneratorArgs(
&types.GeneratorMetaArgs{PluginConfig: pc}))),
t: t,
rf: rf,
ldr: loadertest.NewFakeLoaderWithRestrictor(lr, path),
pc: pc}
pl: plugins.NewLoader(pc, rf)}
}
func (th *KustTestHarness) makeKustTarget() *KustTarget {
kt, err := NewKustTarget(
th.ldr, th.rf, transformer.NewFactoryImpl(), th.pc)
th.ldr, th.rf, transformer.NewFactoryImpl(), th.pl)
if err != nil {
th.t.Fatalf("Unexpected construction error %v", err)
}

View File

@@ -42,8 +42,7 @@ func writeStringPrefixer(th *KustTestHarness, path string) {
apiVersion: someteam.example.com/v1
kind: StringPrefixer
metadata:
name: myStringPrefixer
prefix: apple-
name: apple
`)
}
@@ -52,7 +51,7 @@ func writeDatePrefixer(th *KustTestHarness, path string) {
apiVersion: someteam.example.com/v1
kind: DatePrefixer
metadata:
name: myDatePrefixer
name: irrelevant
`)
}
@@ -73,7 +72,11 @@ resources:
- deployment.yaml
transformers:
- stringPrefixer.yaml
# - datePrefixer.yaml
`)
// TODO(monopole): assure ordering of loaded
// transformers and this will work - the trouble
// is we load into a map (ResMap), not a list.
writeDeployment(th, "/app/deployment.yaml")
writeStringPrefixer(th, "/app/stringPrefixer.yaml")
writeDatePrefixer(th, "/app/datePrefixer.yaml")
@@ -144,7 +147,16 @@ metadata:
`)
}
func xTestTransformedTransformers(t *testing.T) {
func TestTransformedTransformers(t *testing.T) {
tc := NewTestEnvController(t).Set()
defer tc.Reset()
tc.BuildGoPlugin(
"someteam.example.com", "v1", "StringPrefixer")
tc.BuildGoPlugin(
"someteam.example.com", "v1", "DatePrefixer")
th := NewKustTestHarnessWithPluginConfig(
t, "/app/overlay", plugin.ActivePluginConfig())
@@ -170,6 +182,18 @@ transformers:
t.Fatalf("Err: %v", err)
}
th.assertActualEqualsExpected(m, `
HEY
apiVersion: apps/v1
kind: Deployment
metadata:
name: 2018-05-11-apple-myDeployment
spec:
template:
metadata:
labels:
backend: awesome
spec:
containers:
- image: whatever
name: whatever
`)
}

View File

@@ -3,8 +3,6 @@
package main
import (
"time"
"sigs.k8s.io/kustomize/pkg/ifc"
"sigs.k8s.io/kustomize/pkg/resmap"
"sigs.k8s.io/kustomize/pkg/transformers"
@@ -20,9 +18,17 @@ func (p *plugin) Config(
return nil
}
// Returns a constant, rather than
// time.Now().Format("2006-01-02")
// to make tests happy.
// This is just an example.
func getDate() string {
return "2018-05-11"
}
func (p *plugin) Transform(m resmap.ResMap) error {
tr, err := transformers.NewNamePrefixSuffixTransformer(
time.Now().Format("2006-01-02")+"-", "",
getDate()+"-", "",
config.MakeDefaultConfig().NamePrefix)
if err != nil {
return err

View File

@@ -12,6 +12,7 @@
package main
import (
"fmt"
"sigs.k8s.io/kustomize/pkg/ifc"
"sigs.k8s.io/kustomize/pkg/resmap"
"sigs.k8s.io/kustomize/pkg/transformers"
@@ -26,11 +27,11 @@ var KustomizePlugin plugin
func (p *plugin) Config(
ldr ifc.Loader, rf *resmap.Factory, k ifc.Kunstructured) error {
var err error
p.prefix, err = k.GetFieldValue("prefix")
if err != nil {
return err
name := k.GetName()
if name == "" {
return fmt.Errorf("name cannot be empty")
}
p.prefix = name + "-"
return nil
}