Merge pull request #1663 from chaosteil/pluginloadfail

Refactor exec plugin load to error if plugin isn't executable

api/plugins/execplugin/execplugin.go

@@ -44,8 +44,15 @@ type ExecPlugin struct {
 	h *resmap.PluginHelpers
 }
 
-func NewExecPlugin(p string) *ExecPlugin {
-	return &ExecPlugin{path: p}
+func NewExecPlugin(p string) (*ExecPlugin, error) {
+	f, err := os.Stat(p)
+	if err != nil {
+		return nil, err
+	}
+	if f.Mode()&0111 == 0000 {
+		return nil, fmt.Errorf("unable to execute plugin on path: %s", p)
+	}
+	return &ExecPlugin{path: p}, nil
 }
 
 func (p *ExecPlugin) Path() string {
@@ -60,15 +67,6 @@ func (p *ExecPlugin) Cfg() []byte {
 	return p.cfg
 }
 
-// isAvailable checks to see if the plugin is available
-func (p *ExecPlugin) IsAvailable() bool {
-	f, err := os.Stat(p.path)
-	if os.IsNotExist(err) {
-		return false
-	}
-	return f.Mode()&0111 != 0000
-}
-
 func (p *ExecPlugin) Config(h *resmap.PluginHelpers, config []byte) error {
 	p.h = h
 	p.cfg = config

api/plugins/execplugin/execplugin_test.go

@@ -5,6 +5,7 @@ package execplugin_test
 
 import (
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 
@@ -15,7 +16,7 @@ import (
 	"sigs.k8s.io/kustomize/api/plugins/loader"
 	"sigs.k8s.io/kustomize/api/resmap"
 	"sigs.k8s.io/kustomize/api/resource"
-	"sigs.k8s.io/kustomize/api/testutils/valtest"
+	valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest"
 	"sigs.k8s.io/kustomize/api/types"
 )
 
@@ -43,10 +44,13 @@ s/$BAR/bar/g
  \ \ \ 
 `))
 
-	p := NewExecPlugin(
+	p, err := NewExecPlugin(
 		loader.AbsolutePluginPath(
 			config.DefaultPluginConfig(),
 			pluginConfig.OrgId()))
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err.Error())
+	}
 
 	yaml, err := pluginConfig.AsYAML()
 	if err != nil {
@@ -114,7 +118,10 @@ func strptr(s string) *string {
 }
 
 func TestUpdateResourceOptions(t *testing.T) {
-	p := NewExecPlugin("")
+	p, err := NewExecPlugin("")
+	if !os.IsNotExist(err) {
+		t.Fatalf("unexpected error: %v", err.Error())
+	}
 	rf := resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl())
 	in := resmap.New()
 	expected := resmap.New()
@@ -159,7 +166,10 @@ func TestUpdateResourceOptions(t *testing.T) {
 }
 
 func TestUpdateResourceOptionsWithInvalidHashAnnotationValues(t *testing.T) {
-	p := NewExecPlugin("")
+	p, err := NewExecPlugin("")
+	if !os.IsNotExist(err) {
+		t.Fatalf("unexpected error: %v", err.Error())
+	}
 	rf := resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl())
 	cases := []string{
 		"",

api/plugins/loader/loader.go

@@ -5,6 +5,7 @@ package loader
 
 import (
 	"fmt"
+	"os"
 	"path/filepath"
 	"plugin"
 	"reflect"
@@ -145,10 +146,13 @@ func (l *Loader) makeBuiltinPlugin(r resid.Gvk) (resmap.Configurable, error) {
 }
 
 func (l *Loader) loadPlugin(resId resid.ResId) (resmap.Configurable, error) {
-	p := execplugin.NewExecPlugin(l.absolutePluginPath(resId))
-	if p.IsAvailable() {
+	p, err := execplugin.NewExecPlugin(l.absolutePluginPath(resId))
+	if err == nil {
 		return p, nil
 	}
+	if err != nil && !os.IsNotExist(err) {
+		return nil, err
+	}
 	c, err := l.loadGoPlugin(resId)
 	if err != nil {
 		return nil, err

docs/INSTALL.md

@@ -21,8 +21,25 @@ curl -s https://api.github.com/repos/kubernetes-sigs/kustomize/releases/latest |
   grep $opsys |\
   cut -d '"' -f 4 |\
   xargs curl -O -L
-mv kustomize_kustomize\.v*_${opsys}_amd64 kustomize
-chmod u+x kustomize
+tar xzf ./kustomize_v*_${opsys}_amd64.tar.gz
+./kustomize version
+```
+## Try `go`
+
+This method is more to show off how the `go` tool works,
+than for any practical purpose.  A kustomize developer should
+clone the repo (see next section), and CI/CD scripts should 
+download a specific ready-to-run executable rather than
+rely on the `go` tool.
+
+Install the latest kustomize binary in the v3 series to `$GOPATH/bin`:
+```
+go install sigs.k8s.io/kustomize/kustomize/v3
+```
+
+Install a specific version:
+```
+go get sigs.k8s.io/kustomize/kustomize/v3@v3.3.0
 ```
 
 ## Build the kustomize CLI from local source
@@ -50,15 +67,6 @@ git checkout kustomize/v3.2.3
 
 ### Other methods
 
-#### Use go get
-
-This works poorly with existing `Go` package installations at the
-moment, since kustomize switched over to Go modules but hasn't
-historically followed semver with respect to its API.
-
-This is being [fixed](versioningPolicy.md), after which
-`go get` should work correctly.
-
 #### macOS
 
 ```

releasing/README.md

@@ -39,7 +39,8 @@ files.
 #### Packages
 
 There's only one public package in this module.
-It's called `main`, and it holds the _kustomize_ executable.
+It's called `main`, it's therefore unimportable,
+and it holds the _kustomize_ executable.
 
 
 #### Release artifacts
@@ -69,18 +70,18 @@ Go consumers of this API will have a `go.mod` file
 requiring this module at a particular tag, e.g.
 
 ```
-require sigs.k8s.io/kustomize/api v0.0.1
+require sigs.k8s.io/kustomize/api v0.1.0
 ```
 
 #### Release artifacts
 
-This is a Go library only release, so the only
-artifact per se is the repo tag, in the form `v4.3.2`,
+This is a Go library-only release, so the only
+artifact per se is the repo tag, in the form `api/v1.2.3`,
 that API clients can `require` from their `go.mod` file.
 
 Release notes should appear on the [release page].
 
-There's an executable called `api`, which, if
+There's a toy executable called `api`, which, if
 run, prints the API release provenance data, but it's of
 no practical use to an API client.
 
@@ -92,14 +93,14 @@ linkable library code.
 
 #### Packages
 
-There's only one package in this module.  It's called `main`,
+There's only one package in this module.
+It's called `main`, it's therefore unimportable,
 and it holds the _pluginator_ executable.
 
 At the time of writing this binary is only of
 interest to someone writing a new builtin
 transformer or generator.  See the [plugin
 documentation](../docs/plugins).
-
 Its dependence on the API is primarily for
 plugin-related constants, not logic, and will
 only change if there's some change in how
@@ -113,9 +114,17 @@ The tag appears in the URL, e.g. [pluginator/v1.0.0].
 
 ## Release procedure
 
-> TODO: script this, e.g. `go run ./releasing/release.go kustomize minor`
-> would look at the tags, bump the minor version, and
-> create the right branch, tag, etc.  No more bash please.
+> TODO: script what follows, so someone can enter
+> ```
+> go run ./releasing/release.go kustomize minor
+> # or:
+> # go run ./releasing/release.go api patch
+> # go run ./releasing/release.go pluginator minor
+> ```
+> The program would look at the existing remote tags,
+> confirm sanity and increment the appropriate major/minor/patch
+> component, create the right branch and tag, etc.
+> No more bash please.
 
 At any given moment, the repository's master branch is
 passing all its tests and contains code one could release.