Merge 4f28f89aac into 62c7e845d1

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
 	}