fix(plugin): install plugins into the first HELM_PLUGINS directory

HELM_PLUGINS may contain a list of directories separated by the OS path
list separator. Loading and listing plugins already splits that list, but
plugin installation joined the whole value with the plugin name, creating
a literal directory such as "/tmp/abc:/tmp/xyz/helm-secrets". The install
reported success while the plugin was never found afterwards.

Add EnvSettings.PluginInstallDirectory(), which returns the first entry of
HELM_PLUGINS (the one with the highest precedence when loading), and use it
for the install path in both the base and OCI installers.

`helm plugin update` and `helm plugin uninstall` had the same problem: they
passed the raw setting to LoadAllDir, so no plugin was found when several
directories were configured. They now search every configured directory,
and uninstall removes the versioned tarball and provenance files next to
the plugin instead of assuming the first directory.

Closes #11310

Signed-off-by: ChadiDridi <dridichady@gmail.com>

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(),
 	}
 }
 

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",
 		},
 	}
 

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

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

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 {

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 != "" {

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")
+}

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
 	}