diff --git a/internal/plugin/installer/local_installer.go b/internal/plugin/installer/local_installer.go index 59918401b..e32cafe06 100644 --- a/internal/plugin/installer/local_installer.go +++ b/internal/plugin/installer/local_installer.go @@ -39,6 +39,7 @@ type LocalInstaller struct { extractor Extractor pluginData []byte // Cached plugin data provData []byte // Cached provenance data + pluginName string // Cached plugin name } // NewLocalInstaller creates a new LocalInstaller. @@ -115,6 +116,7 @@ func (i *LocalInstaller) installFromArchive() error { if err != nil { return fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) } + i.pluginName = metadata.Name filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) tarballPath := helmpath.DataPath("plugins", filename) if err := os.MkdirAll(filepath.Dir(tarballPath), 0o755); err != nil { @@ -147,10 +149,16 @@ func (i *LocalInstaller) installFromArchive() error { } // Plugin directory should be named after the plugin at the archive root - pluginName := stripPluginName(filepath.Base(i.Source)) - pluginDir := filepath.Join(tempDir, pluginName) + pluginDir := filepath.Join(tempDir, metadata.Name) if _, err = os.Stat(filepath.Join(pluginDir, "plugin.yaml")); err != nil { - return fmt.Errorf("plugin.yaml not found in expected directory %s: %w", pluginDir, err) + // Stay backwards compatible with archives like foo.tgz containing foo/plugin.tgz regardless of actual plugin name + archiveBasedName := stripPluginName(filepath.Base(i.Source)) + fallbackDir := filepath.Join(tempDir, archiveBasedName) + if _, err = os.Stat(filepath.Join(fallbackDir, "plugin.yaml")); err != nil { + return fmt.Errorf("plugin.yaml not found in expected directory %s: %w", pluginDir, err) + } + slog.Warn("plugin is using bad archive path", slog.String("path", archiveBasedName)) + pluginDir = fallbackDir } // Copy to the final destination @@ -170,13 +178,19 @@ func (i *LocalInstaller) Path() string { return "" } - pluginName := filepath.Base(i.Source) - if i.isArchive { - // Strip archive extension to get plugin name - pluginName = stripPluginName(pluginName) - } + return helmpath.DataPath("plugins", i.name()) +} - return helmpath.DataPath("plugins", pluginName) +func (i *LocalInstaller) name() string { + if i.pluginName == "" { + pluginName := filepath.Base(i.Source) + if i.isArchive { + // Strip archive extension to get plugin name + pluginName = stripPluginName(pluginName) + } + i.pluginName = pluginName + } + return i.pluginName } // SupportsVerification returns true if the local installer can verify plugins diff --git a/internal/plugin/installer/local_installer_test.go b/internal/plugin/installer/local_installer_test.go index 94632756c..1ada1a160 100644 --- a/internal/plugin/installer/local_installer_test.go +++ b/internal/plugin/installer/local_installer_test.go @@ -55,57 +55,103 @@ func TestLocalInstallerNotAFolder(t *testing.T) { } func TestLocalInstallerTarball(t *testing.T) { - ensure.HelmHome(t) - - // Create a test tarball - tempDir := t.TempDir() - tarballPath := filepath.Join(tempDir, "test-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 + testCases := []struct { + Name string + Archive string + Files []struct { + Name string + Body string + Mode int64 + } + ExpectedPath string }{ - {"test-plugin/plugin.yaml", "name: test-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", 0o644}, - {"test-plugin/bin/test-plugin", "#!/usr/bin/env sh\necho test", 0o755}, + { + Name: "plugin archive named following the convention", + Archive: "test-plugin-1.0.0.tar.gz", + Files: []struct { + Name string + Body string + Mode int64 + }{ + {"test-plugin/plugin.yaml", "name: test-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", 0o644}, + {"test-plugin/bin/test-plugin", "#!/usr/bin/env sh\necho test", 0o755}, + }, + ExpectedPath: "test-plugin", + }, + { + Name: "plugin archive with unconventional name", + Archive: "unconventional-name-linux-amd64.tar.gz", + Files: []struct { + Name string + Body string + Mode int64 + }{ + {"unconventional-name/plugin.yaml", "name: unconventional-name\napiVersion: v1\ntype: cli/v1\nruntime: subprocess\nversion: 1.0.0\nconfig:\n shortHelp: test\n longHelp: test\nruntimeConfig:\n platformCommand:\n - command: echo", 0o644}, + {"unconventional-name/bin/test-plugin", "#!/usr/bin/env sh\necho test", 0o755}, + }, + ExpectedPath: "unconventional-name", + }, + { + Name: "plugin where archive name matches sub-path", + Archive: "search-here-1.0.0.tar.gz", + Files: []struct { + Name string + Body string + Mode int64 + }{ + {"search-here/plugin.yaml", "name: working-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", 0o644}, + {"search-here/bin/test-plugin", "#!/usr/bin/env sh\necho test", 0o755}, + }, + ExpectedPath: "working-plugin", + }, } - for _, file := range files { - hdr := &tar.Header{ - Name: file.Name, - Mode: file.Mode, - Size: int64(len(file.Body)), - } - require.NoError(t, tw.WriteHeader(hdr)) - _, err := tw.Write([]byte(file.Body)) - require.NoError(t, err) + for _, tt := range testCases { + t.Run(tt.Name, func(t *testing.T) { + ensure.HelmHome(t) + + // Create a test tarball + tempDir := t.TempDir() + tarballPath := filepath.Join(tempDir, tt.Archive) + + // Create tarball content + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + + for _, file := range tt.Files { + hdr := &tar.Header{ + Name: file.Name, + Mode: file.Mode, + Size: int64(len(file.Body)), + } + require.NoError(t, tw.WriteHeader(hdr)) + _, err := tw.Write([]byte(file.Body)) + require.NoError(t, err) + } + + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + + // Write tarball to file + require.NoError(t, os.WriteFile(tarballPath, buf.Bytes(), 0o644)) + + // Test installation + i, err := NewForSource(tarballPath, "") + require.NoError(t, err) + + // Verify it's detected as LocalInstaller + localInstaller, ok := i.(*LocalInstaller) + require.True(t, ok, "expected LocalInstaller") + require.True(t, localInstaller.isArchive, "expected isArchive to be true") + require.NoError(t, Install(i)) + + expectedPath := helmpath.DataPath("plugins", tt.ExpectedPath) + require.Equal(t, expectedPath, i.Path(), "expected path %q, got %q", expectedPath, i.Path()) + + // Verify plugin was installed + _, err = os.Stat(i.Path()) + require.NoErrorf(t, err, "plugin not found at %s", i.Path()) + }) } - - require.NoError(t, tw.Close()) - require.NoError(t, gw.Close()) - - // Write tarball to file - require.NoError(t, os.WriteFile(tarballPath, buf.Bytes(), 0o644)) - - // Test installation - i, err := NewForSource(tarballPath, "") - require.NoError(t, err) - - // Verify it's detected as LocalInstaller - localInstaller, ok := i.(*LocalInstaller) - require.True(t, ok, "expected LocalInstaller") - require.True(t, localInstaller.isArchive, "expected isArchive to be true") - require.NoError(t, Install(i)) - - expectedPath := helmpath.DataPath("plugins", "test-plugin") - require.Equal(t, expectedPath, i.Path(), "expected path %q, got %q", expectedPath, i.Path()) - - // Verify plugin was installed - _, err = os.Stat(i.Path()) - require.NoErrorf(t, err, "plugin not found at %s", i.Path()) }