diff --git a/cmd/helm/load_plugins.go b/cmd/helm/load_plugins.go index e56feab40..a23a067fb 100644 --- a/cmd/helm/load_plugins.go +++ b/cmd/helm/load_plugins.go @@ -58,7 +58,7 @@ func loadPlugins(baseCmd *cobra.Command, out io.Writer) { return } - found, err := findPlugins(settings.PluginsDirectory) + found, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { fmt.Fprintf(os.Stderr, "failed to load plugins: %s", err) return @@ -238,20 +238,6 @@ func manuallyProcessArgs(args []string) ([]string, []string) { return known, unknown } -// findPlugins returns a list of YAML files that describe plugins. -func findPlugins(plugdirs string) ([]*plugin.Plugin, error) { - found := []*plugin.Plugin{} - // Let's get all UNIXy and allow path separators - for _, p := range filepath.SplitList(plugdirs) { - matches, err := plugin.LoadAll(p) - if err != nil { - return matches, err - } - found = append(found, matches...) - } - return found, nil -} - // pluginCommand represents the optional completion.yaml file of a plugin type pluginCommand struct { Name string `json:"name"` diff --git a/cmd/helm/plugin_list.go b/cmd/helm/plugin_list.go index 0440b0b5e..49a454963 100644 --- a/cmd/helm/plugin_list.go +++ b/cmd/helm/plugin_list.go @@ -22,6 +22,8 @@ import ( "github.com/gosuri/uitable" "github.com/spf13/cobra" + + "helm.sh/helm/v3/pkg/plugin" ) func newPluginListCmd(out io.Writer) *cobra.Command { @@ -31,7 +33,7 @@ func newPluginListCmd(out io.Writer) *cobra.Command { Short: "list installed Helm plugins", RunE: func(cmd *cobra.Command, args []string) error { debug("pluginDirs: %s", settings.PluginsDirectory) - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { return err } @@ -51,7 +53,7 @@ func newPluginListCmd(out io.Writer) *cobra.Command { // Provide dynamic auto-completion for plugin names func compListPlugins(toComplete string) []string { var pNames []string - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err == nil { for _, p := range plugins { if strings.HasPrefix(p.Metadata.Name, toComplete) { diff --git a/cmd/helm/plugin_uninstall.go b/cmd/helm/plugin_uninstall.go index f703ddcfb..66cdaccdc 100644 --- a/cmd/helm/plugin_uninstall.go +++ b/cmd/helm/plugin_uninstall.go @@ -68,7 +68,7 @@ func (o *pluginUninstallOptions) complete(args []string) error { func (o *pluginUninstallOptions) run(out io.Writer) error { debug("loading installed plugins from %s", settings.PluginsDirectory) - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { return err } diff --git a/cmd/helm/plugin_update.go b/cmd/helm/plugin_update.go index a24e80518..23f840b4a 100644 --- a/cmd/helm/plugin_update.go +++ b/cmd/helm/plugin_update.go @@ -70,7 +70,7 @@ func (o *pluginUpdateOptions) complete(args []string) error { func (o *pluginUpdateOptions) run(out io.Writer) error { installer.Debug = settings.Debug debug("loading installed plugins from %s", settings.PluginsDirectory) - plugins, err := findPlugins(settings.PluginsDirectory) + plugins, err := plugin.FindPlugins(settings.PluginsDirectory) if err != nil { return err } diff --git a/pkg/plugin/installer/base.go b/pkg/plugin/installer/base.go index a8ec97416..dcc3ad644 100644 --- a/pkg/plugin/installer/base.go +++ b/pkg/plugin/installer/base.go @@ -16,7 +16,6 @@ limitations under the License. package installer // import "helm.sh/helm/v3/pkg/plugin/installer" import ( - "os" "path/filepath" "helm.sh/helm/v3/pkg/helmpath" @@ -31,13 +30,7 @@ func newBase(source string) base { return base{source} } -// link creates a symlink from the plugin source to the base path. -func (b *base) link(from string) error { - debug("symlinking %s to %s", from, b.Path()) - return os.Symlink(from, b.Path()) -} - -// Path is where the plugin will be symlinked to. +// Path is where the plugin will be installed. func (b *base) Path() string { if b.Source == "" { return "" diff --git a/pkg/plugin/installer/http_installer.go b/pkg/plugin/installer/http_installer.go index ea4ac7bcd..629bbec39 100644 --- a/pkg/plugin/installer/http_installer.go +++ b/pkg/plugin/installer/http_installer.go @@ -27,6 +27,7 @@ import ( "github.com/pkg/errors" + "helm.sh/helm/v3/internal/third_party/dep/fs" "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/helmpath" @@ -68,7 +69,6 @@ func NewExtractor(source string) (Extractor, error) { // NewHTTPInstaller creates a new HttpInstaller. func NewHTTPInstaller(source string) (*HTTPInstaller, error) { - key, err := cache.Key(source) if err != nil { return nil, err @@ -108,18 +108,16 @@ func stripPluginName(name string) string { } // Install downloads and extracts the tarball into the cache directory -// and creates a symlink to the plugin directory. +// and installs into the plugin directory. // // Implements Installer. func (i *HTTPInstaller) Install() error { - pluginData, err := i.getter.Get(i.Source) if err != nil { return err } - err = i.extractor.Extract(pluginData, i.CacheDir) - if err != nil { + if err := i.extractor.Extract(pluginData, i.CacheDir); err != nil { return err } @@ -132,7 +130,8 @@ func (i *HTTPInstaller) Install() error { return err } - return i.link(src) + debug("copying %s to %s", src, i.Path()) + return fs.CopyDir(src, i.Path()) } // Update updates a local repository @@ -141,12 +140,6 @@ func (i *HTTPInstaller) Update() error { return errors.Errorf("method Update() not implemented for HttpInstaller") } -// Override link because we want to use HttpInstaller.Path() not base.Path() -func (i *HTTPInstaller) link(from string) error { - debug("symlinking %s to %s", from, i.Path()) - return os.Symlink(from, i.Path()) -} - // Path is overridden because we want to join on the plugin name not the file name func (i HTTPInstaller) Path() string { if i.base.Source == "" { @@ -164,17 +157,16 @@ func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { return err } - tarReader := tar.NewReader(uncompressedStream) - - os.MkdirAll(targetDir, 0755) + if err := os.MkdirAll(targetDir, 0755); err != nil { + return err + } + tarReader := tar.NewReader(uncompressedStream) for { header, err := tarReader.Next() - if err == io.EOF { break } - if err != nil { return err } @@ -200,7 +192,5 @@ func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { return errors.Errorf("unknown type: %b in %s", header.Typeflag, header.Name) } } - return nil - } diff --git a/pkg/plugin/installer/http_installer_test.go b/pkg/plugin/installer/http_installer_test.go index cfa2e4cbe..b496a1b01 100644 --- a/pkg/plugin/installer/http_installer_test.go +++ b/pkg/plugin/installer/http_installer_test.go @@ -73,13 +73,13 @@ func TestHTTPInstaller(t *testing.T) { i, err := NewForSource(source, "0.0.1") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } // ensure a HTTPInstaller was returned httpInstaller, ok := i.(*HTTPInstaller) if !ok { - t.Error("expected a HTTPInstaller") + t.Fatal("expected a HTTPInstaller") } // inject fake http client responding with minimal plugin tarball @@ -94,17 +94,17 @@ func TestHTTPInstaller(t *testing.T) { // install the plugin if err := Install(i); err != nil { - t.Error(err) + t.Fatal(err) } if i.Path() != helmpath.DataPath("plugins", "fake-plugin") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) } // Install again to test plugin exists error if err := Install(i); err == nil { - t.Error("expected error for plugin exists, got none") + t.Fatal("expected error for plugin exists, got none") } else if err.Error() != "plugin already exists" { - t.Errorf("expected error for plugin exists, got (%v)", err) + t.Fatalf("expected error for plugin exists, got (%v)", err) } } @@ -119,13 +119,13 @@ func TestHTTPInstallerNonExistentVersion(t *testing.T) { i, err := NewForSource(source, "0.0.2") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } // ensure a HTTPInstaller was returned httpInstaller, ok := i.(*HTTPInstaller) if !ok { - t.Error("expected a HTTPInstaller") + t.Fatal("expected a HTTPInstaller") } // inject fake http client responding with error @@ -135,7 +135,7 @@ func TestHTTPInstallerNonExistentVersion(t *testing.T) { // attempt to install the plugin if err := Install(i); err == nil { - t.Error("expected error from http client") + t.Fatal("expected error from http client") } } @@ -150,13 +150,13 @@ func TestHTTPInstallerUpdate(t *testing.T) { i, err := NewForSource(source, "0.0.1") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } // ensure a HTTPInstaller was returned httpInstaller, ok := i.(*HTTPInstaller) if !ok { - t.Error("expected a HTTPInstaller") + t.Fatal("expected a HTTPInstaller") } // inject fake http client responding with minimal plugin tarball @@ -171,15 +171,15 @@ func TestHTTPInstallerUpdate(t *testing.T) { // install the plugin before updating if err := Install(i); err != nil { - t.Error(err) + t.Fatal(err) } if i.Path() != helmpath.DataPath("plugins", "fake-plugin") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/fake-plugin', got %q", i.Path()) } // Update plugin, should fail because it is not implemented if err := Update(i); err == nil { - t.Error("update method not implemented for http installer") + t.Fatal("update method not implemented for http installer") } } @@ -240,29 +240,27 @@ func TestExtract(t *testing.T) { } if err = extractor.Extract(&buf, tempDir); err != nil { - t.Errorf("Did not expect error but got error: %v", err) + t.Fatalf("Did not expect error but got error: %v", err) } pluginYAMLFullPath := filepath.Join(tempDir, "plugin.yaml") if info, err := os.Stat(pluginYAMLFullPath); err != nil { if os.IsNotExist(err) { - t.Errorf("Expected %s to exist but doesn't", pluginYAMLFullPath) - } else { - t.Error(err) + t.Fatalf("Expected %s to exist but doesn't", pluginYAMLFullPath) } + t.Fatal(err) } else if info.Mode().Perm() != 0600 { - t.Errorf("Expected %s to have 0600 mode it but has %o", pluginYAMLFullPath, info.Mode().Perm()) + t.Fatalf("Expected %s to have 0600 mode it but has %o", pluginYAMLFullPath, info.Mode().Perm()) } readmeFullPath := filepath.Join(tempDir, "README.md") if info, err := os.Stat(readmeFullPath); err != nil { if os.IsNotExist(err) { - t.Errorf("Expected %s to exist but doesn't", readmeFullPath) - } else { - t.Error(err) + t.Fatalf("Expected %s to exist but doesn't", readmeFullPath) } + t.Fatal(err) } else if info.Mode().Perm() != 0777 { - t.Errorf("Expected %s to have 0777 mode it but has %o", readmeFullPath, info.Mode().Perm()) + t.Fatalf("Expected %s to have 0777 mode it but has %o", readmeFullPath, info.Mode().Perm()) } } diff --git a/pkg/plugin/installer/installer.go b/pkg/plugin/installer/installer.go index 14a02a87e..65c61cd7d 100644 --- a/pkg/plugin/installer/installer.go +++ b/pkg/plugin/installer/installer.go @@ -17,6 +17,7 @@ package installer import ( "fmt" + "log" "os" "path/filepath" "strings" @@ -103,9 +104,10 @@ func isPlugin(dirname string) bool { return err == nil } +var logger = log.New(os.Stderr, "[debug] ", log.Lshortfile) + func debug(format string, args ...interface{}) { if Debug { - format = fmt.Sprintf("[debug] %s\n", format) - fmt.Printf(format, args...) + logger.Output(2, fmt.Sprintf(format, args...)) } } diff --git a/pkg/plugin/installer/local_installer.go b/pkg/plugin/installer/local_installer.go index 662ef5b29..c92bc3fb0 100644 --- a/pkg/plugin/installer/local_installer.go +++ b/pkg/plugin/installer/local_installer.go @@ -16,6 +16,7 @@ limitations under the License. package installer // import "helm.sh/helm/v3/pkg/plugin/installer" import ( + "os" "path/filepath" "github.com/pkg/errors" @@ -45,7 +46,8 @@ func (i *LocalInstaller) Install() error { if !isPlugin(i.Source) { return ErrMissingMetadata } - return i.link(i.Source) + debug("symlinking %s to %s", i.Source, i.Path()) + return os.Symlink(i.Source, i.Path()) } // Update updates a local repository diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go index d11e44860..3d9607331 100644 --- a/pkg/plugin/installer/local_installer_test.go +++ b/pkg/plugin/installer/local_installer_test.go @@ -40,7 +40,7 @@ func TestLocalInstaller(t *testing.T) { source := "../testdata/plugdir/echo" i, err := NewForSource(source, "") if err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } if err := Install(i); err != nil { @@ -48,6 +48,6 @@ func TestLocalInstaller(t *testing.T) { } if i.Path() != helmpath.DataPath("plugins", "echo") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) } } diff --git a/pkg/plugin/installer/vcs_installer.go b/pkg/plugin/installer/vcs_installer.go index 1a5d74cca..f7df5b322 100644 --- a/pkg/plugin/installer/vcs_installer.go +++ b/pkg/plugin/installer/vcs_installer.go @@ -23,6 +23,7 @@ import ( "github.com/Masterminds/vcs" "github.com/pkg/errors" + "helm.sh/helm/v3/internal/third_party/dep/fs" "helm.sh/helm/v3/pkg/helmpath" "helm.sh/helm/v3/pkg/plugin/cache" ) @@ -43,7 +44,7 @@ func existingVCSRepo(location string) (Installer, error) { Repo: repo, base: newBase(repo.Remote()), } - return i, err + return i, nil } // NewVCSInstaller creates a new VCSInstaller. @@ -65,7 +66,7 @@ func NewVCSInstaller(source, version string) (*VCSInstaller, error) { return i, err } -// Install clones a remote repository and creates a symlink to the plugin directory. +// Install clones a remote repository and installs into the plugin directory. // // Implements Installer. func (i *VCSInstaller) Install() error { @@ -87,7 +88,8 @@ func (i *VCSInstaller) Install() error { return ErrMissingMetadata } - return i.link(i.Repo.LocalPath()) + debug("copying %s to %s", i.Repo.LocalPath(), i.Path()) + return fs.CopyDir(i.Repo.LocalPath(), i.Path()) } // Update updates a remote repository diff --git a/pkg/plugin/installer/vcs_installer_test.go b/pkg/plugin/installer/vcs_installer_test.go index ce1ee609e..b8dc6b1e2 100644 --- a/pkg/plugin/installer/vcs_installer_test.go +++ b/pkg/plugin/installer/vcs_installer_test.go @@ -80,24 +80,24 @@ func TestVCSInstaller(t *testing.T) { t.Fatal(err) } if repo.current != "0.1.1" { - t.Errorf("expected version '0.1.1', got %q", repo.current) + t.Fatalf("expected version '0.1.1', got %q", repo.current) } if i.Path() != helmpath.DataPath("plugins", "helm-env") { - t.Errorf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) + t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) } // Install again to test plugin exists error if err := Install(i); err == nil { - t.Error("expected error for plugin exists, got none") + t.Fatalf("expected error for plugin exists, got none") } else if err.Error() != "plugin already exists" { - t.Errorf("expected error for plugin exists, got (%v)", err) + t.Fatalf("expected error for plugin exists, got (%v)", err) } // Testing FindSource method, expect error because plugin code is not a cloned repository if _, err := FindSource(i.Path()); err == nil { - t.Error("expected error for inability to find plugin source, got none") + t.Fatalf("expected error for inability to find plugin source, got none") } else if err.Error() != "cannot get information about plugin source" { - t.Errorf("expected error for inability to find plugin source, got (%v)", err) + t.Fatalf("expected error for inability to find plugin source, got (%v)", err) } } @@ -113,15 +113,14 @@ func TestVCSInstallerNonExistentVersion(t *testing.T) { } // ensure a VCSInstaller was returned - _, ok := i.(*VCSInstaller) - if !ok { + if _, ok := i.(*VCSInstaller); !ok { t.Fatal("expected a VCSInstaller") } if err := Install(i); err == nil { - t.Error("expected error for version does not exists, got none") + t.Fatalf("expected error for version does not exists, got none") } else if err.Error() != fmt.Sprintf("requested version %q does not exist for plugin %q", version, source) { - t.Errorf("expected error for version does not exists, got (%v)", err) + t.Fatalf("expected error for version does not exists, got (%v)", err) } } func TestVCSInstallerUpdate(t *testing.T) { @@ -135,8 +134,7 @@ func TestVCSInstallerUpdate(t *testing.T) { } // ensure a VCSInstaller was returned - _, ok := i.(*VCSInstaller) - if !ok { + if _, ok := i.(*VCSInstaller); !ok { t.Fatal("expected a VCSInstaller") } @@ -157,7 +155,9 @@ func TestVCSInstallerUpdate(t *testing.T) { t.Fatal(err) } - repoRemote := pluginInfo.(*VCSInstaller).Repo.Remote() + vcsInstaller := pluginInfo.(*VCSInstaller) + + repoRemote := vcsInstaller.Repo.Remote() if repoRemote != source { t.Fatalf("invalid source found, expected %q got %q", source, repoRemote) } @@ -168,12 +168,14 @@ func TestVCSInstallerUpdate(t *testing.T) { } // Test update failure - os.Remove(filepath.Join(i.Path(), "plugin.yaml")) + if err := os.Remove(filepath.Join(vcsInstaller.Repo.LocalPath(), "plugin.yaml")); err != nil { + t.Fatal(err) + } // Testing update for error - if err := Update(i); err == nil { - t.Error("expected error for plugin modified, got none") + if err := Update(vcsInstaller); err == nil { + t.Fatalf("expected error for plugin modified, got none") } else if err.Error() != "plugin repo was modified" { - t.Errorf("expected error for plugin modified, got (%v)", err) + t.Fatalf("expected error for plugin modified, got (%v)", err) } } diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index 7bbc3b442..a869255c4 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -28,14 +28,14 @@ import ( func checkCommand(p *Plugin, extraArgs []string, osStrCmp string, t *testing.T) { cmd, args, err := p.PrepareCommand(extraArgs) if err != nil { - t.Errorf(err.Error()) + t.Fatal(err) } if cmd != "echo" { - t.Errorf("Expected echo, got %q", cmd) + t.Fatalf("Expected echo, got %q", cmd) } if l := len(args); l != 5 { - t.Errorf("expected 5 args, got %d", l) + t.Fatalf("expected 5 args, got %d", l) } expect := []string{"-n", osStrCmp, "--debug", "--foo", "bar"} @@ -49,13 +49,13 @@ func checkCommand(p *Plugin, extraArgs []string, osStrCmp string, t *testing.T) p.Metadata.IgnoreFlags = true cmd, args, err = p.PrepareCommand(extraArgs) if err != nil { - t.Errorf(err.Error()) + t.Fatal(err) } if cmd != "echo" { - t.Errorf("Expected echo, got %q", cmd) + t.Fatalf("Expected echo, got %q", cmd) } if l := len(args); l != 2 { - t.Errorf("expected 2 args, got %d", l) + t.Fatalf("expected 2 args, got %d", l) } expect = []string{"-n", osStrCmp} for i := 0; i < len(args); i++ { @@ -155,7 +155,7 @@ func TestNoPrepareCommand(t *testing.T) { _, _, err := p.PrepareCommand(argv) if err == nil { - t.Errorf("Expected error to be returned") + t.Fatalf("Expected error to be returned") } } @@ -172,7 +172,7 @@ func TestNoMatchPrepareCommand(t *testing.T) { argv := []string{"--debug", "--foo", "bar"} if _, _, err := p.PrepareCommand(argv); err == nil { - t.Errorf("Expected error to be returned") + t.Fatalf("Expected error to be returned") } } @@ -184,7 +184,7 @@ func TestLoadDir(t *testing.T) { } if plug.Dir != dirname { - t.Errorf("Expected dir %q, got %q", dirname, plug.Dir) + t.Fatalf("Expected dir %q, got %q", dirname, plug.Dir) } expect := &Metadata{ @@ -200,7 +200,7 @@ func TestLoadDir(t *testing.T) { } if !reflect.DeepEqual(expect, plug.Metadata) { - t.Errorf("Expected plugin metadata %v, got %v", expect, plug.Metadata) + t.Fatalf("Expected plugin metadata %v, got %v", expect, plug.Metadata) } } @@ -212,7 +212,7 @@ func TestDownloader(t *testing.T) { } if plug.Dir != dirname { - t.Errorf("Expected dir %q, got %q", dirname, plug.Dir) + t.Fatalf("Expected dir %q, got %q", dirname, plug.Dir) } expect := &Metadata{ @@ -230,7 +230,7 @@ func TestDownloader(t *testing.T) { } if !reflect.DeepEqual(expect, plug.Metadata) { - t.Errorf("Expected metadata %v, got %v", expect, plug.Metadata) + t.Fatalf("Expected metadata %v, got %v", expect, plug.Metadata) } }