diff --git a/internal/plugin/installer/base.go b/internal/plugin/installer/base.go index c21a245a8..1b2f5c8a9 100644 --- a/internal/plugin/installer/base.go +++ b/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, } } diff --git a/internal/plugin/installer/base_test.go b/internal/plugin/installer/base_test.go index 6df8ec8a1..9dbbb8e1c 100644 --- a/internal/plugin/installer/base_test.go +++ b/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) + } +} diff --git a/internal/plugin/installer/http_installer_test.go b/internal/plugin/installer/http_installer_test.go index efbca90c9..415298bf4 100644 --- a/internal/plugin/installer/http_installer_test.go +++ b/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) diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index 50d01522a..f323c7aa7 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/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 diff --git a/internal/plugin/installer/oci_installer_test.go b/internal/plugin/installer/oci_installer_test.go index 1f25f4e76..d9b52b7cf 100644 --- a/internal/plugin/installer/oci_installer_test.go +++ b/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") } diff --git a/internal/plugin/installer/testhelper_test.go b/internal/plugin/installer/testhelper_test.go new file mode 100644 index 000000000..b0f586d8c --- /dev/null +++ b/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") + })) +}