Repository: GoogleContainerTools/krew
Pull request #36 (2018-09-11 17:10:35)

README: add breaking change notice


Patch:
diff --git a/.travis.yml b/.travis.yml
index 6e86357..417ccb5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -23,6 +23,7 @@ before_install:
 script:
   - hack/gofmt.sh
   - hack/gometalinter.sh
+  - go test -v -cover ./...
   - hack/build-cross-releases.sh
 
 deploy:
diff --git a/README.md b/README.md
index 84daf9e..1f242d3 100644
--- a/README.md
+++ b/README.md
@@ -11,6 +11,12 @@ similar to tools like apt, dnf or [brew](http://brew.sh).
 
 ### Installation
 
+> :warning: **Warning** :warning: **Kubectl v1.12 completely changes the plugin
+> model in a breaking way and it's not compatible with krew yet**
+> ([#33](https://github.com/GoogleContainerTools/krew/issues/33)). Therefore,
+> krew will make breaking changes in v0.2. If you're installing krew v0.1.0 to
+> try out, make sure you have kubectl v1.11.
+
 For macOS and Linux:
 
 - Make sure that git is installed.
diff --git a/cmd/krew/cmd/root.go b/cmd/krew/cmd/root.go
index 6c85c0e..43fe8be 100644
--- a/cmd/krew/cmd/root.go
+++ b/cmd/krew/cmd/root.go
@@ -62,8 +62,8 @@ func init() {
 	// Required by glog
 	flag.Parse()
 
-	paths = environment.MustGetKrewPathsFromEnvs(os.Environ())
-	if err := ensureDirs(paths.Base, paths.Download, paths.Install); err != nil {
+	paths = environment.MustGetKrewPaths()
+	if err := ensureDirs(paths.Base, paths.Download, paths.Install, paths.Bin); err != nil {
 		glog.Fatal(err)
 	}
 
diff --git a/pkg/environment/environment.go b/pkg/environment/environment.go
index 0a36418..ec79e25 100644
--- a/pkg/environment/environment.go
+++ b/pkg/environment/environment.go
@@ -20,11 +20,12 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/GoogleContainerTools/krew/pkg/pathutil"
 	"k8s.io/client-go/util/homedir"
+
+	"github.com/GoogleContainerTools/krew/pkg/pathutil"
 )
 
-// KrewPaths contains all important enviroment paths
+// KrewPaths contains all important environment paths
 type KrewPaths struct {
 	// Base is the path of the krew root.
 	// The default path is ~/.kube/plugins/krew
@@ -41,6 +42,10 @@ type KrewPaths struct {
 	// Download is a directory where plugins will be temporarily downloaded to.
 	// This is currently a path in os.TempDir()
 	Download string
+
+	// Bin is the path krew links all plugin binaries to.
+	// This path has to be added to the $PATH.
+	Bin string
 }
 
 func parseEnvs(environ []string) map[string]string {
@@ -52,9 +57,9 @@ func parseEnvs(environ []string) map[string]string {
 	return flags
 }
 
-// MustGetKrewPathsFromEnvs returns ensured index paths for krew.
-func MustGetKrewPathsFromEnvs(envs []string) KrewPaths {
-	base := filepath.Join(getKubectlPluginsPath(envs), "krew")
+// MustGetKrewPaths returns ensured index paths for krew.
+func MustGetKrewPaths() KrewPaths {
+	base := filepath.Join(homedir.HomeDir(), ".kube", "plugins", "krew")
 	base, err := filepath.Abs(base)
 	if err != nil {
 		panic(fmt.Errorf("cannot get current pwd err: %v", err))
@@ -65,33 +70,13 @@ func MustGetKrewPathsFromEnvs(envs []string) KrewPaths {
 		Index:    filepath.Join(base, "index"),
 		Install:  filepath.Join(base, "store"),
 		Download: filepath.Join(os.TempDir(), "krew"),
+		Bin:      filepath.Join(base, "bin"),
 	}
 }
 
-func getKubectlPluginsPath(envs []string) string {
-	envvars := parseEnvs(envs)
-
-	// Look for "${KUBECTL_PLUGINS_PATH}"
-	if path, ok := envvars["KUBECTL_PLUGINS_PATH"]; ok && path != "" {
-		return path
-	}
-
-	// TODO(lbb): make os call testable! (dep injection)
-	// Look for "${XDG_DATA_DIRS}/kubectl/plugins"
-	if path := envvars["XDG_DATA_DIRS"]; path != "" {
-		fullPath := filepath.Join(path, "kubectl", "plugins")
-		stat, err := os.Stat(fullPath)
-		if err == nil && stat.IsDir() {
-			return fullPath
-		}
-	}
-
-	// Golang does no '~' expansion
-	return filepath.Join(homedir.HomeDir(), ".kube", "plugins")
-}
-
 // GetExecutedVersion returns the currently executed version. If krew is
 // not executed as an plugin it will return a nil error and an empty string.
+// TODO(lbb): Breaks, refactor.
 func GetExecutedVersion(paths KrewPaths, cmdArgs []string) (string, bool, error) {
 	currentBinaryPath, err := filepath.Abs(cmdArgs[0])
 	if err != nil {
diff --git a/pkg/index/types.go b/pkg/index/types.go
index dc51ecd..9c4c9f6 100644
--- a/pkg/index/types.go
+++ b/pkg/index/types.go
@@ -45,6 +45,11 @@ type Platform struct {
 
 	Selector *metav1.LabelSelector `json:"selector,omitempty"`
 	Files    []FileOperation       `json:"files"`
+
+	// Bin specifies the path to the plugin executable.
+	// The path is relative to the root of the installation folder.
+	// The binary will be linked after all FileOperations are executed.
+	Bin string `json:"bin"`
 }
 
 // FileOperation TODO(lbb)
diff --git a/pkg/index/validate.go b/pkg/index/validate.go
index 35c0287..7381299 100644
--- a/pkg/index/validate.go
+++ b/pkg/index/validate.go
@@ -69,6 +69,12 @@ func (p Platform) Validate() error {
 	if p.Head == "" && p.URI == "" {
 		return fmt.Errorf("head or URI have to be set")
 	}
+	if p.Bin == "" {
+		return fmt.Errorf("bin has to be set")
+	}
+	if !strings.HasPrefix(p.Bin, "kubectl-") {
+		return fmt.Errorf("the plugin executeable needs to have the 'kubectl-' prefix")
+	}
 	if len(p.Files) == 0 {
 		return fmt.Errorf("can't have a plugin without specifying file operations")
 	}
diff --git a/pkg/installation/move.go b/pkg/installation/move.go
index 2da095d..8c302bc 100644
--- a/pkg/installation/move.go
+++ b/pkg/installation/move.go
@@ -41,11 +41,11 @@ func findMoveTargets(fromDir, toDir string, fo index.FileOperation) ([]move, err
 		return nil, fmt.Errorf("could not get the relative path for the move src, err: %v", err)
 	}
 
-	glog.V(4).Infoln("Trying to move single file directly from=%q to=%q with file operation=%+v", fromDir, toDir, fo)
+	glog.V(4).Infof("Trying to move single file directly from=%q to=%q with file operation=%+v", fromDir, toDir, fo)
 	if m, ok, err := getDirectMove(fromDir, toDir, fo); err != nil {
 		return nil, fmt.Errorf("failed to detect single move operation, err: %v", err)
 	} else if ok {
-		glog.V(3).Infof("Detected single move from file operation", fo)
+		glog.V(3).Infof("Detected single move from file operation=%+v", fo)
 		return []move{m}, nil
 	}
 


Test patch:
diff --git a/pkg/environment/environment_test.go b/pkg/environment/environment_test.go
index ea34689..454aa83 100644
--- a/pkg/environment/environment_test.go
+++ b/pkg/environment/environment_test.go
@@ -18,8 +18,6 @@ import (
 	"path/filepath"
 	"reflect"
 	"testing"
-
-	"k8s.io/client-go/util/homedir"
 )
 
 func Test_parseEnvs(t *testing.T) {
@@ -67,47 +65,6 @@ func Test_parseEnvs(t *testing.T) {
 	}
 }
 
-func Test_getKubectlPluginsPath(t *testing.T) {
-	type args struct {
-		envs []string
-	}
-	tests := []struct {
-		name string
-		args args
-		want string
-	}{
-		{
-			name: "default path",
-			args: args{
-				envs: []string{"HOME=~"},
-			},
-			want: filepath.Join(homedir.HomeDir(), ".kube", "plugins"),
-		},
-		{
-			name: "manual plugin path",
-			args: args{
-				envs: []string{"KUBECTL_PLUGINS_PATH=/foobar", "HOME=~"},
-			},
-			want: "/foobar",
-		},
-		{
-			name: "no further xdg",
-			args: args{
-				envs: []string{"XDG_DATA_DIRS=/", "HOME=~"},
-			},
-			want: filepath.Join(homedir.HomeDir(), ".kube", "plugins"),
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := getKubectlPluginsPath(tt.args.envs); got != tt.want {
-				t.Errorf("getKubectlPluginsPath() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func TestIsPlugin(t *testing.T) {
 	type args struct {
 		environ []string
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
index 83256a3..9c67846 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/bar.yaml
@@ -21,8 +21,10 @@ spec:
   - files:
     - from: "*"
     head: https://example.com
+    bin: kubectl-bar
   - files:
     - from: "*"
     uri: https://example.com
     sha256: alsoSet
+    bin: kubectl-bar
   shortDescription: "exists"
diff --git a/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml b/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml
index a8431f6..ec25b88 100644
--- a/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml
+++ b/pkg/index/indexscanner/testdata/testindex/plugins/foo.yaml
@@ -21,12 +21,14 @@ spec:
   - files:
     - from: "*"
     head: https://example.com
+    bin: kubectl-foo
     selector:
       matchExpressions:
       - {key: os, operator: In, values: [macos, linux]}
   - files:
     - from: "*"
     head: https://example.com
+    bin: kubectl-foo
     selector:
       matchLabels:
         os: "windows"
diff --git a/pkg/index/validate_test.go b/pkg/index/validate_test.go
index 570a83f..ab7911b 100644
--- a/pkg/index/validate_test.go
+++ b/pkg/index/validate_test.go
@@ -97,6 +97,7 @@ func TestPlugin_Validate(t *testing.T) {
 						Sha256:   "",
 						Selector: nil,
 						Files:    []FileOperation{{"", ""}},
+						Bin:      "kubectl-foo",
 					}},
 				},
 			},
@@ -120,6 +121,7 @@ func TestPlugin_Validate(t *testing.T) {
 						Sha256:   "",
 						Selector: nil,
 						Files:    []FileOperation{{"", ""}},
+						Bin:      "kubectl-foo",
 					}},
 				},
 			},
@@ -143,6 +145,7 @@ func TestPlugin_Validate(t *testing.T) {
 						Sha256:   "",
 						Selector: nil,
 						Files:    []FileOperation{},
+						Bin:      "kubectl-foo",
 					}},
 				},
 			},
@@ -166,6 +169,7 @@ func TestPlugin_Validate(t *testing.T) {
 						Sha256:   "",
 						Selector: nil,
 						Files:    []FileOperation{{"", ""}},
+						Bin:      "kubectl-foo",
 					}},
 				},
 			},
@@ -189,6 +193,7 @@ func TestPlugin_Validate(t *testing.T) {
 						Sha256:   "",
 						Selector: nil,
 						Files:    []FileOperation{{"", ""}},
+						Bin:      "kubectl-foo",
 					}},
 				},
 			},
@@ -219,6 +224,7 @@ func TestPlatform_Validate(t *testing.T) {
 		Sha256   string
 		Selector *metav1.LabelSelector
 		Files    []FileOperation
+		Bin      string
 	}
 	tests := []struct {
 		name    string
@@ -233,6 +239,7 @@ func TestPlatform_Validate(t *testing.T) {
 				Sha256:   "",
 				Selector: nil,
 				Files:    []FileOperation{{"", ""}},
+				Bin:      "kubectl-foo",
 			},
 			wantErr: false,
 		},
@@ -244,6 +251,7 @@ func TestPlatform_Validate(t *testing.T) {
 				Sha256:   "",
 				Selector: nil,
 				Files:    []FileOperation{{"", ""}},
+				Bin:      "kubectl-foo",
 			},
 			wantErr: true,
 		},
@@ -255,6 +263,7 @@ func TestPlatform_Validate(t *testing.T) {
 				Sha256:   "deadbeef",
 				Selector: nil,
 				Files:    []FileOperation{{"", ""}},
+				Bin:      "kubectl-foo",
 			},
 			wantErr: true,
 		},
@@ -266,6 +275,7 @@ func TestPlatform_Validate(t *testing.T) {
 				Sha256:   "",
 				Selector: nil,
 				Files:    []FileOperation{{"", ""}},
+				Bin:      "kubectl-foo",
 			},
 			wantErr: true,
 		},
@@ -277,6 +287,31 @@ func TestPlatform_Validate(t *testing.T) {
 				Sha256:   "",
 				Selector: nil,
 				Files:    []FileOperation{},
+				Bin:      "kubectl-foo",
+			},
+			wantErr: true,
+		},
+		{
+			name: "no bin",
+			fields: fields{
+				Head:     "http://example.com",
+				URI:      "",
+				Sha256:   "",
+				Selector: nil,
+				Files:    []FileOperation{{"", ""}},
+				Bin:      "",
+			},
+			wantErr: true,
+		},
+		{
+			name: "wrong bin prefix",
+			fields: fields{
+				Head:     "http://example.com",
+				URI:      "",
+				Sha256:   "",
+				Selector: nil,
+				Files:    []FileOperation{{"", ""}},
+				Bin:      "foo",
 			},
 			wantErr: true,
 		},
@@ -289,6 +324,7 @@ func TestPlatform_Validate(t *testing.T) {
 				Sha256:   tt.fields.Sha256,
 				Selector: tt.fields.Selector,
 				Files:    tt.fields.Files,
+				Bin:      tt.fields.Bin,
 			}
 			if err := p.Validate(); (err != nil) != tt.wantErr {
 				t.Errorf("Platform.Validate() error = %v, wantErr %v", err, tt.wantErr)