diff --git a/internal/plugin/installer/http_installer.go b/internal/plugin/installer/http_installer.go index bb96314f4..a422bacfb 100644 --- a/internal/plugin/installer/http_installer.go +++ b/internal/plugin/installer/http_installer.go @@ -91,7 +91,15 @@ func (i *HTTPInstaller) Install() error { return fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) } filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) - tarballPath := helmpath.DataPath("plugins", filename) + pluginsPath := filepath.Dir(i.Path()) + foundPlugins, err := plugin.FindPlugins([]string{pluginsPath}, plugin.Descriptor{Name: metadata.Name}) + if err != nil { + return fmt.Errorf("failed to search for existing plugins: %w", err) + } + if len(foundPlugins) > 0 { + return fmt.Errorf("plugin %q already exists at %q", metadata.Name, foundPlugins[0].Dir()) + } + tarballPath := filepath.Join(pluginsPath, filename) if err := os.MkdirAll(filepath.Dir(tarballPath), 0755); err != nil { return fmt.Errorf("failed to create plugins directory: %w", err) } diff --git a/internal/plugin/installer/http_installer_test.go b/internal/plugin/installer/http_installer_test.go index 7f7e6cef6..9acfc1ef1 100644 --- a/internal/plugin/installer/http_installer_test.go +++ b/internal/plugin/installer/http_installer_test.go @@ -127,6 +127,58 @@ func TestHTTPInstaller(t *testing.T) { } +func TestHTTPInstallerPluginAlreadyExists(t *testing.T) { + ensure.HelmHome(t) + + srv := mockArchiveServer() + defer srv.Close() + source := srv.URL + "/plugins/fake-plugin-0.0.1.tar.gz" + + if err := os.MkdirAll(helmpath.DataPath("plugins"), 0755); err != nil { + t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) + } + + i, err := NewForSource(source, "0.0.1") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + // ensure a HTTPInstaller was returned + httpInstaller, ok := i.(*HTTPInstaller) + if !ok { + t.Fatal("expected a HTTPInstaller") + } + + // inject fake http client responding with minimal plugin tarball + mockTgz, err := base64.StdEncoding.DecodeString(fakePluginB64) + if err != nil { + t.Fatalf("Could not decode fake tgz plugin: %s", err) + } + + httpInstaller.getter = &TestHTTPGetter{ + MockResponse: bytes.NewBuffer(mockTgz), + } + + // First install should succeed + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Reset the getter mock for second install attempt + httpInstaller.getter = &TestHTTPGetter{ + MockResponse: bytes.NewBuffer(mockTgz), + } + + // Second install should fail with specific error message + err = httpInstaller.Install() + if err == nil { + t.Fatal("expected error for plugin already exists") + } + if !strings.Contains(err.Error(), "plugin \"fake-plugin\" already exists at") { + t.Fatalf("expected 'plugin already exists' error message, got: %v", err) + } +} + func TestHTTPInstallerNonExistentVersion(t *testing.T) { ensure.HelmHome(t) srv := mockArchiveServer() diff --git a/internal/plugin/installer/local_installer.go b/internal/plugin/installer/local_installer.go index 1c8314282..91d9c428b 100644 --- a/internal/plugin/installer/local_installer.go +++ b/internal/plugin/installer/local_installer.go @@ -97,6 +97,23 @@ func (i *LocalInstaller) installFromDirectory() error { if !isPlugin(i.Source) { return ErrMissingMetadata } + + // Load plugin to get metadata and check for existing plugin with same name + p, err := plugin.LoadDir(i.Source) + if err != nil { + return fmt.Errorf("failed to load plugin: %w", err) + } + metadata := p.Metadata() + + pluginsPath := helmpath.DataPath("plugins") + foundPlugins, err := plugin.FindPlugins([]string{pluginsPath}, plugin.Descriptor{Name: metadata.Name}) + if err != nil { + return fmt.Errorf("failed to search for existing plugins: %w", err) + } + if len(foundPlugins) > 0 { + return fmt.Errorf("plugin %q already exists at %q", metadata.Name, foundPlugins[0].Dir()) + } + slog.Debug("symlinking", "source", i.Source, "path", i.Path()) return os.Symlink(i.Source, i.Path()) } @@ -116,7 +133,15 @@ func (i *LocalInstaller) installFromArchive() error { return fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) } filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) - tarballPath := helmpath.DataPath("plugins", filename) + pluginsPath := filepath.Dir(i.Path()) + foundPlugins, err := plugin.FindPlugins([]string{pluginsPath}, plugin.Descriptor{Name: metadata.Name}) + if err != nil { + return fmt.Errorf("failed to search for existing plugins: %w", err) + } + if len(foundPlugins) > 0 { + return fmt.Errorf("plugin %q already exists at %q", metadata.Name, foundPlugins[0].Dir()) + } + tarballPath := filepath.Join(pluginsPath, filename) if err := os.MkdirAll(filepath.Dir(tarballPath), 0755); err != nil { return fmt.Errorf("failed to create plugins directory: %w", err) } diff --git a/internal/plugin/installer/local_installer_test.go b/internal/plugin/installer/local_installer_test.go index 3ee8ab6d0..cc11e58fc 100644 --- a/internal/plugin/installer/local_installer_test.go +++ b/internal/plugin/installer/local_installer_test.go @@ -21,6 +21,7 @@ import ( "compress/gzip" "os" "path/filepath" + "strings" "testing" "helm.sh/helm/v4/internal/test/ensure" @@ -146,3 +147,116 @@ func TestLocalInstallerTarball(t *testing.T) { t.Fatalf("plugin not found at %s: %v", i.Path(), err) } } + +func TestLocalInstallerDirectoryAlreadyExists(t *testing.T) { + ensure.HelmHome(t) + + source := "../testdata/plugdir/good/echo-v1" + i, err := NewForSource(source, "") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + // First install should succeed + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Create another installer for the same plugin directory + i2, err := NewForSource(source, "") + if err != nil { + t.Fatalf("unexpected error creating second installer: %s", err) + } + + localInstaller, ok := i2.(*LocalInstaller) + if !ok { + t.Fatal("expected LocalInstaller") + } + + // Second install should fail with plugin already exists error + err = localInstaller.installFromDirectory() + if err == nil { + t.Fatal("expected error for plugin already exists") + } + if !strings.Contains(err.Error(), "plugin \"echo-v1\" already exists at") { + t.Fatalf("expected 'plugin already exists' error message, got: %v", err) + } +} + +func TestLocalInstallerTarballAlreadyExists(t *testing.T) { + ensure.HelmHome(t) + + // Create a test tarball + tempDir := t.TempDir() + tarballPath := filepath.Join(tempDir, "duplicate-plugin-1.0.0.tar.gz") + + // Create tarball content + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + + files := []struct { + Name string + Body string + Mode int64 + }{ + {"duplicate-plugin/plugin.yaml", "name: duplicate-plugin\napiVersion: v1\ntype: cli/v1\nruntime: subprocess\nversion: 1.0.0\nconfig:\n shortHelp: test\n longHelp: test\nruntimeConfig:\n platformCommand:\n - command: echo", 0644}, + {"duplicate-plugin/bin/duplicate-plugin", "#!/bin/bash\necho test", 0755}, + } + + for _, file := range files { + hdr := &tar.Header{ + Name: file.Name, + Mode: file.Mode, + Size: int64(len(file.Body)), + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(file.Body)); err != nil { + t.Fatal(err) + } + } + + if err := tw.Close(); err != nil { + t.Fatal(err) + } + if err := gw.Close(); err != nil { + t.Fatal(err) + } + + // Write tarball to file + if err := os.WriteFile(tarballPath, buf.Bytes(), 0644); err != nil { + t.Fatal(err) + } + + // First install + i, err := NewForSource(tarballPath, "") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Create second installer for same tarball + i2, err := NewForSource(tarballPath, "") + if err != nil { + t.Fatalf("unexpected error creating second installer: %s", err) + } + + localInstaller, ok := i2.(*LocalInstaller) + if !ok { + t.Fatal("expected LocalInstaller") + } + + // Second install should fail + err = localInstaller.installFromArchive() + if err == nil { + t.Fatal("expected error for plugin already exists") + } + if !strings.Contains(err.Error(), "plugin \"duplicate-plugin\" already exists at") { + t.Fatalf("expected 'plugin already exists' error message, got: %v", err) + } +} diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index 67f99b6f8..fd77a322a 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/internal/plugin/installer/oci_installer.go @@ -102,8 +102,15 @@ func (i *OCIInstaller) Install() error { return fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) } filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) - - tarballPath := helmpath.DataPath("plugins", filename) + pluginsPath := filepath.Dir(i.Path()) + foundPlugins, err := plugin.FindPlugins([]string{pluginsPath}, plugin.Descriptor{Name: metadata.Name}) + if err != nil { + return fmt.Errorf("failed to search for existing plugins: %w", err) + } + if len(foundPlugins) > 0 { + return fmt.Errorf("plugin %q already exists at %q", metadata.Name, foundPlugins[0].Dir()) + } + tarballPath := filepath.Join(pluginsPath, filename) if err := os.MkdirAll(filepath.Dir(tarballPath), 0755); err != nil { return fmt.Errorf("failed to create plugins directory: %w", err) } diff --git a/internal/plugin/installer/vcs_installer.go b/internal/plugin/installer/vcs_installer.go index 3601ec7a8..99f19e006 100644 --- a/internal/plugin/installer/vcs_installer.go +++ b/internal/plugin/installer/vcs_installer.go @@ -21,11 +21,13 @@ import ( stdfs "io/fs" "log/slog" "os" + "path/filepath" "sort" "github.com/Masterminds/semver/v3" "github.com/Masterminds/vcs" + "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/helmpath" @@ -91,6 +93,23 @@ func (i *VCSInstaller) Install() error { return ErrMissingMetadata } + // Load plugin to get metadata and check for existing plugin with same name + p, err := plugin.LoadDir(i.Repo.LocalPath()) + if err != nil { + return fmt.Errorf("failed to load plugin: %w", err) + } + metadata := p.Metadata() + + // Check if a plugin with the same name already exists + pluginsPath := filepath.Dir(i.Path()) + foundPlugins, err := plugin.FindPlugins([]string{pluginsPath}, plugin.Descriptor{Name: metadata.Name}) + if err != nil { + return fmt.Errorf("failed to search for existing plugins: %w", err) + } + if len(foundPlugins) > 0 { + return fmt.Errorf("plugin %q already exists at %q", metadata.Name, foundPlugins[0].Dir()) + } + slog.Debug("copying files", "source", i.Repo.LocalPath(), "destination", i.Path()) return fs.CopyDir(i.Repo.LocalPath(), i.Path()) } diff --git a/internal/plugin/installer/vcs_installer_test.go b/internal/plugin/installer/vcs_installer_test.go index d542a0f75..4344a4500 100644 --- a/internal/plugin/installer/vcs_installer_test.go +++ b/internal/plugin/installer/vcs_installer_test.go @@ -103,6 +103,63 @@ func TestVCSInstaller(t *testing.T) { } } +func TestVCSInstallerPluginAlreadyExists(t *testing.T) { + ensure.HelmHome(t) + + if err := os.MkdirAll(helmpath.DataPath("plugins"), 0755); err != nil { + t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) + } + + source := "https://github.com/adamreese/helm-env" + testRepoPath, _ := filepath.Abs("../testdata/plugdir/good/echo-v1") + repo := &testRepo{ + local: testRepoPath, + tags: []string{"0.1.0", "0.1.1"}, + } + + i, err := NewForSource(source, "~0.1.0") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + // ensure a VCSInstaller was returned + vcsInstaller, ok := i.(*VCSInstaller) + if !ok { + t.Fatal("expected a VCSInstaller") + } + + // set the testRepo in the VCSInstaller + vcsInstaller.Repo = repo + + // First install should succeed + if err := Install(i); err != nil { + t.Fatal(err) + } + + // Create a second VCS installer for the same plugin + i2, err := NewForSource(source, "~0.1.0") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + vcsInstaller2, ok := i2.(*VCSInstaller) + if !ok { + t.Fatal("expected a VCSInstaller") + } + + // set the testRepo in the VCSInstaller + vcsInstaller2.Repo = repo + + // Second install should fail with plugin already exists error + err = vcsInstaller2.Install() + if err == nil { + t.Fatal("expected error for plugin already exists") + } + if !strings.Contains(err.Error(), "plugin \"echo-v1\" already exists at") { + t.Fatalf("expected 'plugin already exists' error message, got: %v", err) + } +} + func TestVCSInstallerNonExistentVersion(t *testing.T) { ensure.HelmHome(t) diff --git a/internal/test/ensure/ensure.go b/internal/test/ensure/ensure.go index a72f48c2d..a7f9fb9a2 100644 --- a/internal/test/ensure/ensure.go +++ b/internal/test/ensure/ensure.go @@ -29,7 +29,7 @@ import ( func HelmHome(t *testing.T) { t.Helper() base := t.TempDir() - t.Setenv(xdg.CacheHomeEnvVar, base) + t.Setenv(xdg.CacheHomeEnvVar, filepath.Join(base, "cache")) t.Setenv(xdg.ConfigHomeEnvVar, base) t.Setenv(xdg.DataHomeEnvVar, base) t.Setenv(helmpath.CacheHomeEnvVar, "")