diff --git a/Makefile b/Makefile index c6d4ced6e..bb1b6c0de 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 @@ -288,6 +294,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 75d3a961f..2ca04c437 100644 --- a/api/internal/plugins/compiler/compiler.go +++ b/api/internal/plugins/compiler/compiler.go @@ -6,6 +6,7 @@ package compiler import ( "bytes" "fmt" + "log" "os" "os/exec" "path/filepath" @@ -16,61 +17,74 @@ import ( ) // 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 + // 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") } -// 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 FileYoungerThan(objFile, time.Minute) { - // Skip rebuilding it. +func (b *Compiler) objFile() string { + return b.rawKind + ".so" +} + +// 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) { @@ -78,12 +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 } + +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 e30c1ca14..89640f7b1 100644 --- a/api/internal/plugins/compiler/compiler_test.go +++ b/api/internal/plugins/compiler/compiler_test.go @@ -4,8 +4,6 @@ package compiler_test import ( - "io/ioutil" - "os" "path/filepath" "testing" "time" @@ -16,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 !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 !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/testutils/kusttest/harness.go b/api/testutils/kusttest/harness.go index ef059b03a..b35969670 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -70,8 +70,7 @@ func (th Harness) MakeOptionsPluginsDisabled() krusty.Options { // Enables use of non-builtin plugins. func (th Harness) MakeOptionsPluginsEnabled() krusty.Options { - // TODO: Change to types.BploLoadFromFileSys to enable debugging. - c, err := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) + 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 40b2d46f0..6ed84af14 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -47,8 +47,7 @@ type HarnessEnhanced struct { func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced { pte := newPluginTestEnv(t).set() - // TODO: Change to types.BploLoadFromFileSys to enable debugging. - pc, err := konfig.EnabledPluginConfig(types.BploUseStaticallyLinked) + pc, err := konfig.EnabledPluginConfig(types.BploLoadFromFileSys) if err != nil { t.Fatal(err) } diff --git a/api/testutils/kusttest/plugintestenv.go b/api/testutils/kusttest/plugintestenv.go index 69adc23ec..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" @@ -17,16 +13,13 @@ import ( ) // pluginTestEnv manages compiling plugins for tests. -// It manages a Go plugin compiler, -// maybe makes and removes a temporary working directory, -// maybe sets/resets shell env vars as needed. +// 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() } // 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. +// created object code alongside the src code. func (x *pluginTestEnv) prepareGoPlugin(g, v, k string) { - err := x.compiler.Compile(g, v, k) + x.compiler.SetGVK(g, v, k) + err := x.compiler.Compile() if err != nil { x.t.Errorf("compile failed: %v", err) } } -// 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 -// (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) 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) - 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/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)