Repository: helm/helm
Pull request #11508 (2022-11-04 14:57:04)

improve error message on plugin install
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:

Plugins are commonly downloadable as tar files from github releases.
If a users downloads the archive and tries to install it without extracting helm fails with the following error `Error: plugin metadata (plugin.yaml) missing`. This is not very helpful, as there is a `plugin.yaml` in the archive, helm simply expects a folder instead of an archive. Which led to the issue https://github.com/helm/helm/issues/11272

This PR changes the error message to detect this case and print a (hopefully) more helpful error message: `Error: expected plugin to be a folder`

Additional, helm could just extract the archive. Most of the code for this is already there (plugins installed via http support tar files already). I can contribute this, if you think this would be helpful.

**Special notes for your reviewer**:

**If applicable**:
- [ ] this PR contains documentation
- [x] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility


yxxhero: @phil9909 please fix the ci issue. Thanks very much.

Patch:
diff --git a/pkg/plugin/installer/local_installer.go b/pkg/plugin/installer/local_installer.go
index c92bc3fb0..759df38be 100644
--- a/pkg/plugin/installer/local_installer.go
+++ b/pkg/plugin/installer/local_installer.go
@@ -22,6 +22,9 @@ import (
 	"github.com/pkg/errors"
 )
 
+// ErrPluginNotAFolder indicates that the plugin path is not a folder.
+var ErrPluginNotAFolder = errors.New("expected plugin to be a folder")
+
 // LocalInstaller installs plugins from the filesystem.
 type LocalInstaller struct {
 	base
@@ -43,6 +46,14 @@ func NewLocalInstaller(source string) (*LocalInstaller, error) {
 //
 // Implements Installer.
 func (i *LocalInstaller) Install() error {
+	stat, err := os.Stat(i.Source)
+	if err != nil {
+		return err
+	}
+	if !stat.IsDir() {
+		return ErrPluginNotAFolder
+	}
+
 	if !isPlugin(i.Source) {
 		return ErrMissingMetadata
 	}


Test patch:
diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go
index 9b5cbf59e..710a24537 100644
--- a/pkg/plugin/installer/local_installer_test.go
+++ b/pkg/plugin/installer/local_installer_test.go
@@ -48,3 +48,19 @@ func TestLocalInstaller(t *testing.T) {
 	}
 	defer os.RemoveAll(filepath.Dir(helmpath.DataPath())) // helmpath.DataPath is like /tmp/helm013130971/helm
 }
+
+func TestLocalInstallerNotAFolder(t *testing.T) {
+	source := "../testdata/plugdir/good/echo/plugin.yaml"
+	i, err := NewForSource(source, "")
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	err = Install(i)
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if err != ErrPluginNotAFolder {
+		t.Fatalf("expected error to equal: %q", err)
+	}
+}