diff --git a/pkg/kubectl/cmd/plugin/BUILD b/pkg/kubectl/cmd/plugin/BUILD index fb6091de39..5d9b2cd2b1 100644 --- a/pkg/kubectl/cmd/plugin/BUILD +++ b/pkg/kubectl/cmd/plugin/BUILD @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -33,3 +33,11 @@ filegroup( data = glob(["testdata/**"]), visibility = ["//visibility:public"], ) + +go_test( + name = "go_default_test", + srcs = ["plugin_test.go"], + data = glob(["testdata/**"]), + embed = [":go_default_library"], + deps = ["//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library"], +) diff --git a/pkg/kubectl/cmd/plugin/plugin.go b/pkg/kubectl/cmd/plugin/plugin.go index 7196804caf..d7df928098 100644 --- a/pkg/kubectl/cmd/plugin/plugin.go +++ b/pkg/kubectl/cmd/plugin/plugin.go @@ -69,6 +69,8 @@ type PluginListOptions struct { Verifier PathVerifier NameOnly bool + PluginPaths []string + genericclioptions.IOStreams } @@ -97,20 +99,22 @@ func (o *PluginListOptions) Complete(cmd *cobra.Command) error { root: cmd.Root(), seenPlugins: make(map[string]string, 0), } - return nil -} -func (o *PluginListOptions) Run() error { path := "PATH" if runtime.GOOS == "windows" { path = "path" } + o.PluginPaths = filepath.SplitList(os.Getenv(path)) + return nil +} +func (o *PluginListOptions) Run() error { pluginsFound := false isFirstFile := true pluginErrors := []error{} pluginWarnings := 0 - for _, dir := range filepath.SplitList(os.Getenv(path)) { + + for _, dir := range uniquePathsList(o.PluginPaths) { files, err := ioutil.ReadDir(dir) if err != nil { pluginErrors = append(pluginErrors, fmt.Errorf("error: unable to read directory %q in your PATH: %v", dir, err)) @@ -238,3 +242,18 @@ func isExecutable(fullPath string) (bool, error) { return false, nil } + +// uniquePathsList deduplicates a given slice of strings without +// sorting or otherwise altering its order in any way. +func uniquePathsList(paths []string) []string { + seen := map[string]bool{} + newPaths := []string{} + for _, p := range paths { + if seen[p] { + continue + } + seen[p] = true + newPaths = append(newPaths, p) + } + return newPaths +} diff --git a/pkg/kubectl/cmd/plugin/plugin_test.go b/pkg/kubectl/cmd/plugin/plugin_test.go new file mode 100644 index 0000000000..5f6c0d5b72 --- /dev/null +++ b/pkg/kubectl/cmd/plugin/plugin_test.go @@ -0,0 +1,197 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + "bytes" + "fmt" + "io/ioutil" + "os" + "strings" + "testing" + + "k8s.io/cli-runtime/pkg/genericclioptions" +) + +func TestPluginPathsAreUnaltered(t *testing.T) { + tempDir, err := ioutil.TempDir(os.TempDir(), "test-cmd-plugins") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + tempDir2, err := ioutil.TempDir(os.TempDir(), "test-cmd-plugins2") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // cleanup + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + panic(fmt.Errorf("unexpected cleanup error: %v", err)) + } + if err := os.RemoveAll(tempDir2); err != nil { + panic(fmt.Errorf("unexpected cleanup error: %v", err)) + } + }() + + ioStreams, _, _, errOut := genericclioptions.NewTestIOStreams() + verifier := newFakePluginPathVerifier() + pluginPaths := []string{tempDir, tempDir2} + o := &PluginListOptions{ + Verifier: verifier, + IOStreams: ioStreams, + + PluginPaths: pluginPaths, + } + + // write at least one valid plugin file + if _, err := ioutil.TempFile(tempDir, "kubectl-"); err != nil { + t.Fatalf("unexpected error %v", err) + } + if _, err := ioutil.TempFile(tempDir2, "kubectl-"); err != nil { + t.Fatalf("unexpected error %v", err) + } + + if err := o.Run(); err != nil { + t.Fatalf("unexpected error %v - %v", err, errOut.String()) + } + + // ensure original paths remain unaltered + if len(verifier.seenUnsorted) != len(pluginPaths) { + t.Fatalf("saw unexpected plugin paths. Expecting %v, got %v", pluginPaths, verifier.seenUnsorted) + } + for actual := range verifier.seenUnsorted { + if !strings.HasPrefix(verifier.seenUnsorted[actual], pluginPaths[actual]) { + t.Fatalf("expected PATH slice to be unaltered. Expecting %v, but got %v", pluginPaths[actual], verifier.seenUnsorted[actual]) + } + } +} + +func TestPluginPathsAreValid(t *testing.T) { + tempDir, err := ioutil.TempDir(os.TempDir(), "test-cmd-plugins") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // cleanup + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + panic(fmt.Errorf("unexpected cleanup error: %v", err)) + } + }() + + tc := []struct { + name string + pluginPaths []string + pluginFile func() (*os.File, error) + verifier *fakePluginPathVerifier + expectVerifyErrors []error + expectErr string + }{ + { + name: "ensure no plugins found if no files begin with kubectl- prefix", + pluginPaths: []string{tempDir}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { + return ioutil.TempFile(tempDir, "notkubectl-") + }, + expectErr: "unable to find any kubectl plugins in your PATH", + }, + { + name: "ensure de-duplicated plugin-paths slice", + pluginPaths: []string{tempDir, tempDir}, + verifier: newFakePluginPathVerifier(), + pluginFile: func() (*os.File, error) { + return ioutil.TempFile(tempDir, "kubectl-") + }, + }, + } + + for _, test := range tc { + t.Run(test.name, func(t *testing.T) { + ioStreams, _, _, errOut := genericclioptions.NewTestIOStreams() + o := &PluginListOptions{ + Verifier: test.verifier, + IOStreams: ioStreams, + + PluginPaths: test.pluginPaths, + } + + // create files + if test.pluginFile != nil { + if _, err := test.pluginFile(); err != nil { + t.Fatalf("unexpected error creating plugin file: %v", err) + } + } + + for _, expected := range test.expectVerifyErrors { + for _, actual := range test.verifier.errors { + if expected != actual { + t.Fatalf("unexpected error: expected %v, but got %v", expected, actual) + } + } + } + + err := o.Run() + if err == nil && len(test.expectErr) > 0 { + t.Fatalf("unexpected non-error: expecting %v", test.expectErr) + } + if err != nil && len(test.expectErr) == 0 { + t.Fatalf("unexpected error: %v - %v", err, errOut.String()) + } + if err == nil { + return + } + + allErrs := bytes.NewBuffer(errOut.Bytes()) + if _, err := allErrs.WriteString(err.Error()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(test.expectErr) > 0 { + if !strings.Contains(allErrs.String(), test.expectErr) { + t.Fatalf("unexpected error: expected %v, but got %v", test.expectErr, allErrs.String()) + } + } + }) + } +} + +type duplicatePathError struct { + path string +} + +func (d *duplicatePathError) Error() string { + return fmt.Sprintf("path %q already visited", d.path) +} + +type fakePluginPathVerifier struct { + errors []error + seen map[string]bool + seenUnsorted []string +} + +func (f *fakePluginPathVerifier) Verify(path string) []error { + if f.seen[path] { + err := &duplicatePathError{path} + f.errors = append(f.errors, err) + return []error{err} + } + f.seen[path] = true + f.seenUnsorted = append(f.seenUnsorted, path) + return nil +} + +func newFakePluginPathVerifier() *fakePluginPathVerifier { + return &fakePluginPathVerifier{seen: make(map[string]bool)} +}