Repository: GoogleContainerTools/krew
Pull request #77 (2018-10-08 21:51:14)

cmd: print stack traces for errors when glog.V>=1
- Added a decorator function that uses glog.V() to print stack traces from
  pkg/errors with %+v.
- Fixed a minor bug where specifying a non-existing plugin name to "upgrade"
  cmd was resulting in "" to be added to error string.

Examples:

	➜  kubectl krew upgrade foobar
	Updated index

	➜  kubectl krew upgrade foobar -v=1
	I1008 14:45:39.200042   15234 git.go:62] Going to run git pull --ff-only -v
	From https://github.com/GoogleContainerTools/krew-index
	 = [up to date]      master     -> origin/master
	Already up to date.
	Updated index
	I1008 14:45:39.669936   15234 scanner.go:74] Reading plugin "foobar"
	F1008 14:45:39.670188   15234 root.go:51] ERROR=open /Users/ahmetb/.krew/index/plugins/foobar.yaml: no such file or directory
	failed to load the index file for plugin foobar
	github.com/GoogleContainerTools/krew/cmd/krew/cmd.glob..func4
		/Users/ahmetb/workspace/gopath-krew/src/github.com/GoogleContainerTools/krew/cmd/krew/cmd/upgrade.go:59
	github.com/GoogleContainerTools/krew/cmd/krew/cmd.withStackTrace.func1
		/Users/ahmetb/workspace/gopath-krew/src/github.com/GoogleContainerTools/krew/cmd/krew/cmd/root.go:113
	github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra.(*Command).execute
		/Users/ahmetb/workspace/gopath-krew/src/github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra/command.go:762
	github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra.(*Command).ExecuteC
		/Users/ahmetb/workspace/gopath-krew/src/github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra/command.go:852
	github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra.(*Command).Execute
		/Users/ahmetb/workspace/gopath-krew/src/github.com/GoogleContainerTools/krew/vendor/github.com/spf13/cobra/command.go:800
	github.com/GoogleContainerTools/krew/cmd/krew/cmd.Execute
		/Users/ahmetb/workspace/gopath-krew/src/github.com/GoogleContainerTools/krew/cmd/krew/cmd/root.go:50
	main.main
		/Users/ahmetb/workspace/gopath-krew/src/github.com/GoogleContainerTools/krew/cmd/krew/main.go:23
	runtime.main
		/Users/ahmetb/.homebrew/Cellar/go/1.11.1/libexec/src/runtime/proc.go:201
	runtime.goexit
		/Users/ahmetb/.homebrew/Cellar/go/1.11.1/libexec/src/runtime/asm_amd64.s:1333

codecov-io: # [Codecov](https://codecov.io/gh/GoogleContainerTools/krew/pull/77?src=pr&el=h1) Report
> Merging [#77](https://codecov.io/gh/GoogleContainerTools/krew/pull/77?src=pr&el=desc) into [master](https://codecov.io/gh/GoogleContainerTools/krew/commit/8b32cf246ab12f47956bdd4adf9fe83a78f6bc8a?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/GoogleContainerTools/krew/pull/77/graphs/tree.svg?width=650&token=J3nFtARI2m&height=150&src=pr)](https://codecov.io/gh/GoogleContainerTools/krew/pull/77?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master      #77   +/-   ##
=======================================
  Coverage   46.16%   46.16%           
=======================================
  Files          12       12           
  Lines         574      574           
=======================================
  Hits          265      265           
  Misses        262      262           
  Partials       47       47
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/GoogleContainerTools/krew/pull/77?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/GoogleContainerTools/krew/pull/77?src=pr&el=footer). Last update [8b32cf2...e8b7071](https://codecov.io/gh/GoogleContainerTools/krew/pull/77?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

lbb: Is it possible to wrap the whole cobra command, instead of every entry point?
https://github.com/GoogleContainerTools/krew/blob/8b32cf246ab12f47956bdd4adf9fe83a78f6bc8a/cmd/krew/cmd/root.go#L48-L52
ahmetb: @lbb you're right lol
ahmetb: Updated.

Patch:
diff --git a/cmd/krew/cmd/info.go b/cmd/krew/cmd/info.go
index 312dca1..fc83de8 100644
--- a/cmd/krew/cmd/info.go
+++ b/cmd/krew/cmd/info.go
@@ -21,9 +21,9 @@ import (
 
 	"github.com/GoogleContainerTools/krew/pkg/index"
 	"github.com/GoogleContainerTools/krew/pkg/index/indexscanner"
-
 	"github.com/GoogleContainerTools/krew/pkg/installation"
-	"github.com/golang/glog"
+
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -33,16 +33,17 @@ var infoCmd = &cobra.Command{
 	Short: "Info shows plugin details",
 	Long: `Info shows plugin details.
 Use this command to find out about plugin requirements and caveats.`,
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		for _, arg := range args {
 			plugin, err := indexscanner.LoadPluginFileFromFS(paths.IndexPath(), arg)
 			if os.IsNotExist(err) {
-				glog.Fatalf("plugin %q not found", arg)
+				return errors.Errorf("plugin %q not found", arg)
 			} else if err != nil {
-				glog.Fatal(err)
+				return errors.Wrap(err, "failed to load plugin manifest")
 			}
 			printPluginInfo(os.Stdout, plugin)
 		}
+		return nil
 	},
 	PreRunE: checkIndex,
 	Args:    cobra.MinimumNArgs(1),
diff --git a/cmd/krew/cmd/install.go b/cmd/krew/cmd/install.go
index a2b9c67..daa5f11 100644
--- a/cmd/krew/cmd/install.go
+++ b/cmd/krew/cmd/install.go
@@ -67,7 +67,7 @@ All plugins will be downloaded and made available to: "kubectl plugin <name>"`,
 			for _, name := range pluginNames {
 				plugin, err := indexscanner.LoadPluginFileFromFS(paths.IndexPath(), name)
 				if err != nil {
-					return errors.Wrapf(err, "failed to load plugin %q from index", name)
+					return errors.Wrapf(err, "failed to load plugin %q from the index", name)
 				}
 				install = append(install, plugin)
 			}
diff --git a/cmd/krew/cmd/root.go b/cmd/krew/cmd/root.go
index 2fcbe45..37783ab 100644
--- a/cmd/krew/cmd/root.go
+++ b/cmd/krew/cmd/root.go
@@ -47,7 +47,11 @@ You can invoke krew through kubectl with: "kubectl plugin [krew] option..."`,
 // This is called by main.main(). It only needs to happen once to the rootCmd.
 func Execute() {
 	if err := rootCmd.Execute(); err != nil {
-		glog.Fatal(err)
+		if glog.V(1) {
+			glog.Fatalf("%+v", err) // with stack trace
+		} else {
+			glog.Fatal(err) // just error message
+		}
 	}
 }
 
@@ -57,7 +61,6 @@ func init() {
 	flag.Set("logtostderr", "true")
 	// Required by glog
 	flag.Parse()
-
 	paths = environment.MustGetKrewPaths()
 	if err := ensureDirs(paths.BasePath(),
 		paths.DownloadPath(),
@@ -90,7 +93,7 @@ func setGlogFlags(hidden bool) {
 
 func checkIndex(_ *cobra.Command, _ []string) error {
 	if ok, err := gitutil.IsGitCloned(paths.IndexPath()); err != nil {
-		return err
+		return errors.Wrap(err, "failed to check local index git repository")
 	} else if !ok {
 		return errors.New(`krew local plugin index is not initialized (run "krew update")`)
 	}
diff --git a/pkg/index/validate.go b/pkg/index/validate.go
index 5d673ac..10d0dcd 100644
--- a/pkg/index/validate.go
+++ b/pkg/index/validate.go
@@ -21,10 +21,17 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	currentAPIVersion = "krew.googlecontainertools.github.com/v1alpha2"
+)
+
 var (
 	safePluginRegexp = regexp.MustCompile(`^[\w-]+$`)
+
 	// windowsForbidden is taken from  https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file
-	windowsForbidden = []string{"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"}
+	windowsForbidden = []string{"CON", "PRN", "AUX", "NUL", "COM1", "COM2",
+		"COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2",
+		"LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"}
 )
 
 // IsSafePluginName checks if the plugin Name is save to use.
@@ -40,8 +47,16 @@ func IsSafePluginName(name string) bool {
 	return true
 }
 
+func isSupportedAPIVersion(apiVersion string) bool {
+	return apiVersion == currentAPIVersion
+}
+
 // Validate TODO(lbb)
 func (p Plugin) Validate(name string) error {
+	if !isSupportedAPIVersion(p.APIVersion) {
+		return errors.Errorf("plugin manifest has apiVersion=%q, not supported in this version of krew (try updating plugin index or install a newer version of krew)", p.APIVersion)
+	}
+
 	if !IsSafePluginName(name) {
 		return errors.Errorf("the plugin name %q is not allowed, must match %q", name, safePluginRegexp.String())
 	}


Test patch:
diff --git a/pkg/index/indexscanner/testdata/testindex/dontscan.yaml b/pkg/index/indexscanner/testdata/testindex/dontscan.yaml
index ca49915..a963065 100644
--- a/pkg/index/indexscanner/testdata/testindex/dontscan.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/dontscan.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-apiVersion: plugin.krew.app/v1alpha1
+apiVersion: krew.googlecontainertools.github.com/v1alpha2
 kind: Plugin
 metadata:
   name: dontscan
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/badplugin.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/badplugin.yaml
index 2285923..eccc518 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/badplugin.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/badplugin.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-apiVersion: plugin.krew.app/v1alpha1
+apiVersion: krew.googlecontainertools.github.com/v1alpha2
 kind: Plugin
 metadata:
   name: badplugin
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/badplugin2.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/badplugin2.yaml
index a17029d..2c47451 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/badplugin2.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/badplugin2.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-apiVersion: plugin.krew.app/v1alpha1
+apiVersion: krew.googlecontainertools.github.com/v1alpha2
 kind: Plugin
 metadata:
   name: badplugin
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
index 9c67846..87b28ff 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-apiVersion: plugin.krew.app/v1alpha1
+apiVersion: krew.googlecontainertools.github.com/v1alpha2
 kind: Plugin
 metadata:
   name: bar
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml
index ec25b88..4f050cb 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-apiVersion: plugin.krew.app/v1alpha1
+apiVersion: krew.googlecontainertools.github.com/v1alpha2
 kind: Plugin
 metadata:
   name: foo
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/notyaml.txt b/pkg/index/indexscanner/testdata/testindex/plugins/notyaml.txt
index 253a6c5..d1bd3f0 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/notyaml.txt
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/notyaml.txt
@@ -1,4 +1,4 @@
-apiVersion: plugin.krew.app/v1alpha1
+apiVersion: krew.googlecontainertools.github.com/v1alpha2
 kind: Plugin
 metadata:
   name: notyaml-plugin
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/wrongname.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/wrongname.yaml
index 68f8718..64dda4f 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/wrongname.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/wrongname.yaml
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-apiVersion: plugin.krew.app/v1alpha1
+apiVersion: krew.googlecontainertools.github.com/v1alpha2
 kind: Plugin
 metadata:
   name: mismatchedname
diff --git a/pkg/index/validate_test.go b/pkg/index/validate_test.go
index d991a55..60f4c8d 100644
--- a/pkg/index/validate_test.go
+++ b/pkg/index/validate_test.go
@@ -67,24 +67,45 @@ func Test_IsSafePluginName(t *testing.T) {
 	}
 }
 
+func Test_isSupportedAPIVersion(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		want bool
+	}{
+		{"wrong group", "networking.k8s.io/v1", false},
+		{"just api group", "krew.googlecontainertools.github.com", false},
+		{"old version", "krew.googlecontainertools.github.com/v1alpha1", false},
+		{"equal version", "krew.googlecontainertools.github.com/v1alpha2", true},
+		{"newer 1", "krew.googlecontainertools.github.com/v1alpha3", false},
+		{"newer 2", "krew.googlecontainertools.github.com/v1", false},
+		{"newer 2", "krew.googlecontainertools.github.com/v2alpha1", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isSupportedAPIVersion(tt.in); got != tt.want {
+				t.Errorf("isSupportedAPIVersion(%v) = %v, want %v", tt.in, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestPlugin_Validate(t *testing.T) {
 	type fields struct {
 		TypeMeta   metav1.TypeMeta
 		ObjectMeta metav1.ObjectMeta
 		Spec       PluginSpec
 	}
-	type args struct {
-		name string
-	}
 	tests := []struct {
-		name    string
-		fields  fields
-		args    args
-		wantErr bool
+		name       string
+		fields     fields
+		pluginName string
+		wantErr    bool
 	}{
 		{
 			name: "validate success",
 			fields: fields{
+				TypeMeta:   metav1.TypeMeta{APIVersion: currentAPIVersion},
 				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
 				Spec: PluginSpec{
 					Version:          "",
@@ -101,14 +122,36 @@ func TestPlugin_Validate(t *testing.T) {
 					}},
 				},
 			},
-			args: args{
-				name: "foo",
+			pluginName: "foo",
+			wantErr:    false,
+		},
+		{
+			name: "bad api version",
+			fields: fields{
+				TypeMeta:   metav1.TypeMeta{APIVersion: "core/v1"},
+				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
+				Spec: PluginSpec{
+					Version:          "",
+					ShortDescription: "short",
+					Description:      "",
+					Caveats:          "",
+					Platforms: []Platform{{
+						Head:     "http://example.com",
+						URI:      "",
+						Sha256:   "",
+						Selector: nil,
+						Files:    []FileOperation{{"", ""}},
+						Bin:      "foo",
+					}},
+				},
 			},
-			wantErr: false,
+			pluginName: "foo",
+			wantErr:    true,
 		},
 		{
 			name: "no short description",
 			fields: fields{
+				TypeMeta:   metav1.TypeMeta{APIVersion: currentAPIVersion},
 				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
 				Spec: PluginSpec{
 					Version:          "",
@@ -125,14 +168,13 @@ func TestPlugin_Validate(t *testing.T) {
 					}},
 				},
 			},
-			args: args{
-				name: "foo",
-			},
-			wantErr: true,
+			pluginName: "foo",
+			wantErr:    true,
 		},
 		{
 			name: "no file operations",
 			fields: fields{
+				TypeMeta:   metav1.TypeMeta{APIVersion: currentAPIVersion},
 				ObjectMeta: metav1.ObjectMeta{Name: "foo"},
 				Spec: PluginSpec{
 					Version:          "",
@@ -149,14 +191,13 @@ func TestPlugin_Validate(t *testing.T) {
 					}},
 				},
 			},
-			args: args{
-				name: "foo",
-			},
-			wantErr: true,
+			pluginName: "foo",
+			wantErr:    true,
 		},
 		{
 			name: "wrong plugin name",
 			fields: fields{
+				TypeMeta:   metav1.TypeMeta{APIVersion: currentAPIVersion},
 				ObjectMeta: metav1.ObjectMeta{Name: "wrong-name"},
 				Spec: PluginSpec{
 					Version:          "",
@@ -173,14 +214,13 @@ func TestPlugin_Validate(t *testing.T) {
 					}},
 				},
 			},
-			args: args{
-				name: "foo",
-			},
-			wantErr: true,
+			pluginName: "foo",
+			wantErr:    true,
 		},
 		{
 			name: "unsafe plugin name",
 			fields: fields{
+				TypeMeta:   metav1.TypeMeta{APIVersion: currentAPIVersion},
 				ObjectMeta: metav1.ObjectMeta{Name: "../foo"},
 				Spec: PluginSpec{
 					Version:          "",
@@ -197,10 +237,8 @@ func TestPlugin_Validate(t *testing.T) {
 					}},
 				},
 			},
-			args: args{
-				name: "../foo",
-			},
-			wantErr: true,
+			pluginName: "../foo",
+			wantErr:    true,
 		},
 	}
 	for _, tt := range tests {
@@ -210,8 +248,8 @@ func TestPlugin_Validate(t *testing.T) {
 				ObjectMeta: tt.fields.ObjectMeta,
 				Spec:       tt.fields.Spec,
 			}
-			if err := p.Validate(tt.args.name); (err != nil) != tt.wantErr {
-				t.Errorf("Plugin.Validate() error = %v, wantErr %v", err, tt.wantErr)
+			if err := p.Validate(tt.pluginName); (err != nil) != tt.wantErr {
+				t.Errorf("Plugin.Validate(%s) error = %v, wantErr %v", tt.pluginName, err, tt.wantErr)
 			}
 		})
 	}