Merge pull request #10913 from sureshdsk/pluginbug

fix: plugin does not load when helm base dir contains space

cmd/helm/load_plugins.go

@@ -128,7 +128,8 @@ func callPluginExecutable(pluginName string, main string, argv []string, out io.
 		env = append(env, fmt.Sprintf("%s=%s", k, v))
 	}
 
-	prog := exec.Command(main, argv...)
+	mainCmdExp := os.ExpandEnv(main)
+	prog := exec.Command(mainCmdExp, argv...)
 	prog.Env = env
 	prog.Stdin = os.Stdin
 	prog.Stdout = out

cmd/helm/plugin_test.go

@@ -161,6 +161,81 @@ func TestLoadPlugins(t *testing.T) {
 	}
 }
 
+func TestLoadPluginsWithSpace(t *testing.T) {
+	settings.PluginsDirectory = "testdata/helm home with space/helm/plugins"
+	settings.RepositoryConfig = "testdata/helm home with space/helm/repositories.yaml"
+	settings.RepositoryCache = "testdata/helm home with space/helm/repository"
+
+	var (
+		out bytes.Buffer
+		cmd cobra.Command
+	)
+	loadPlugins(&cmd, &out)
+
+	envs := strings.Join([]string{
+		"fullenv",
+		"testdata/helm home with space/helm/plugins/fullenv",
+		"testdata/helm home with space/helm/plugins",
+		"testdata/helm home with space/helm/repositories.yaml",
+		"testdata/helm home with space/helm/repository",
+		os.Args[0],
+	}, "\n")
+
+	// Test that the YAML file was correctly converted to a command.
+	tests := []struct {
+		use    string
+		short  string
+		long   string
+		expect string
+		args   []string
+		code   int
+	}{
+		{"fullenv", "show env vars", "show all env vars", envs + "\n", []string{}, 0},
+	}
+
+	plugins := cmd.Commands()
+
+	if len(plugins) != len(tests) {
+		t.Fatalf("Expected %d plugins, got %d", len(tests), len(plugins))
+	}
+
+	for i := 0; i < len(plugins); i++ {
+		out.Reset()
+		tt := tests[i]
+		pp := plugins[i]
+		if pp.Use != tt.use {
+			t.Errorf("%d: Expected Use=%q, got %q", i, tt.use, pp.Use)
+		}
+		if pp.Short != tt.short {
+			t.Errorf("%d: Expected Use=%q, got %q", i, tt.short, pp.Short)
+		}
+		if pp.Long != tt.long {
+			t.Errorf("%d: Expected Use=%q, got %q", i, tt.long, pp.Long)
+		}
+
+		// Currently, plugins assume a Linux subsystem. Skip the execution
+		// tests until this is fixed
+		if runtime.GOOS != "windows" {
+			if err := pp.RunE(pp, tt.args); err != nil {
+				if tt.code > 0 {
+					perr, ok := err.(pluginError)
+					if !ok {
+						t.Errorf("Expected %s to return pluginError: got %v(%T)", tt.use, err, err)
+					}
+					if perr.code != tt.code {
+						t.Errorf("Expected %s to return %d: got %d", tt.use, tt.code, perr.code)
+					}
+				} else {
+					t.Errorf("Error running %s: %+v", tt.use, err)
+				}
+			}
+			if out.String() != tt.expect {
+				t.Errorf("Expected %s to output:\n%s\ngot\n%s", tt.use, tt.expect, out.String())
+			}
+		}
+	}
+}
+
 type staticCompletionDetails struct {
 	use       string
 	validArgs []string

cmd/helm/testdata/helm home with space/helm/plugins/fullenv/completion.yaml

@@ -0,0 +1,19 @@
+name: wrongname
+commands:
+  - name: empty
+  - name: full
+    commands:
+    - name: more
+      validArgs:
+      - one
+      - two
+      flags:
+      - b
+      - ball
+    - name: less
+      flags:
+      - a
+      - all
+flags:
+- z
+- q

cmd/helm/testdata/helm home with space/helm/plugins/fullenv/fullenv.sh

@@ -0,0 +1,7 @@
+#!/bin/sh
+echo $HELM_PLUGIN_NAME
+echo $HELM_PLUGIN_DIR
+echo $HELM_PLUGINS
+echo $HELM_REPOSITORY_CONFIG
+echo $HELM_REPOSITORY_CACHE
+echo $HELM_BIN

cmd/helm/testdata/helm home with space/helm/plugins/fullenv/plugin.yaml

@@ -0,0 +1,4 @@
+name: fullenv
+usage: "show env vars"
+description: "show all env vars"
+command: "$HELM_PLUGIN_DIR/fullenv.sh"

cmd/helm/testdata/helm home with space/helm/repositories.yaml

@@ -0,0 +1,6 @@
+apiVersion: v1
+generated: 2016-10-03T16:03:10.640376913-06:00
+repositories:
+  - cache: testing-index.yaml
+    name: testing
+    url: http://example.com/charts

cmd/helm/testdata/helm home with space/helm/repository/test-name-index.yaml

@@ -0,0 +1,3 @@
+apiVersion: v1
+entries: {}
+generated: "2020-09-09T19:50:50.198347916-04:00"

cmd/helm/testdata/helm home with space/helm/repository/testing-index.yaml

@@ -0,0 +1,66 @@
+apiVersion: v1
+entries:
+  alpine:
+    - name: alpine
+      url: https://charts.helm.sh/stable/alpine-0.1.0.tgz
+      checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
+      created: "2018-06-27T10:00:18.230700509Z"
+      deprecated: true
+      home: https://helm.sh/helm
+      sources:
+        - https://github.com/helm/helm
+      version: 0.1.0
+      appVersion: 1.2.3
+      description: Deploy a basic Alpine Linux pod
+      keywords: []
+      maintainers: []
+      icon: ""
+      apiVersion: v2
+    - name: alpine
+      url: https://charts.helm.sh/stable/alpine-0.2.0.tgz
+      checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
+      created: "2018-07-09T11:34:37.797864902Z"
+      home: https://helm.sh/helm
+      sources:
+        - https://github.com/helm/helm
+      version: 0.2.0
+      appVersion: 2.3.4
+      description: Deploy a basic Alpine Linux pod
+      keywords: []
+      maintainers: []
+      icon: ""
+      apiVersion: v2
+    - name: alpine
+      url: https://charts.helm.sh/stable/alpine-0.3.0-rc.1.tgz
+      checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
+      created: "2020-11-12T08:44:58.872726222Z"
+      home: https://helm.sh/helm
+      sources:
+        - https://github.com/helm/helm
+      version: 0.3.0-rc.1
+      appVersion: 3.0.0
+      description: Deploy a basic Alpine Linux pod
+      keywords: []
+      maintainers: []
+      icon: ""
+      apiVersion: v2
+  mariadb:
+    - name: mariadb
+      url: https://charts.helm.sh/stable/mariadb-0.3.0.tgz
+      checksum: 65229f6de44a2be9f215d11dbff311673fc8ba56
+      created: "2018-04-23T08:20:27.160959131Z"
+      home: https://mariadb.org
+      sources:
+        - https://github.com/bitnami/bitnami-docker-mariadb
+      version: 0.3.0
+      description: Chart for MariaDB
+      keywords:
+        - mariadb
+        - mysql
+        - database
+        - sql
+      maintainers:
+        - name: Bitnami
+          email: containers@bitnami.com
+      icon: ""
+      apiVersion: v2

pkg/plugin/plugin.go

@@ -121,10 +121,10 @@ func getPlatformCommand(cmds []PlatformCommand) []string {
 	eq := strings.EqualFold
 	for _, c := range cmds {
 		if eq(c.OperatingSystem, runtime.GOOS) {
-			command = strings.Split(os.ExpandEnv(c.Command), " ")
+			command = strings.Split(c.Command, " ")
 		}
 		if eq(c.OperatingSystem, runtime.GOOS) && eq(c.Architecture, runtime.GOARCH) {
-			return strings.Split(os.ExpandEnv(c.Command), " ")
+			return strings.Split(c.Command, " ")
 		}
 	}
 	return command
@@ -148,16 +148,19 @@ func (p *Plugin) PrepareCommand(extraArgs []string) (string, []string, error) {
 		parts = getPlatformCommand(p.Metadata.PlatformCommand)
 	}
 	if platCmdLen == 0 || parts == nil {
-		parts = strings.Split(os.ExpandEnv(p.Metadata.Command), " ")
+		parts = strings.Split(p.Metadata.Command, " ")
 	}
 	if len(parts) == 0 || parts[0] == "" {
 		return "", nil, fmt.Errorf("no plugin command is applicable")
 	}
 
-	main := parts[0]
+	main := os.ExpandEnv(parts[0])
 	baseArgs := []string{}
 	if len(parts) > 1 {
-		baseArgs = parts[1:]
+		for _, cmdpart := range parts[1:] {
+			cmdexp := os.ExpandEnv(cmdpart)
+			baseArgs = append(baseArgs, cmdexp)
+		}
 	}
 	if !p.Metadata.IgnoreFlags {
 		baseArgs = append(baseArgs, extraArgs...)

pkg/plugin/plugin_test.go

@@ -329,6 +329,26 @@ func TestSetupEnv(t *testing.T) {
 	}
 }
 
+func TestSetupEnvWithSpace(t *testing.T) {
+	name := "sureshdsk"
+	base := filepath.Join("testdata/helm home/helm/plugins", name)
+
+	s := cli.New()
+	s.PluginsDirectory = "testdata/helm home/helm/plugins"
+
+	SetupPluginEnv(s, name, base)
+	for _, tt := range []struct {
+		name, expect string
+	}{
+		{"HELM_PLUGIN_NAME", name},
+		{"HELM_PLUGIN_DIR", base},
+	} {
+		if got := os.Getenv(tt.name); got != tt.expect {
+			t.Errorf("Expected $%s=%q, got %q", tt.name, tt.expect, got)
+		}
+	}
+}
+
 func TestValidatePluginData(t *testing.T) {
 	for i, item := range []struct {
 		pass bool