diff --git a/internal/plugin/installer/base.go b/internal/plugin/installer/base.go index 54d25287a..b6ea2c99b 100644 --- a/internal/plugin/installer/base.go +++ b/internal/plugin/installer/base.go @@ -32,7 +32,7 @@ func newBase(source string) base { settings := cli.New() return base{ Source: source, - PluginsDirectory: settings.PluginsDirectory, + PluginsDirectory: settings.PluginInstallDirectory(), } } diff --git a/internal/plugin/installer/base_test.go b/internal/plugin/installer/base_test.go index 8ef7ff588..ff605ad1e 100644 --- a/internal/plugin/installer/base_test.go +++ b/internal/plugin/installer/base_test.go @@ -14,6 +14,8 @@ limitations under the License. package installer import ( + "os" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -33,6 +35,13 @@ func TestPath(t *testing.T) { source: "https://github.com/jkroepke/helm-secrets", helmPluginsDir: "/helm/data/plugins", expectPath: "/helm/data/plugins/helm-secrets", + }, { + // HELM_PLUGINS may hold a list of directories. New plugins are + // installed into the first one, not into a literal path built + // from the whole list. + source: "https://github.com/jkroepke/helm-secrets", + helmPluginsDir: strings.Join([]string{"/helm/data/plugins", "/shared/plugins"}, string(os.PathListSeparator)), + expectPath: "/helm/data/plugins/helm-secrets", }, } diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index 383ddb914..c70e4adaa 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/internal/plugin/installer/oci_installer.go @@ -195,7 +195,7 @@ func (i OCIInstaller) Path() string { if i.Source == "" { return "" } - return filepath.Join(i.settings.PluginsDirectory, i.PluginName) + return filepath.Join(i.settings.PluginInstallDirectory(), i.PluginName) } // extractTarGz extracts a gzipped tar archive to a directory diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go index b0463d01f..342318439 100644 --- a/pkg/cli/environment.go +++ b/pkg/cli/environment.go @@ -26,6 +26,7 @@ package cli import ( "net/http" "os" + "path/filepath" "strconv" "strings" @@ -290,6 +291,19 @@ func (s *EnvSettings) SetNamespace(namespace string) { s.namespace = namespace } +// PluginInstallDirectory returns the directory new plugins are installed into. +// +// PluginsDirectory may hold a list of directories separated by the OS specific +// path list separator (as accepted by filepath.SplitList). All of them are +// searched when loading plugins, but a new plugin has to be written to exactly +// one of them, so the first (highest precedence) entry is used. +func (s *EnvSettings) PluginInstallDirectory() string { + if dirs := filepath.SplitList(s.PluginsDirectory); len(dirs) > 0 { + return dirs[0] + } + return s.PluginsDirectory +} + // RESTClientGetter gets the kubeconfig from EnvSettings func (s *EnvSettings) RESTClientGetter() genericclioptions.RESTClientGetter { return s.config diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go index fc71038f8..e4301ee17 100644 --- a/pkg/cli/environment_test.go +++ b/pkg/cli/environment_test.go @@ -147,6 +147,40 @@ func TestEnvSettings(t *testing.T) { } } +func TestPluginInstallDirectory(t *testing.T) { + first := filepath.Join("home", "user", "plugins") + second := filepath.Join("shared", "plugins") + + tests := []struct { + name string + env string + expected string + }{ + { + name: "single directory", + env: first, + expected: first, + }, + { + name: "list of directories uses the first one", + env: strings.Join([]string{first, second}, string(os.PathListSeparator)), + expected: first, + }, + { + name: "empty value falls back to the raw setting", + env: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + settings := &EnvSettings{PluginsDirectory: tt.env} + assert.Equal(t, tt.expected, settings.PluginInstallDirectory()) + }) + } +} + func TestEnvOrBool(t *testing.T) { const envName = "TEST_ENV_OR_BOOL" tests := []struct { diff --git a/pkg/cmd/plugin_uninstall.go b/pkg/cmd/plugin_uninstall.go index 81d9b31ee..faf2a8900 100644 --- a/pkg/cmd/plugin_uninstall.go +++ b/pkg/cmd/plugin_uninstall.go @@ -62,7 +62,7 @@ func (o *pluginUninstallOptions) complete(args []string) error { func (o *pluginUninstallOptions) run(out io.Writer) error { slog.Debug("loading installer plugins", "dir", settings.PluginsDirectory) - plugins, err := plugin.LoadAllDir(settings.PluginsDirectory, plugin.LogIgnorePluginLoadErrorFilterFunc) + plugins, err := plugin.FindPlugins(filepath.SplitList(settings.PluginsDirectory), plugin.Descriptor{}) if err != nil { return err } @@ -89,11 +89,13 @@ func uninstallPlugin(p plugin.Plugin) error { return err } - // Clean up versioned tarball and provenance files from HELM_PLUGINS directory + // Clean up versioned tarball and provenance files from the plugins directory + // the plugin was installed into. HELM_PLUGINS may list several directories, + // so the files are looked up next to the plugin itself. // These files are saved with pattern: PLUGIN_NAME-VERSION.tgz and PLUGIN_NAME-VERSION.tgz.prov pluginName := p.Metadata().Name pluginVersion := p.Metadata().Version - pluginsDir := settings.PluginsDirectory + pluginsDir := filepath.Dir(p.Dir()) // Remove versioned files: plugin-name-version.tgz and plugin-name-version.tgz.prov if pluginVersion != "" { diff --git a/pkg/cmd/plugin_uninstall_test.go b/pkg/cmd/plugin_uninstall_test.go index bd08f6f13..baffa7bfa 100644 --- a/pkg/cmd/plugin_uninstall_test.go +++ b/pkg/cmd/plugin_uninstall_test.go @@ -17,8 +17,10 @@ package cmd import ( "fmt" + "io" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -126,3 +128,41 @@ command: $HELM_PLUGIN_DIR/test-plugin _, err = os.Stat(otherVersionTarball) assert.False(t, os.IsNotExist(err), "other version tarball should NOT be removed") } + +func TestPluginUninstallWithMultiplePluginDirs(t *testing.T) { + ensure.HelmHome(t) + + firstDir := t.TempDir() + secondDir := t.TempDir() + t.Setenv("HELM_PLUGINS", strings.Join([]string{firstDir, secondDir}, string(os.PathListSeparator))) + + origSettings := settings + settings = cli.New() + t.Cleanup(func() { settings = origSettings }) + + // The plugin lives in the second directory of HELM_PLUGINS, along with the + // versioned files kept next to it. + pluginDir := filepath.Join(secondDir, "test-plugin") + require.NoError(t, os.MkdirAll(pluginDir, 0o755)) + pluginYAML := `name: test-plugin +version: 1.2.3 +description: Test plugin +command: $HELM_PLUGIN_DIR/test-plugin +` + require.NoError(t, os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(pluginYAML), 0o644)) + + tarballFile := filepath.Join(secondDir, "test-plugin-1.2.3.tgz") + provFile := filepath.Join(secondDir, "test-plugin-1.2.3.tgz.prov") + require.NoError(t, os.WriteFile(tarballFile, []byte("fake tarball"), 0o644)) + require.NoError(t, os.WriteFile(provFile, []byte("fake provenance"), 0o644)) + + o := &pluginUninstallOptions{names: []string{"test-plugin"}} + require.NoError(t, o.run(io.Discard)) + + _, err := os.Stat(pluginDir) + assert.True(t, os.IsNotExist(err), "plugin directory should be removed") + _, err = os.Stat(tarballFile) + assert.True(t, os.IsNotExist(err), "versioned tarball file should be removed") + _, err = os.Stat(provFile) + assert.True(t, os.IsNotExist(err), "versioned provenance file should be removed") +} diff --git a/pkg/cmd/plugin_update.go b/pkg/cmd/plugin_update.go index 1cba7ffa9..29032f802 100644 --- a/pkg/cmd/plugin_update.go +++ b/pkg/cmd/plugin_update.go @@ -62,7 +62,7 @@ func (o *pluginUpdateOptions) complete(args []string) error { func (o *pluginUpdateOptions) run(out io.Writer) error { slog.Debug("loading installed plugins", "path", settings.PluginsDirectory) - plugins, err := plugin.LoadAllDir(settings.PluginsDirectory, plugin.LogIgnorePluginLoadErrorFilterFunc) + plugins, err := plugin.FindPlugins(filepath.SplitList(settings.PluginsDirectory), plugin.Descriptor{}) if err != nil { return err }