diff --git a/Makefile b/Makefile index c6d4ced6e..677a7c460 100644 --- a/Makefile +++ b/Makefile @@ -188,8 +188,14 @@ lint-kustomize: install-tools $(builtinplugins) cd pluginator; \ $(MYGOBIN)/golangci-lint-kustomize -c ../.golangci-kustomize.yml run ./... +# Used to add non-default compilation flags when experimenting with +# plugin-to-api compatibility checks. +.PHONY: build-kustomize-api +build-kustomize-api: $(builtinplugins) + cd api; go build ./... + .PHONY: test-unit-kustomize-api -test-unit-kustomize-api: $(builtinplugins) +test-unit-kustomize-api: build-kustomize-api cd api; go test ./... .PHONY: test-unit-kustomize-plugins @@ -255,18 +261,31 @@ $(MYGOBIN)/kubeval: ) # linux only. -# This is for testing an example plugin that -# uses helm to inflate a chart for subsequent kustomization. -# Don't want to add a hard dependence in go.mod file -# to helm. -# Instead, download the binary. -$(MYGOBIN)/helm: +# This is for testing an example plugin that uses helm to inflate a chart +# for subsequent kustomization. +# Don't want to add a hard dependence in go.mod file to helm. +# Instead, download the binaries. +$(MYGOBIN)/helmV2: ( \ set -e; \ d=$(shell mktemp -d); cd $$d; \ - wget https://storage.googleapis.com/kubernetes-helm/helm-v2.13.1-linux-amd64.tar.gz; \ - tar -xvzf helm-v2.13.1-linux-amd64.tar.gz; \ - mv linux-amd64/helm $(MYGOBIN); \ + tgzFile=helm-v2.13.1-linux-amd64.tar.gz; \ + wget https://storage.googleapis.com/kubernetes-helm/$$tgzFile; \ + tar -xvzf $$tgzFile; \ + mv linux-amd64/helm $(MYGOBIN)/helmV2; \ + rm -rf $$d \ + ) + +# Helm V3 differs from helm V2; downloading it to provide coverage for the +# chart inflator plugin under helm v3. +$(MYGOBIN)/helmV3: + ( \ + set -e; \ + d=$(shell mktemp -d); cd $$d; \ + tgzFile=helm-v3.2.0-rc.1-linux-amd64.tar.gz; \ + wget https://get.helm.sh/$$tgzFile; \ + tar -xvzf $$tgzFile; \ + mv linux-amd64/helm $(MYGOBIN)/helmV3; \ rm -rf $$d \ ) @@ -288,6 +307,7 @@ clean: kustomize-external-go-plugin-clean rm -f $(MYGOBIN)/kustomize rm -f $(MYGOBIN)/golangci-lint-kustomize +# Nuke the site from orbit. It's the only way to be sure. .PHONY: nuke nuke: clean - sudo rm -rf $(shell go env GOPATH)/pkg/mod/sigs.k8s.io + go clean --modcache diff --git a/api/internal/plugins/compiler/compiler.go b/api/internal/plugins/compiler/compiler.go index 809fada48..2ca04c437 100644 --- a/api/internal/plugins/compiler/compiler.go +++ b/api/internal/plugins/compiler/compiler.go @@ -6,143 +6,85 @@ package compiler import ( "bytes" "fmt" + "log" "os" "os/exec" "path/filepath" - "runtime" "strings" "time" "github.com/pkg/errors" - "sigs.k8s.io/kustomize/api/filesys" - "sigs.k8s.io/kustomize/api/konfig" ) // Compiler creates Go plugin object files. -// -// Source code is read from -// ${srcRoot}/${g}/${v}/${k}.go -// -// Object code is written to -// ${objRoot}/${g}/${v}/${k}.so type Compiler struct { - srcRoot string - objRoot string -} - -// DeterminePluginSrcRoot guesses where the user -// has her ${g}/${v}/$lower(${k})/${k}.go files. -func DeterminePluginSrcRoot(fSys filesys.FileSystem) (string, error) { - return konfig.FirstDirThatExistsElseError( - "source directory", fSys, []konfig.NotedFunc{ - { - Note: "relative to unit test", - F: func() string { - return filepath.Clean( - filepath.Join( - os.Getenv("PWD"), - "..", "..", - konfig.RelPluginHome)) - }, - }, - { - Note: "relative to unit test (internal pkg)", - F: func() string { - return filepath.Clean( - filepath.Join( - os.Getenv("PWD"), - "..", "..", "..", "..", - konfig.RelPluginHome)) - }, - }, - { - Note: "relative to api package", - F: func() string { - return filepath.Clean( - filepath.Join( - os.Getenv("PWD"), - "..", "..", "..", - konfig.RelPluginHome)) - }, - }, - { - Note: "old style $GOPATH", - F: func() string { - return filepath.Join( - os.Getenv("GOPATH"), - "src", konfig.DomainName, - konfig.ProgramName, konfig.RelPluginHome) - }, - }, - { - Note: "HOME with literal 'gopath'", - F: func() string { - return filepath.Join( - konfig.HomeDir(), "gopath", - "src", konfig.DomainName, - konfig.ProgramName, konfig.RelPluginHome) - }, - }, - { - Note: "home directory", - F: func() string { - return filepath.Join( - konfig.HomeDir(), konfig.DomainName, - konfig.ProgramName, konfig.RelPluginHome) - }, - }, - }) + // pluginRoot is where the user + // has her ${g}/${v}/$lower(${k})/${k}.go files. + pluginRoot string + // Where compilation happens. + workDir string + // Used as the root file name for src and object. + rawKind string + // Capture compiler output. + stderr bytes.Buffer + // Capture compiler output. + stdout bytes.Buffer } // NewCompiler returns a new compiler instance. -func NewCompiler(srcRoot, objRoot string) *Compiler { - return &Compiler{srcRoot: srcRoot, objRoot: objRoot} +func NewCompiler(root string) *Compiler { + return &Compiler{pluginRoot: root} } -// ObjRoot is root of compilation target tree. -func (b *Compiler) ObjRoot() string { - return b.objRoot +// Set GVK converts g,v,k tuples to file path components. +func (b *Compiler) SetGVK(g, v, k string) { + b.rawKind = k + b.workDir = filepath.Join(b.pluginRoot, g, v, strings.ToLower(k)) } -// SrcRoot is where to find src. -func (b *Compiler) SrcRoot() string { - return b.srcRoot +func (b *Compiler) srcPath() string { + return filepath.Join(b.workDir, b.rawKind+".go") } -func goBin() string { - return filepath.Join(runtime.GOROOT(), "bin", "go") +func (b *Compiler) objFile() string { + return b.rawKind + ".so" } -// Compile reads ${srcRoot}/${g}/${v}/${k}.go -// and writes ${objRoot}/${g}/${v}/${k}.so -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) { - // Skip rebuilding it. +// Absolute path to the compiler output (the .so file). +func (b *Compiler) ObjPath() string { + return filepath.Join(b.workDir, b.objFile()) +} + +// Cleanup provides a hook to delete the .so file. +// Ignore errors. +func (b *Compiler) Cleanup() { + _ = os.Remove(b.ObjPath()) +} + +// Compile changes its working directory to +// ${pluginRoot}/${g}/${v}/$lower(${k} and places +// object code next to source code. +func (b *Compiler) Compile() error { + if FileYoungerThan(b.ObjPath(), 8*time.Second) { + // Skip rebuilding it, to save time in a plugin test file + // that has many distinct calls to make a harness and compile + // the plugin (only the first compile will happen). + // Make it a short time to avoid tricking someone who's actively + // developing a plugin. return nil } - err := os.MkdirAll(objDir, os.ModePerm) - if err != nil { - return err - } - srcFile := filepath.Join(b.srcRoot, g, v, lowK, k) + ".go" - if !FileExists(srcFile) { - // Handy for tests of lone plugins. - s := k + ".go" - if !FileExists(s) { - return fmt.Errorf( - "cannot find source at '%s' or '%s'", srcFile, s) - - } - srcFile = s + if !FileExists(b.srcPath()) { + return fmt.Errorf("cannot find source at '%s'", b.srcPath()) } + // If you use an IDE, make sure it's go build and test flags + // match those used below. Same goes for Makefile targets. commands := []string{ "build", + // "-trimpath", This flag used to make it better, now it makes it worse, + // see https://github.com/golang/go/issues/31354 "-buildmode", "plugin", - "-o", objFile, srcFile, + "-o", b.objFile(), } goBin := goBin() if !FileExists(goBin) { @@ -150,34 +92,26 @@ func (b *Compiler) Compile(g, v, k string) error { "cannot find go compiler %s", goBin) } cmd := exec.Command(goBin, commands...) - var stderr bytes.Buffer - cmd.Stderr = &stderr + b.stderr.Reset() + cmd.Stderr = &b.stderr + b.stdout.Reset() + cmd.Stdout = &b.stdout cmd.Env = os.Environ() + cmd.Dir = b.workDir if err := cmd.Run(); err != nil { + b.report() return errors.Wrapf( - err, "cannot compile %s:\nSTDERR\n%s\n", srcFile, stderr.String()) + err, "cannot compile %s:\nSTDERR\n%s\n", + b.srcPath(), b.stderr.String()) } 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 { - fi, err := os.Stat(path) - if err != nil { - if os.IsNotExist(err) { - return false - } - } - age := time.Since(fi.ModTime()) - return age.Minutes() < 3 -} - -func FileExists(name string) bool { - if _, err := os.Stat(name); err != nil { - if os.IsNotExist(err) { - return false - } - } - return true +func (b *Compiler) report() { + log.Println("stdout: -------") + log.Println(b.stdout.String()) + log.Println("----------------") + log.Println("stderr: -------") + log.Println(b.stderr.String()) + log.Println("----------------") } diff --git a/api/internal/plugins/compiler/compiler_test.go b/api/internal/plugins/compiler/compiler_test.go index 28eee0ede..89640f7b1 100644 --- a/api/internal/plugins/compiler/compiler_test.go +++ b/api/internal/plugins/compiler/compiler_test.go @@ -4,10 +4,9 @@ package compiler_test import ( - "io/ioutil" - "os" "path/filepath" "testing" + "time" "sigs.k8s.io/kustomize/api/filesys" . "sigs.k8s.io/kustomize/api/internal/plugins/compiler" @@ -15,53 +14,46 @@ import ( // Regression coverage over compiler behavior. func TestCompiler(t *testing.T) { - configRoot, err := ioutil.TempDir("", "kustomize-compiler-test") - if err != nil { - t.Errorf("failed to make temp dir: %v", err) - } srcRoot, err := DeterminePluginSrcRoot(filesys.MakeFsOnDisk()) if err != nil { t.Error(err) } - c := NewCompiler(srcRoot, configRoot) - if configRoot != c.ObjRoot() { - t.Errorf("unexpected objRoot %s", c.ObjRoot()) - } + c := NewCompiler(srcRoot) + c.SetGVK("someteam.example.com", "v1", "DatePrefixer") expectObj := filepath.Join( - c.ObjRoot(), - "someteam.example.com", "v1", "dateprefixer", "DatePrefixer.so") - if FileExists(expectObj) { - t.Errorf("obj file should not exist yet: %s", expectObj) + srcRoot, "someteam.example.com", "v1", "dateprefixer", "DatePrefixer.so") + if expectObj != c.ObjPath() { + t.Errorf("Expected '%s', got '%s'", expectObj, c.ObjPath()) } - err = c.Compile("someteam.example.com", "v1", "DatePrefixer") + err = c.Compile() if err != nil { t.Error(err) } - if !RecentFileExists(expectObj) { + if !FileYoungerThan(expectObj, time.Second) { t.Errorf("didn't find expected obj file %s", expectObj) } + c.Cleanup() + if FileExists(expectObj) { + t.Errorf("obj file '%s' should be gone", expectObj) + } + c.SetGVK("builtin", "", "SecretGenerator") expectObj = filepath.Join( - c.ObjRoot(), + srcRoot, "builtin", "", "secretgenerator", "SecretGenerator.so") - if FileExists(expectObj) { - t.Errorf("obj file should not exist yet: %s", expectObj) + if expectObj != c.ObjPath() { + t.Errorf("Expected '%s', got '%s'", expectObj, c.ObjPath()) } - err = c.Compile("builtin", "", "SecretGenerator") + err = c.Compile() if err != nil { t.Error(err) } - if !RecentFileExists(expectObj) { + if !FileYoungerThan(expectObj, time.Second) { t.Errorf("didn't find expected obj file %s", expectObj) } - - err = os.RemoveAll(c.ObjRoot()) - if err != nil { - t.Errorf( - "removing temp dir: %s %v", c.ObjRoot(), err) - } + c.Cleanup() if FileExists(expectObj) { - t.Errorf("cleanup failed; still see: %s", expectObj) + t.Errorf("obj file '%s' should be gone", expectObj) } } diff --git a/api/internal/plugins/compiler/utils.go b/api/internal/plugins/compiler/utils.go new file mode 100644 index 000000000..d117184ff --- /dev/null +++ b/api/internal/plugins/compiler/utils.go @@ -0,0 +1,102 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package compiler + +import ( + "os" + "path/filepath" + "runtime" + "time" + + "sigs.k8s.io/kustomize/api/filesys" + "sigs.k8s.io/kustomize/api/konfig" +) + +func goBin() string { + return filepath.Join(runtime.GOROOT(), "bin", "go") +} + +// DeterminePluginSrcRoot guesses where the user +// has her ${g}/${v}/$lower(${k})/${k}.go files. +func DeterminePluginSrcRoot(fSys filesys.FileSystem) (string, error) { + return konfig.FirstDirThatExistsElseError( + "source directory", fSys, []konfig.NotedFunc{ + { + Note: "relative to unit test", + F: func() string { + return filepath.Clean( + filepath.Join( + os.Getenv("PWD"), + "..", "..", + konfig.RelPluginHome)) + }, + }, + { + Note: "relative to unit test (internal pkg)", + F: func() string { + return filepath.Clean( + filepath.Join( + os.Getenv("PWD"), + "..", "..", "..", "..", + konfig.RelPluginHome)) + }, + }, + { + Note: "relative to api package", + F: func() string { + return filepath.Clean( + filepath.Join( + os.Getenv("PWD"), + "..", "..", "..", + konfig.RelPluginHome)) + }, + }, + { + Note: "old style $GOPATH", + F: func() string { + return filepath.Join( + os.Getenv("GOPATH"), + "src", konfig.DomainName, + konfig.ProgramName, konfig.RelPluginHome) + }, + }, + { + Note: "HOME with literal 'gopath'", + F: func() string { + return filepath.Join( + konfig.HomeDir(), "gopath", + "src", konfig.DomainName, + konfig.ProgramName, konfig.RelPluginHome) + }, + }, + { + Note: "home directory", + F: func() string { + return filepath.Join( + konfig.HomeDir(), konfig.DomainName, + konfig.ProgramName, konfig.RelPluginHome) + }, + }, + }) +} + +// 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 + } + } + return time.Since(fi.ModTime()) <= d +} + +func FileExists(name string) bool { + if _, err := os.Stat(name); err != nil { + if os.IsNotExist(err) { + return false + } + } + return true +} diff --git a/api/internal/plugins/compiler/utils_test.go b/api/internal/plugins/compiler/utils_test.go new file mode 100644 index 000000000..0cb14eaa2 --- /dev/null +++ b/api/internal/plugins/compiler/utils_test.go @@ -0,0 +1,26 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package compiler + +import ( + "path/filepath" + "strings" + "testing" + + "sigs.k8s.io/kustomize/api/filesys" +) + +func TestDeterminePluginSrcRoot(t *testing.T) { + actual, err := DeterminePluginSrcRoot(filesys.MakeFsOnDisk()) + if err != nil { + t.Error(err) + } + if !filepath.IsAbs(actual) { + t.Errorf("expected absolute path, but got '%s'", actual) + } + expectedSuffix := filepath.Join("sigs.k8s.io", "kustomize", "plugin") + if !strings.HasSuffix(actual, expectedSuffix) { + t.Errorf("expected suffix '%s' in '%s'", expectedSuffix, actual) + } +} 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..b35969670 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -70,7 +70,7 @@ func (th Harness) MakeOptionsPluginsDisabled() krusty.Options { // Enables use of non-builtin plugins. func (th Harness) MakeOptionsPluginsEnabled() krusty.Options { - c, err := konfig.EnabledPluginConfig() + c, err := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) 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..6ed84af14 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -19,38 +19,50 @@ 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() + pc, err := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) 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 +72,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 +151,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..3bbb8d702 100644 --- a/api/testutils/kusttest/plugintestenv.go +++ b/api/testutils/kusttest/plugintestenv.go @@ -4,11 +4,7 @@ package kusttest_test import ( - "io/ioutil" "os" - "os/exec" - "path/filepath" - "strings" "testing" "sigs.k8s.io/kustomize/api/filesys" @@ -16,17 +12,14 @@ import ( "sigs.k8s.io/kustomize/api/konfig" ) -// pluginTestEnv manages plugins for tests. -// It manages a Go plugin compiler, -// makes and removes a temporary working directory, -// and sets/resets shell env vars as needed. +// pluginTestEnv manages compiling plugins for tests. +// It manages a Go plugin compiler, and sets/resets shell env vars as needed. type pluginTestEnv struct { - t *testing.T - compiler *compiler.Compiler - srcRoot string - workDir string - oldXdg string - wasSet bool + t *testing.T + compiler *compiler.Compiler + pluginRoot string + oldXdg string + wasSet bool } // newPluginTestEnv returns a new instance of pluginTestEnv. @@ -39,76 +32,44 @@ func newPluginTestEnv(t *testing.T) *pluginTestEnv { // plugin code - this FileSystem has nothing to do with // the FileSystem used for loading config yaml in the tests. func (x *pluginTestEnv) set() *pluginTestEnv { - x.createWorkDir() var err error - x.srcRoot, err = compiler.DeterminePluginSrcRoot(filesys.MakeFsOnDisk()) + x.pluginRoot, err = compiler.DeterminePluginSrcRoot(filesys.MakeFsOnDisk()) if err != nil { x.t.Error(err) } - x.compiler = compiler.NewCompiler(x.srcRoot, x.workDir) + x.compiler = compiler.NewCompiler(x.pluginRoot) x.setEnv() return x } // reset restores the environment to pre-test state. func (x *pluginTestEnv) reset() { + // Calling Cleanup forces recompilation in a test file with multiple + // calls to MakeEnhancedHarness - so leaving it out. Your .gitignore + // should ignore .so files anyway. + // x.compiler.Cleanup() x.resetEnv() - x.removeWorkDir() } -// buildGoPlugin 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) { - err := x.compiler.Compile(g, v, k) +// prepareGoPlugin compiles a Go plugin, leaving the newly +// created object code alongside the src code. +func (x *pluginTestEnv) prepareGoPlugin(g, v, k string) { + x.compiler.SetGVK(g, v, k) + err := x.compiler.Compile() if err != nil { x.t.Errorf("compile failed: %v", err) } } -// prepExecPlugin 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 -// (Go or Exec) in the same spot, and since the test -// 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) { - lowK := strings.ToLower(k) - src := filepath.Join(x.srcRoot, g, v, lowK, k) - tmp := filepath.Join(x.workDir, g, v, lowK, k) - if err := os.MkdirAll(filepath.Dir(tmp), 0755); err != nil { - x.t.Errorf("error making directory: %s", filepath.Dir(tmp)) - } - cmd := exec.Command("cp", src, tmp) - cmd.Env = os.Environ() - if err := cmd.Run(); err != nil { - x.t.Errorf("error copying %s to %s: %v", src, tmp, err) - } -} - -func (x *pluginTestEnv) createWorkDir() { - var err error - x.workDir, err = ioutil.TempDir("", "kustomize-plugin-tests") - if err != nil { - x.t.Errorf("failed to make work dir: %v", err) - } -} - -func (x *pluginTestEnv) removeWorkDir() { - err := os.RemoveAll(x.workDir) - if err != nil { - x.t.Errorf( - "removing work dir: %s %v", x.workDir, err) - } +func (x *pluginTestEnv) prepareExecPlugin(_, _, _ string) { + // Do nothing. At one point this method + // copied the exec plugin directory to a temp dir + // and ran it from there. Left as a hook. } func (x *pluginTestEnv) setEnv() { x.oldXdg, x.wasSet = os.LookupEnv(konfig.KustomizePluginHomeEnv) - os.Setenv(konfig.KustomizePluginHomeEnv, x.workDir) + os.Setenv(konfig.KustomizePluginHomeEnv, x.pluginRoot) } func (x *pluginTestEnv) resetEnv() { 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/cmd/config/internal/commands/run-fns.go b/cmd/config/internal/commands/run-fns.go index 260808561..38ba38190 100644 --- a/cmd/config/internal/commands/run-fns.go +++ b/cmd/config/internal/commands/run-fns.go @@ -217,8 +217,8 @@ func toStorageMounts(mounts []string) []filters.StorageMount { } func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { - if r.EnableStar != (r.StarPath != "") { - return errors.Errorf("must specify --star-path with --enable-star") + if !r.EnableStar && r.StarPath != "" { + return errors.Errorf("must specify --enable-star with --star-path") } if c.ArgsLenAtDash() >= 0 && r.Image == "" && diff --git a/cmd/config/internal/commands/run_test.go b/cmd/config/internal/commands/run_test.go index 4c3c6d4fc..7a3bf6939 100644 --- a/cmd/config/internal/commands/run_test.go +++ b/cmd/config/internal/commands/run_test.go @@ -183,7 +183,7 @@ apiVersion: v1 "--star-name", "foo", "--", "Foo", "g=h"}, path: "dir", - err: "must specify --star-path with --enable-star", + err: "must specify --enable-star with --star-path", }, { name: "image-star-not-enabled", @@ -193,7 +193,17 @@ apiVersion: v1 "--star-name", "foo", "--", "Foo", "g=h"}, path: "dir", - err: "must specify --star-path with --enable-star", + err: "must specify --enable-star with --star-path", + }, + { + name: "star-enabled", + args: []string{"run", "dir", "--enable-star"}, + path: "dir", + expectedStruct: &runfn.RunFns{ + Path: "dir", + NetworkName: "bridge", + EnableStarlark: true, + }, }, { name: "function paths", diff --git a/functions/examples/application-cr/image/main.go b/functions/examples/application-cr/image/main.go index 2ec0ebfe2..ea2e9b886 100644 --- a/functions/examples/application-cr/image/main.go +++ b/functions/examples/application-cr/image/main.go @@ -109,6 +109,7 @@ func getGroupKinds(in []*yaml.RNode) ([]metav1.GroupKind, error) { found := false for _, gk := range groupKinds { if gk.Group == gvk.Group && gk.Kind == gvk.Kind { + found = true break } } 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/hack/testUnitKustomizePlugins.sh b/hack/testUnitKustomizePlugins.sh index 99ba361ac..354fa652b 100755 --- a/hack/testUnitKustomizePlugins.sh +++ b/hack/testUnitKustomizePlugins.sh @@ -15,7 +15,13 @@ set -o pipefail rcAccumulator=0 function onLinuxAndNotOnTravis { - [[ ("linux" == "$(go env GOOS)") && (-z ${TRAVIS+x}) ]] && return + # TODO: Make the code ignorant of the CI environment "brand name". + # We used to run CI tests on travis, and disabled certain tests + # when running there. Now we run on Prow, so look for that. + # https://github.com/kubernetes/test-infra/blob/master/prow/jobs.md + # Should eschew using the brand name of the CI environment + # (replace "travis" with "CI_env" or something - not just switch to "prow"). + [[ ("linux" == "$(go env GOOS)") && (-z ${$PROW_JOB_ID+x}) ]] && return false } @@ -24,7 +30,7 @@ function runTest { local code=0 if grep -q "// +build notravis" "$file"; then if onLinuxAndNotOnTravis; then - go test -v -tags=notravis $file + go test -v -tags=notravis $file code=$? else # TODO: make work for non-linux @@ -51,7 +57,8 @@ function scanDir { if onLinuxAndNotOnTravis; then # Some of these tests have special deps. - make $(go env GOPATH)/bin/helm + make $(go env GOPATH)/bin/helmV2 + make $(go env GOPATH)/bin/helmV3 make $(go env GOPATH)/bin/kubeval fi @@ -60,7 +67,7 @@ for goMod in $(find ./plugin -name 'go.mod'); do if [[ "$d" == "./plugin/someteam.example.com/v1/gogetter" ]]; then echo "Skipping broken $d" else - scanDir $d + scanDir $d fi done 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) } diff --git a/kyaml/copyutil/copyutil.go b/kyaml/copyutil/copyutil.go index 53e3cd6ff..fa1891110 100644 --- a/kyaml/copyutil/copyutil.go +++ b/kyaml/copyutil/copyutil.go @@ -121,17 +121,23 @@ func Diff(sourceDir, destDir string) (sets.String, error) { return diff, err } if !bytes.Equal(b1, b2) { - dmp := diffmatchpatch.New() - diffs := dmp.DiffMain(string(b1), string(b2), false) - fmt.Println(dmp.DiffPrettyText(diffs)) + fmt.Println(PrettyFileDiff(string(b1), string(b2))) diff.Insert(f) } } - // return the differing files return diff, nil } +// PrettyFileDiff takes the content of two files and returns the pretty diff +func PrettyFileDiff(s1, s2 string) string { + dmp := diffmatchpatch.New() + wSrc, wDst, warray := dmp.DiffLinesToRunes(s1, s2) + diffs := dmp.DiffMainRunes(wSrc, wDst, false) + diffs = dmp.DiffCharsToLines(diffs, warray) + return dmp.DiffPrettyText(diffs) +} + // SyncFile copies file from src file path to a dst file path by replacement // deletes dst file if src file doesn't exist func SyncFile(src, dst string) error { diff --git a/kyaml/copyutil/copyutil_test.go b/kyaml/copyutil/copyutil_test.go index 33446572b..02ac7a57b 100644 --- a/kyaml/copyutil/copyutil_test.go +++ b/kyaml/copyutil/copyutil_test.go @@ -261,3 +261,25 @@ func TestSyncFileNoSrcFile(t *testing.T) { assert.Error(t, err) assert.True(t, strings.Contains(err.Error(), "no such file or directory")) } + +func TestPrettyFileDiff(t *testing.T) { + s1 := `apiVersion: someversion/v1alpha2 +kind: ContainerCluster +metadata: + clusterName: "some_cluster" + name: asm-cluster + namespace: "PROJECT_ID" # {"$ref":"#/definitions/io.k8s.cli.setters.gcloud.core.project"}` + + s2 := `apiVersion: someversion/v1alpha2 +kind: ContainerCluster +metadata: + clusterName: "some_cluster" + name: asm-cluster + namespace: "some_project" # {"$ref":"#/definitions/io.k8s.cli.setters.gcloud.core.project"}` + + expectedLine1 := `[31m namespace: "PROJECT_ID" # {"$ref":"#/definitions/io.k8s.cli.setters.gcloud.core.project"}` + expectedLine2 := `[32m namespace: "some_project" # {"$ref":"#/definitions/io.k8s.cli.setters.gcloud.core.project"}` + + assert.Contains(t, PrettyFileDiff(s1, s2), expectedLine1) + assert.Contains(t, PrettyFileDiff(s1, s2), expectedLine2) +} diff --git a/kyaml/kio/filters/container.go b/kyaml/kio/filters/container.go index 0beb62f21..8c30d0225 100644 --- a/kyaml/kio/filters/container.go +++ b/kyaml/kio/filters/container.go @@ -151,6 +151,10 @@ type ContainerFilter struct { Results *yaml.RNode + DeferFailure bool + + Exit error + // SetFlowStyleForConfig sets the style for config to Flow when serializing it SetFlowStyleForConfig bool @@ -160,7 +164,18 @@ type ContainerFilter struct { checkInput func(string) } +func (c ContainerFilter) GetExit() error { + return c.Exit +} + +type DeferFailureFunction interface { + GetExit() error +} + func (c ContainerFilter) String() string { + if c.DeferFailure { + return fmt.Sprintf("%s deferFailure: %v", c.Image, c.DeferFailure) + } return c.Image } @@ -300,18 +315,9 @@ func (c *ContainerFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { } cmd.Stdin = in cmd.Stdout = out - if err := cmd.Run(); err != nil { - // write the results file on failure - results, e := r.Read() - if e != nil { - return nil, e - } - if e = c.doResults(r); e != nil { - return nil, e - } - // return the results from the function even on failure - return results, err - } + + // don't exit immediately if the function fails -- write out the validation + c.Exit = cmd.Run() output, err := r.Read() if err != nil { @@ -322,6 +328,10 @@ func (c *ContainerFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { return nil, err } + if c.Exit != nil && !c.DeferFailure { + return append(output, saved...), c.Exit + } + // annotate any generated Resources with a path and index if they don't already have one if err := kioutil.DefaultPathAnnotation(functionDir, output); err != nil { return nil, err @@ -380,6 +390,7 @@ func (c *ContainerFilter) getArgs() []string { // tell functions to write error messages to stderr as well as results os.Setenv("LOG_TO_STDERR", "true") + os.Setenv("STRUCTURED_RESULTS", "true") // export the local environment vars to the container for _, pair := range os.Environ() { diff --git a/kyaml/kio/filters/container_test.go b/kyaml/kio/filters/container_test.go index d043e9cba..c960b8b66 100644 --- a/kyaml/kio/filters/container_test.go +++ b/kyaml/kio/filters/container_test.go @@ -18,13 +18,14 @@ import ( func TestContainerFilter_Filter(t *testing.T) { var tests = []struct { - name string - input []string - expectedOutput []string - expectedError string - expectedResults string - noMakeResultsFile bool - instance ContainerFilter + name string + input []string + expectedOutput []string + expectedError string + expectedSavedError string + expectedResults string + noMakeResultsFile bool + instance ContainerFilter }{ { name: "add_path_annotation", @@ -220,6 +221,87 @@ metadata: `, }, + { + name: "write_results_defer_failure", + expectedSavedError: "exit status 1", + instance: ContainerFilter{ + DeferFailure: true, + args: []string{"sh", "-c", + `echo ' +apiVersion: config.kubernetes.io/v1alpha1 +kind: ResourceList +items: +- apiVersion: apps/v1 + kind: Deployment + metadata: + name: deployment-foo +- apiVersion: v1 + kind: Service + metadata: + name: service-foo +results: +- apiVersion: config.k8s.io/v1alpha1 + kind: ObjectError + name: "some-validator" + items: + - type: error + message: "some message" + resourceRef: + apiVersion: apps/v1 + kind: Deployment + name: foo + namespace: bar + file: + path: deploy.yaml + index: 0 + field: + path: "spec.template.spec.containers[3].resources.limits.cpu" + currentValue: "200" + suggestedValue: "2" +' && cat not-real-dir +`, + }, + }, + expectedOutput: []string{ + ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: deployment-foo + annotations: + config.kubernetes.io/path: 'deployment_deployment-foo.yaml' +`, + ` +apiVersion: v1 +kind: Service +metadata: + name: service-foo + annotations: + config.kubernetes.io/path: 'service_service-foo.yaml' +`, + }, + expectedResults: ` +- apiVersion: config.k8s.io/v1alpha1 + kind: ObjectError + name: "some-validator" + items: + - type: error + message: "some message" + resourceRef: + apiVersion: apps/v1 + kind: Deployment + name: foo + namespace: bar + file: + path: deploy.yaml + index: 0 + field: + path: "spec.template.spec.containers[3].resources.limits.cpu" + currentValue: "200" + suggestedValue: "2" +`, + }, + { name: "write_results_non_0_exit_missing_file", expectedError: "open /not/real/file: no such file or directory", @@ -332,6 +414,13 @@ metadata: return } + if tt.expectedSavedError != "" { + if !assert.EqualError(t, tt.instance.Exit, tt.expectedSavedError) { + t.FailNow() + } + return + } + if !assert.NoError(t, err) { t.FailNow() } diff --git a/kyaml/kio/filters/functiontypes.go b/kyaml/kio/filters/functiontypes.go index 5145adf46..212cb589c 100644 --- a/kyaml/kio/filters/functiontypes.go +++ b/kyaml/kio/filters/functiontypes.go @@ -19,6 +19,8 @@ type FunctionSpec struct { // Network is the name of the network to use from a container Network string `json:"network,omitempty" yaml:"network,omitempty"` + DeferFailure bool `json:"deferFailure,omitempty" yaml:"deferFailure,omitempty"` + // Container is the spec for running a function as a container Container ContainerSpec `json:"container,omitempty" yaml:"container,omitempty"` diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index 0d6ec195f..6802259b7 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -164,7 +164,27 @@ func (r RunFns) runFunctions( // the output is nil (reading from Input) outputs = append(outputs, kio.ByteWriter{Writer: r.Output}) } - return kio.Pipeline{Inputs: []kio.Reader{input}, Filters: fltrs, Outputs: outputs}.Execute() + err := kio.Pipeline{ + Inputs: []kio.Reader{input}, Filters: fltrs, Outputs: outputs}.Execute() + if err != nil { + return err + } + + // check for deferred function errors + var errs []string + for i := range fltrs { + cf, ok := fltrs[i].(filters.DeferFailureFunction) + if !ok { + continue + } + if cf.GetExit() != nil { + errs = append(errs, cf.GetExit().Error()) + } + } + if len(errs) > 0 { + return fmt.Errorf(strings.Join(errs, "\n---\n")) + } + return nil } // getFunctionsFromInput scans the input for functions and runs them @@ -328,6 +348,7 @@ func (r *RunFns) ffp(spec filters.FunctionSpec, api *yaml.RNode) (kio.Filter, er StorageMounts: r.StorageMounts, GlobalScope: r.GlobalScope, ResultsFile: resultsFile, + DeferFailure: spec.DeferFailure, }, nil } if r.EnableStarlark && spec.Starlark.Path != "" { diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go index b0e5cd92f..e725bcb58 100644 --- a/kyaml/runfn/runfn_test.go +++ b/kyaml/runfn/runfn_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/kyaml/copyutil" + "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/kio/filters" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -234,6 +235,29 @@ metadata: out: []string{"gcr.io/example.com/image:v1.0.0"}, }, + // Test + // + // + {name: "defer_failure", + in: []f{ + { + path: filepath.Join("foo", "bar.yaml"), + value: ` +apiVersion: example.com/v1alpha1 +kind: ExampleFunction +metadata: + annotations: + config.kubernetes.io/function: | + deferFailure: true + container: + image: gcr.io/example.com/image:v1.0.0 + config.kubernetes.io/local-config: "true" +`, + }, + }, + out: []string{"gcr.io/example.com/image:v1.0.0 deferFailure: true"}, + }, + {name: "disable containers", in: []f{ { @@ -674,6 +698,93 @@ func TestCmd_Execute(t *testing.T) { assert.Contains(t, string(b), "kind: StatefulSet") } +type TestFilter struct { + invoked bool + Exit error +} + +func (f *TestFilter) Filter(input []*yaml.RNode) ([]*yaml.RNode, error) { + f.invoked = true + return input, nil +} + +func (f *TestFilter) GetExit() error { + return f.Exit +} + +func TestCmd_Execute_deferFailure(t *testing.T) { + dir := setupTest(t) + defer os.RemoveAll(dir) + + // write a test filter to the directory of configuration + if !assert.NoError(t, ioutil.WriteFile( + filepath.Join(dir, "filter1.yaml"), []byte(`apiVersion: v1 +kind: ValueReplacer +metadata: + annotations: + config.kubernetes.io/function: | + container: + image: 1 + config.kubernetes.io/local-config: "true" +stringMatch: Deployment +replace: StatefulSet +`), 0600)) { + t.FailNow() + } + + // write a test filter to the directory of configuration + if !assert.NoError(t, ioutil.WriteFile( + filepath.Join(dir, "filter2.yaml"), []byte(`apiVersion: v1 +kind: ValueReplacer +metadata: + annotations: + config.kubernetes.io/function: | + container: + image: 2 + config.kubernetes.io/local-config: "true" +stringMatch: Deployment +replace: StatefulSet +`), 0600)) { + t.FailNow() + } + + var fltrs []*TestFilter + instance := RunFns{ + Path: dir, + functionFilterProvider: func(f filters.FunctionSpec, node *yaml.RNode) (kio.Filter, error) { + tf := &TestFilter{ + Exit: errors.Errorf("message: %s", f.Container.Image), + } + fltrs = append(fltrs, tf) + return tf, nil + }, + } + instance.init() + + err := instance.Execute() + + // make sure all filters were run + if !assert.Equal(t, 2, len(fltrs)) { + t.FailNow() + } + for i := range fltrs { + if !assert.True(t, fltrs[i].invoked) { + t.FailNow() + } + } + + if !assert.EqualError(t, err, "message: 1\n---\nmessage: 2") { + t.FailNow() + } + b, err := ioutil.ReadFile( + filepath.Join(dir, "java", "java-deployment.resource.yaml")) + if !assert.NoError(t, err) { + t.FailNow() + } + // files weren't changed because there was an error + assert.Contains(t, string(b), "kind: Deployment") +} + // TestCmd_Execute_setOutput tests the execution of a filter reading and writing to a dir func TestCmd_Execute_setFunctionPaths(t *testing.T) { dir := setupTest(t) diff --git a/plugin/builtin/patchtransformer/PatchTransformer_test.go b/plugin/builtin/patchtransformer/PatchTransformer_test.go index 173a50881..7373f6437 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer_test.go +++ b/plugin/builtin/patchtransformer/PatchTransformer_test.go @@ -102,7 +102,6 @@ patch: "thisIsNotAPatch" if err == nil { t.Fatalf("expected error") } - fmt.Print(err) if !strings.Contains(err.Error(), "unable to parse SM or JSON patch from ") { t.Fatalf("unexpected err: %v", err) diff --git a/plugin/someteam.example.com/v1/chartinflator/ChartInflator b/plugin/someteam.example.com/v1/chartinflator/ChartInflator index 7bf69be3c..aba44f26e 100755 --- a/plugin/someteam.example.com/v1/chartinflator/ChartInflator +++ b/plugin/someteam.example.com/v1/chartinflator/ChartInflator @@ -115,25 +115,69 @@ if [ -z "$releaseNamespace" ]; then releaseNamespace=default fi -function doHelm { - $helmBin --home $helmHome $@ +function v2RunHelm { + $helmBin --home $helmHome "$@" } -# The init command is extremely chatty -doHelm init --client-only >& /dev/null +function v3RunHelm { + XDG_CONFIG_DIR=$helmHome + $helmBin "$@" +} -if [ ! -d "$chartHome/$chartName" ]; then - doHelm fetch $chartVersionArg \ - $chartRepoArg \ - --untar \ - --untardir $chartHome \ - $chartNameArg -fi +function v2InitHelm { + # The init command is extremely chatty + v2RunHelm init --client-only >& /dev/null +} -doHelm template \ - --name $releaseName \ - --namespace $releaseNamespace \ - --values $valuesFile \ - $chartHome/$chartName +function v3InitHelm { + # Do nothing - there's no init command in helm v3 + true +} + +function v2PullChart { + if [ ! -d "$chartHome/$chartName" ]; then + v2RunHelm fetch $chartVersionArg \ + $chartRepoArg \ + --untar \ + --untardir $chartHome \ + $chartNameArg + fi +} + +function v3PullChart { + # TODO implement + echo "?? helmV3 pull --repo https://hub.helm.sh/charts/ stable/minecraft --destination /tmp/junk" +} + +function v2InflateChart { + v2RunHelm template \ + --name $releaseName \ + --namespace $releaseNamespace \ + --values $valuesFile \ + $chartHome/$chartName +} + +function v3InflateChart { + # TODO implement + true +} + +HELM_VERSION=$($helmBin version -c --short) + +case $HELM_VERSION in + 'Client: v2'*) + v2InitHelm + v2PullChart + v2InflateChart + ;; + v3*) + v3InitHelm + v3PullChart + v3InflateChart + ;; + *) + echo "[!] Unsupported 'helm' version '${HELM_VERSION}'" 1>&2 && exit 1 + ;; +esac /bin/rm -rf $TMP_DIR diff --git a/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go b/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go index 2571a4321..2d41fe3e8 100644 --- a/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go +++ b/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go @@ -14,27 +14,7 @@ import ( kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) -// This test requires having the helm binary on the PATH. -// -// TODO: Download and inflate the chart, and check that -// in for the test. -func TestChartInflator(t *testing.T) { - th := kusttest_test.MakeEnhancedHarness(t). - PrepExecPlugin("someteam.example.com", "v1", "ChartInflator") - defer th.Reset() - - m := th.LoadAndRunGenerator(` -apiVersion: someteam.example.com/v1 -kind: ChartInflator -metadata: - name: notImportantHere -chartName: minecraft`) - - chartName := regexp.MustCompile("chart: minecraft-[0-9.]+") - th.AssertActualEqualsExpectedWithTweak(m, - func(x []byte) []byte { - return chartName.ReplaceAll(x, []byte("chart: minecraft-SOMEVERSION")) - }, ` +const expectedResources = ` apiVersion: v1 data: rcon-password: Q0hBTkdFTUUh @@ -84,5 +64,52 @@ spec: selector: app: release-name-minecraft type: LoadBalancer +` + +// This test requires having "helmV2" (presumably helm V2 series) on the PATH. +// +// TODO: Download and inflate the chart, and check that +// in for the test. +func TestHelmV2ChartInflator(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t). + PrepExecPlugin("someteam.example.com", "v1", "ChartInflator") + defer th.Reset() + + m := th.LoadAndRunGenerator(` +apiVersion: someteam.example.com/v1 +kind: ChartInflator +metadata: + name: notImportantHere +chartName: minecraft +helmBin: helmV2 `) + + chartName := regexp.MustCompile("chart: minecraft-[0-9.]+") + th.AssertActualEqualsExpectedWithTweak(m, + func(x []byte) []byte { + return chartName.ReplaceAll(x, []byte("chart: minecraft-SOMEVERSION")) + }, expectedResources) +} + +// This test requires having "helmV3" (presumably helm V3 series) on the PATH. +// +func disabled_TestHelmV3ChartInflator(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t). + PrepExecPlugin("someteam.example.com", "v1", "ChartInflator") + defer th.Reset() + + m := th.LoadAndRunGenerator(` +apiVersion: someteam.example.com/v1 +kind: ChartInflator +metadata: + name: notImportantHere +chartName: minecraft +helmBin: helmV3 +`) + + chartName := regexp.MustCompile("chart: minecraft-[0-9.]+") + th.AssertActualEqualsExpectedWithTweak(m, + func(x []byte) []byte { + return chartName.ReplaceAll(x, []byte("chart: minecraft-SOMEVERSION")) + }, expectedResources) } diff --git a/releasing/VERSIONS b/releasing/VERSIONS index a09277c7a..a1364fa6d 100644 --- a/releasing/VERSIONS +++ b/releasing/VERSIONS @@ -6,7 +6,7 @@ # kyaml version export kyaml_major=0 export kyaml_minor=1 -export kyaml_patch=5 +export kyaml_patch=6 # kstatus version export kstatus_major=0 @@ -21,7 +21,7 @@ export api_patch=3 # cmd/config version export cmd_config_major=0 export cmd_config_minor=1 -export cmd_config_patch=4 +export cmd_config_patch=5 # cmd/kubectl version export cmd_kubectl_major=0