Merge 6e2ab6955b into 0bca9871ad

internal/plugin/installer/base.go

@@ -30,9 +30,20 @@ type base struct {
 
 func newBase(source string) base {
 	settings := cli.New()
+	// When HELM_PLUGINS contains a list of paths, install into the first non-empty
+	// entry so the plugin ends up in a directory that is actually searched. Skipping
+	// empty segments avoids installing into a relative path when HELM_PLUGINS contains
+	// entries like ":/path" or "/path:".
+	pluginsDir := settings.PluginsDirectory
+	for _, dir := range filepath.SplitList(pluginsDir) {
+		if dir != "" {
+			pluginsDir = dir
+			break
+		}
+	}
 	return base{
 		Source:           source,
-		PluginsDirectory: settings.PluginsDirectory,
+		PluginsDirectory: pluginsDir,
 	}
 }
 

internal/plugin/installer/base_test.go

@@ -14,10 +14,12 @@ limitations under the License.
 package installer // import "helm.sh/helm/v4/internal/plugin/installer"
 
 import (
+	"path/filepath"
 	"testing"
 )
 
 func TestPath(t *testing.T) {
+	pluginsDir := filepath.FromSlash("/helm/data/plugins")
 	tests := []struct {
 		source         string
 		helmPluginsDir string
@@ -25,12 +27,12 @@ func TestPath(t *testing.T) {
 	}{
 		{
 			source:         "",
-			helmPluginsDir: "/helm/data/plugins",
+			helmPluginsDir: pluginsDir,
 			expectPath:     "",
 		}, {
 			source:         "https://github.com/jkroepke/helm-secrets",
-			helmPluginsDir: "/helm/data/plugins",
-			expectPath:     "/helm/data/plugins/helm-secrets",
+			helmPluginsDir: pluginsDir,
+			expectPath:     filepath.Join(pluginsDir, "helm-secrets"),
 		},
 	}
 
@@ -43,3 +45,42 @@ func TestPath(t *testing.T) {
 		}
 	}
 }
+
+func TestPathMultiplePluginDirs(t *testing.T) {
+	// When HELM_PLUGINS contains a list of paths, install into the first one.
+	first := filepath.FromSlash("/helm/data/plugins")
+	second := filepath.FromSlash("/helm/extra/plugins")
+	multiPath := first + string(filepath.ListSeparator) + second
+
+	t.Setenv("HELM_PLUGINS", multiPath)
+	b := newBase("https://github.com/jkroepke/helm-secrets")
+	got := b.Path()
+	expected := filepath.Join(first, "helm-secrets")
+	if got != expected {
+		t.Errorf("expected path %s, got %s", expected, got)
+	}
+}
+
+func TestPathEmptyPluginDir(t *testing.T) {
+	// When HELM_PLUGINS is explicitly empty, newBase must not panic.
+	t.Setenv("HELM_PLUGINS", "")
+	b := newBase("https://github.com/jkroepke/helm-secrets")
+	// Path() only returns "" when Source is ""; with an empty PluginsDirectory it
+	// returns a relative path. Just verify no panic.
+	_ = b.Path()
+}
+
+func TestPathSkipsEmptyPluginDirs(t *testing.T) {
+	// A leading empty segment (e.g. ":/real/path") must be skipped so the plugin is
+	// installed into the first real directory rather than a relative path.
+	realDir := filepath.FromSlash("/helm/data/plugins")
+	multiPath := string(filepath.ListSeparator) + realDir
+
+	t.Setenv("HELM_PLUGINS", multiPath)
+	b := newBase("https://github.com/jkroepke/helm-secrets")
+	got := b.Path()
+	expected := filepath.Join(realDir, "helm-secrets")
+	if got != expected {
+		t.Errorf("expected path %s, got %s", expected, got)
+	}
+}

internal/plugin/installer/http_installer_test.go

@@ -1,3 +1,5 @@
+//go:build !windows
+
 /*
 Copyright The Helm Authors.
 Licensed under the Apache License, Version 2.0 (the "License");
@@ -21,13 +23,9 @@ import (
 	"compress/gzip"
 	"encoding/base64"
 	"errors"
-	"fmt"
 	"io/fs"
-	"net/http"
-	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"strings"
 	"syscall"
 	"testing"
 
@@ -66,18 +64,6 @@ func TestStripName(t *testing.T) {
 	}
 }
 
-func mockArchiveServer() *httptest.Server {
-	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if !strings.HasSuffix(r.URL.Path, ".tar.gz") {
-			w.Header().Add("Content-Type", "text/html")
-			fmt.Fprintln(w, "broken")
-			return
-		}
-		w.Header().Add("Content-Type", "application/gzip")
-		fmt.Fprintln(w, "test")
-	}))
-}
-
 func TestHTTPInstaller(t *testing.T) {
 	ensure.HelmHome(t)
 

internal/plugin/installer/oci_installer.go

@@ -29,7 +29,6 @@ import (
 	"helm.sh/helm/v4/internal/plugin"
 	"helm.sh/helm/v4/internal/plugin/cache"
 	"helm.sh/helm/v4/internal/third_party/dep/fs"
-	"helm.sh/helm/v4/pkg/cli"
 	"helm.sh/helm/v4/pkg/getter"
 	"helm.sh/helm/v4/pkg/helmpath"
 	"helm.sh/helm/v4/pkg/registry"
@@ -43,8 +42,7 @@ type OCIInstaller struct {
 	CacheDir   string
 	PluginName string
 	base
-	settings *cli.EnvSettings
-	getter   getter.Getter
+	getter getter.Getter
 	// Cached data to avoid duplicate downloads
 	pluginData []byte
 	provData   []byte
@@ -63,8 +61,6 @@ func NewOCIInstaller(source string, options ...getter.Option) (*OCIInstaller, er
 		return nil, err
 	}
 
-	settings := cli.New()
-
 	// Always add plugin artifact type and any provided options
 	pluginOptions := append([]getter.Option{getter.WithArtifactType("plugin")}, options...)
 	getterProvider, err := getter.NewOCIGetter(pluginOptions...)
@@ -76,7 +72,6 @@ func NewOCIInstaller(source string, options ...getter.Option) (*OCIInstaller, er
 		CacheDir:   helmpath.CachePath("plugins", key),
 		PluginName: pluginName,
 		base:       newBase(source),
-		settings:   settings,
 		getter:     getterProvider,
 	}
 	return i, nil
@@ -195,7 +190,7 @@ func (i OCIInstaller) Path() string {
 	if i.Source == "" {
 		return ""
 	}
-	return filepath.Join(i.settings.PluginsDirectory, i.PluginName)
+	return filepath.Join(i.PluginsDirectory, i.PluginName)
 }
 
 // extractTarGz extracts a gzipped tar archive to a directory

internal/plugin/installer/oci_installer_test.go

@@ -27,6 +27,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -35,7 +36,6 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 
 	"helm.sh/helm/v4/internal/test/ensure"
-	"helm.sh/helm/v4/pkg/cli"
 	"helm.sh/helm/v4/pkg/getter"
 	"helm.sh/helm/v4/pkg/helmpath"
 )
@@ -266,10 +266,6 @@ func TestNewOCIInstaller(t *testing.T) {
 				t.Errorf("expected cache directory to contain 'plugins', got %s", installer.CacheDir)
 			}
 
-			if installer.settings == nil {
-				t.Error("expected settings to be initialized")
-			}
-
 			// Check that Path() method works
 			expectedPath := helmpath.DataPath("plugins", tt.expectName)
 			if installer.Path() != expectedPath {
@@ -305,7 +301,6 @@ func TestOCIInstaller_Path(t *testing.T) {
 			installer := &OCIInstaller{
 				PluginName: tt.pluginName,
 				base:       newBase(tt.source),
-				settings:   cli.New(),
 			}
 
 			path := installer.Path()
@@ -539,7 +534,7 @@ func TestOCIInstaller_Install_ComponentExtraction(t *testing.T) {
 	execPath := filepath.Join(tempDir, "bin", pluginName)
 	if info, err := os.Stat(execPath); err != nil {
 		t.Errorf("executable not found: %v", err)
-	} else if info.Mode()&0111 == 0 {
+	} else if runtime.GOOS != "windows" && info.Mode()&0111 == 0 {
 		t.Error("file is not executable")
 	}
 

internal/plugin/installer/testhelper_test.go

@@ -0,0 +1,35 @@
+/*
+Copyright The Helm Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package installer // import "helm.sh/helm/v4/internal/plugin/installer"
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+)
+
+func mockArchiveServer() *httptest.Server {
+	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !strings.HasSuffix(r.URL.Path, ".tar.gz") {
+			w.Header().Add("Content-Type", "text/html")
+			fmt.Fprintln(w, "broken")
+			return
+		}
+		w.Header().Add("Content-Type", "application/gzip")
+		fmt.Fprintln(w, "test")
+	}))
+}