From ecf82a882ccc0606fdbf6d0e081711d5d108cef8 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sat, 7 Aug 2021 09:29:43 +0800 Subject: [PATCH] Optimization logic Signed-off-by: yxxhero --- cmd/helm/plugin_install.go | 2 +- cmd/helm/plugin_update.go | 2 +- pkg/plugin/installer/http_installer.go | 13 +++++++++-- pkg/plugin/installer/http_installer_test.go | 6 ++--- pkg/plugin/installer/installer.go | 23 +++++++++++++++----- pkg/plugin/installer/local_installer.go | 16 +++++++++----- pkg/plugin/installer/local_installer_test.go | 2 +- pkg/plugin/installer/vcs_installer.go | 20 +++++++++++++---- pkg/plugin/installer/vcs_installer_test.go | 10 ++++----- 9 files changed, 65 insertions(+), 29 deletions(-) diff --git a/cmd/helm/plugin_install.go b/cmd/helm/plugin_install.go index deccfd2fe..6d5bb60e0 100644 --- a/cmd/helm/plugin_install.go +++ b/cmd/helm/plugin_install.go @@ -71,7 +71,7 @@ func (o *pluginInstallOptions) complete(args []string) error { func (o *pluginInstallOptions) run(out io.Writer) error { installer.Debug = settings.Debug - i, err := installer.NewForSource(o.source, o.version, settings.PluginsDirectory) + i, err := installer.NewForSourceWithPluginsDirectory(o.source, o.version, settings.PluginsDirectory) if err != nil { return err } diff --git a/cmd/helm/plugin_update.go b/cmd/helm/plugin_update.go index b2ea68fc8..4515acdbb 100644 --- a/cmd/helm/plugin_update.go +++ b/cmd/helm/plugin_update.go @@ -96,7 +96,7 @@ func updatePlugin(p *plugin.Plugin) error { return err } - i, err := installer.FindSource(absExactLocation, settings.PluginsDirectory) + i, err := installer.FindSource(absExactLocation) if err != nil { return err } diff --git a/pkg/plugin/installer/http_installer.go b/pkg/plugin/installer/http_installer.go index 09d5d1754..5202f04d5 100644 --- a/pkg/plugin/installer/http_installer.go +++ b/pkg/plugin/installer/http_installer.go @@ -82,7 +82,7 @@ func NewExtractor(source string) (Extractor, error) { } // NewHTTPInstaller creates a new HttpInstaller. -func NewHTTPInstaller(source, pluginsDirectory string) (*HTTPInstaller, error) { +func NewHTTPInstaller(source string) (*HTTPInstaller, error) { key, err := cache.Key(source) if err != nil { return nil, err @@ -101,13 +101,22 @@ func NewHTTPInstaller(source, pluginsDirectory string) (*HTTPInstaller, error) { i := &HTTPInstaller{ CacheDir: helmpath.CachePath("plugins", key), PluginName: stripPluginName(filepath.Base(source)), - base: newBase(source, pluginsDirectory), extractor: extractor, getter: get, } return i, nil } +// NewHTTPInstallerWithPluginsDirectory creates a new HttpInstaller with pluginsDirectory arg, it should be removed in helm v4. +func NewHTTPInstallerWithPluginsDirectory(source, pluginsDirectory string) (*HTTPInstaller, error) { + i, err := NewHTTPInstaller(source) + if err != nil { + return nil, err + } + i.base = newBase(source, pluginsDirectory) + return i, nil +} + // helper that relies on some sort of convention for plugin name (plugin-name-) func stripPluginName(name string) string { var strippedName string diff --git a/pkg/plugin/installer/http_installer_test.go b/pkg/plugin/installer/http_installer_test.go index 7268e361f..1369ece0c 100644 --- a/pkg/plugin/installer/http_installer_test.go +++ b/pkg/plugin/installer/http_installer_test.go @@ -92,7 +92,7 @@ func TestHTTPInstaller(t *testing.T) { t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) } - i, err := NewForSource(source, "0.0.1", settings.PluginsDirectory) + i, err := NewForSourceWithPluginsDirectory(source, "0.0.1", settings.PluginsDirectory) if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -141,7 +141,7 @@ func TestHTTPInstallerNonExistentVersion(t *testing.T) { t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) } - i, err := NewForSource(source, "0.0.2", settings.PluginsDirectory) + i, err := NewForSourceWithPluginsDirectory(source, "0.0.2", settings.PluginsDirectory) if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -175,7 +175,7 @@ func TestHTTPInstallerUpdate(t *testing.T) { t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) } - i, err := NewForSource(source, "0.0.1", settings.PluginsDirectory) + i, err := NewForSourceWithPluginsDirectory(source, "0.0.1", settings.PluginsDirectory) if err != nil { t.Fatalf("unexpected error: %s", err) } diff --git a/pkg/plugin/installer/installer.go b/pkg/plugin/installer/installer.go index 9cf28a962..5b5ad4470 100644 --- a/pkg/plugin/installer/installer.go +++ b/pkg/plugin/installer/installer.go @@ -63,20 +63,31 @@ func Update(i Installer) error { return i.Update() } +// NewForSourceWithPluginsDirectory determines the correct Installer for the given source with pluginsDiretory arg, it should be removed in helm v4. +func NewForSourceWithPluginsDirectory(source, version, pluginsDirectory string) (Installer, error) { + // Check if source is a local directory + if isLocalReference(source) { + return NewLocalInstallerWithPluginsDirectory(source, pluginsDirectory) + } else if isRemoteHTTPArchive(source) { + return NewHTTPInstallerWithPluginsDirectory(source, pluginsDirectory) + } + return NewVCSInstallerWithPluginsDirectory(source, version, pluginsDirectory) +} + // NewForSource determines the correct Installer for the given source. -func NewForSource(source, version, pluginsDirectory string) (Installer, error) { +func NewForSource(source, version string) (Installer, error) { // Check if source is a local directory if isLocalReference(source) { - return NewLocalInstaller(source, pluginsDirectory) + return NewLocalInstaller(source) } else if isRemoteHTTPArchive(source) { - return NewHTTPInstaller(source, pluginsDirectory) + return NewHTTPInstaller(source) } - return NewVCSInstaller(source, version, pluginsDirectory) + return NewVCSInstaller(source, version) } // FindSource determines the correct Installer for the given source. -func FindSource(location, pluginsDirectory string) (Installer, error) { - installer, err := existingVCSRepo(location, pluginsDirectory) +func FindSource(location string) (Installer, error) { + installer, err := existingVCSRepo(location) if err != nil && err.Error() == "Cannot detect VCS" { return installer, errors.New("cannot get information about plugin source") } diff --git a/pkg/plugin/installer/local_installer.go b/pkg/plugin/installer/local_installer.go index d6c238242..4a4129bad 100644 --- a/pkg/plugin/installer/local_installer.go +++ b/pkg/plugin/installer/local_installer.go @@ -27,16 +27,20 @@ type LocalInstaller struct { base } -// NewLocalInstaller creates a new LocalInstaller. -func NewLocalInstaller(source, pluginsDirectory string) (*LocalInstaller, error) { +// NewLocalInstallerWithPluginsDirectory creates a new LocalInstaller with pluginsDirectory arg, it should be removed in helm v4. +func NewLocalInstallerWithPluginsDirectory(source, pluginsDirectory string) (*LocalInstaller, error) { src, err := filepath.Abs(source) if err != nil { return nil, errors.Wrap(err, "unable to get absolute path to plugin") } - i := &LocalInstaller{ - base: newBase(src, pluginsDirectory), - } - return i, nil + i, err := NewLocalInstaller(source) + i.base = newBase(pluginsDirectory, src) + return i, err +} + +// NewLocalInstaller creates a new LocalInstaller. +func NewLocalInstaller(source string) (*LocalInstaller, error) { + return &LocalInstaller{}, nil } // Install creates a symlink to the plugin directory. diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go index 0c4d0ed35..7a169dc08 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/good/echo" - i, err := NewForSource(source, "", settings.PluginsDirectory) + i, err := NewForSourceWithPluginsDirectory(source, "", settings.PluginsDirectory) if err != nil { t.Fatalf("unexpected error: %s", err) } diff --git a/pkg/plugin/installer/vcs_installer.go b/pkg/plugin/installer/vcs_installer.go index 8217f1325..5b5e438c7 100644 --- a/pkg/plugin/installer/vcs_installer.go +++ b/pkg/plugin/installer/vcs_installer.go @@ -24,6 +24,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/helmpath" "helm.sh/helm/v3/pkg/plugin/cache" ) @@ -35,20 +36,32 @@ type VCSInstaller struct { base } -func existingVCSRepo(location, pluginsDirectory string) (Installer, error) { +func existingVCSRepo(location string) (Installer, error) { repo, err := vcs.NewRepo("", location) if err != nil { return nil, err } + settings := cli.New() + i := &VCSInstaller{ Repo: repo, - base: newBase(repo.Remote(), pluginsDirectory), + base: newBase(repo.Remote(), settings.PluginsDirectory), + } + return i, nil +} + +// NewVCSInstallerWithPluginsDirectory creates a new VCSInstaller with pluginsDirector arg, it should be removed in helm v4. +func NewVCSInstallerWithPluginsDirectory(source, version, pluginsDirectory string) (*VCSInstaller, error) { + i, err := NewVCSInstaller(source, version) + if err != nil { + return nil, err } + i.base = newBase(source, pluginsDirectory) return i, nil } // NewVCSInstaller creates a new VCSInstaller. -func NewVCSInstaller(source, version, pluginsDirectory string) (*VCSInstaller, error) { +func NewVCSInstaller(source, version string) (*VCSInstaller, error) { key, err := cache.Key(source) if err != nil { return nil, err @@ -61,7 +74,6 @@ func NewVCSInstaller(source, version, pluginsDirectory string) (*VCSInstaller, e i := &VCSInstaller{ Repo: repo, Version: version, - base: newBase(source, pluginsDirectory), } return i, err } diff --git a/pkg/plugin/installer/vcs_installer_test.go b/pkg/plugin/installer/vcs_installer_test.go index 72e7e0ec3..c9329e42e 100644 --- a/pkg/plugin/installer/vcs_installer_test.go +++ b/pkg/plugin/installer/vcs_installer_test.go @@ -64,7 +64,7 @@ func TestVCSInstaller(t *testing.T) { tags: []string{"0.1.0", "0.1.1"}, } - i, err := NewForSource(source, "~0.1.0", settings.PluginsDirectory) + i, err := NewForSourceWithPluginsDirectory(source, "~0.1.0", settings.PluginsDirectory) if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -96,7 +96,7 @@ func TestVCSInstaller(t *testing.T) { } // Testing FindSource method, expect error because plugin code is not a cloned repository - if _, err := FindSource(i.Path(), settings.PluginsDirectory); err == nil { + if _, err := FindSource(i.Path()); err == nil { t.Fatalf("expected error for inability to find plugin source, got none") } else if err.Error() != "cannot get information about plugin source" { t.Fatalf("expected error for inability to find plugin source, got (%v)", err) @@ -110,7 +110,7 @@ func TestVCSInstallerNonExistentVersion(t *testing.T) { version := "0.2.0" settings := cli.New() - i, err := NewForSource(source, version, settings.PluginsDirectory) + i, err := NewForSourceWithPluginsDirectory(source, version, settings.PluginsDirectory) if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -132,7 +132,7 @@ func TestVCSInstallerUpdate(t *testing.T) { source := "https://github.com/adamreese/helm-env" settings := cli.New() - i, err := NewForSource(source, "", settings.PluginsDirectory) + i, err := NewForSourceWithPluginsDirectory(source, "", settings.PluginsDirectory) if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -154,7 +154,7 @@ func TestVCSInstallerUpdate(t *testing.T) { } // Test FindSource method for positive result - pluginInfo, err := FindSource(i.Path(), settings.PluginsDirectory) + pluginInfo, err := FindSource(i.Path()) if err != nil { t.Fatal(err) }