plugin fixes for windows and root level flags (#849)

* enforce .exe extension when running plugin child on windows

* parse flags for cli plugin run cmd but pass raw args to plugin child

* repair plugin_cmds flags test

* fix plugin tests on windows

* make binary extension a public utility

pkg/cmd/plugin_cmds.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"os"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
@@ -28,11 +29,10 @@ func newPluginTemplateCmd(config *config.Config, plugin *plugins.Plugin) *plugin
 	ptc.cfg = config
 
 	ptc.cmd = &cobra.Command{
-		Use:                plugin.Shortname,
-		Short:              plugin.Shortdesc,
-		RunE:               ptc.runPluginCmd,
-		Annotations:        map[string]string{"scope": "plugin"},
-		DisableFlagParsing: true,
+		Use:         plugin.Shortname,
+		Short:       plugin.Shortdesc,
+		RunE:        ptc.runPluginCmd,
+		Annotations: map[string]string{"scope": "plugin"},
 	}
 
 	// override the CLI's help command and let the plugin supply the help text instead
@@ -52,15 +52,16 @@ func (ptc *pluginTemplateCmd) runPluginCmd(cmd *cobra.Command, args []string) er
 		}).Debug("Ctrl+C received, cleaning up...")
 	})
 
-	ptc.ParsedArgs = args
+	ptc.ParsedArgs = os.Args[2:]
+
 	fs := afero.NewOsFs()
 	plugin, err := plugins.LookUpPlugin(ctx, ptc.cfg, fs, cmd.CalledAs())
 
 	if err != nil {
 		return err
 	}
 
-	err = plugin.Run(ctx, ptc.cfg, fs, args)
+	err = plugin.Run(ctx, ptc.cfg, fs, ptc.ParsedArgs)
 	plugins.CleanupAllClients()
 
 	if err != nil {

pkg/cmd/plugin_cmds_test.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"context"
+	"os"
 	"strings"
 	"testing"
 
@@ -29,13 +30,23 @@ func createPluginCmd() *pluginTemplateCmd {
 	return pluginCmd
 }
 
+// TestFlagsArePassedAsArgs ensures that the plugin is passing all args and flags as expected.
+// This is a complex dance between the CLI itself and the plugin, so the flags come from
+// two different sources as a result. This test is here to catch any non-obvious regressions
 func TestFlagsArePassedAsArgs(t *testing.T) {
-	Execute(context.Background())
-
 	pluginCmd := createPluginCmd()
 	rootCmd.AddCommand(pluginCmd.cmd)
-	executeCommandC(rootCmd, "test", "testarg", "--testflag")
 
-	require.Equal(t, len(pluginCmd.ParsedArgs), 2)
-	require.Equal(t, strings.Join(pluginCmd.ParsedArgs, " "), "testarg --testflag")
+	Execute(context.Background())
+
+	// temp override for the os.Args so that the pluginCmd can use them
+	oldArgs := os.Args
+	os.Args = []string{"stripe", "test", "testarg", "--log-level=info"}
+	defer func() { os.Args = oldArgs }()
+
+	rootCmd.SetArgs([]string{"test", "testarg", "--log-level=info"})
+	executeCommandC(rootCmd, "test", "testarg", "--log-level=info")
+
+	require.Equal(t, 2, len(pluginCmd.ParsedArgs))
+	require.Equal(t, "testarg --log-level=info", strings.Join(pluginCmd.ParsedArgs, " "))
 }

pkg/plugins/plugin.go

@@ -224,6 +224,7 @@ func (p *Plugin) downloadAndSavePlugin(config config.IConfig, pluginDownloadURL
 
 	pluginDir := p.getPluginInstallPath(config, version)
 	pluginFilePath := filepath.Join(pluginDir, p.Binary)
+	pluginFilePath += GetBinaryExtension()
 
 	logger.Debugf("installing %s to %s...", p.Shortname, pluginFilePath)
 
@@ -313,6 +314,7 @@ func (p *Plugin) Run(ctx context.Context, config *config.Config, fs afero.Fs, ar
 
 	pluginDir := p.getPluginInstallPath(config, version)
 	pluginBinaryPath := filepath.Join(pluginDir, p.Binary)
+	pluginBinaryPath += GetBinaryExtension()
 
 	cmd := exec.Command(pluginBinaryPath)
 

pkg/plugins/plugin_test.go

@@ -2,6 +2,7 @@ package plugins
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"testing"
 
@@ -27,7 +28,8 @@ func TestInstall(t *testing.T) {
 	plugin, _ := LookUpPlugin(context.Background(), config, fs, "appA")
 	err := plugin.Install(context.Background(), config, fs, "2.0.1", testServers.StripeServer.URL)
 	require.Nil(t, err)
-	fileExists, err := afero.Exists(fs, "/plugins/appA/2.0.1/stripe-cli-app-a")
+	file := fmt.Sprintf("/plugins/appA/2.0.1/stripe-cli-app-a%s", GetBinaryExtension())
+	fileExists, err := afero.Exists(fs, file)
 	require.Nil(t, err)
 	require.True(t, fileExists)
 }
@@ -43,7 +45,8 @@ func TestInstallFailsIfChecksumCouldNotBeFound(t *testing.T) {
 	require.EqualError(t, err, "Could not locate a valid checksum for appA version 0.0.0")
 
 	// Require that we don't save the binary if checkum does not match
-	fileExists, err := afero.Exists(fs, "/plugins/appA/0.0.1/stripe-cli-app-a")
+	file := fmt.Sprintf("/plugins/appA/0.0.0/stripe-cli-app-a%s", GetBinaryExtension())
+	fileExists, err := afero.Exists(fs, file)
 	require.Nil(t, err)
 	require.False(t, fileExists)
 }
@@ -59,7 +62,8 @@ func TestInstallationFailsIfChecksumDoesNotMatch(t *testing.T) {
 	require.EqualError(t, err, "installed plugin 'appB' could not be verified, aborting installation")
 
 	// Require that we don't save the binary if checkum does not match
-	fileExists, err := afero.Exists(fs, "/plugins/appB/1.2.1/stripe-cli-app-b")
+	file := fmt.Sprintf("/plugins/appB/1.2.1/stripe-cli-app-b%s", GetBinaryExtension())
+	fileExists, err := afero.Exists(fs, file)
 	require.Nil(t, err)
 	require.False(t, fileExists)
 }
@@ -74,17 +78,19 @@ func TestInstallCleansOtherVersionsOfPlugin(t *testing.T) {
 	plugin, _ := LookUpPlugin(context.Background(), config, fs, "appA")
 	err := plugin.Install(context.Background(), config, fs, "0.0.1", testServers.StripeServer.URL)
 	require.Nil(t, err)
-	fileExists, _ := afero.Exists(fs, "/plugins/appA/0.0.1/stripe-cli-app-a")
+	file := fmt.Sprintf("/plugins/appA/0.0.1/stripe-cli-app-a%s", GetBinaryExtension())
+	fileExists, _ := afero.Exists(fs, file)
 	require.True(t, fileExists, "Test setup failed -- did not download plugin version 0.0.1")
 
 	// Download valid plugin
 	err = plugin.Install(context.Background(), config, fs, "2.0.1", testServers.StripeServer.URL)
 	require.Nil(t, err)
-	fileExists, _ = afero.Exists(fs, "/plugins/appA/2.0.1/stripe-cli-app-a")
+	newFile := fmt.Sprintf("/plugins/appA/2.0.1/stripe-cli-app-a%s", GetBinaryExtension())
+	fileExists, _ = afero.Exists(fs, newFile)
 	require.True(t, fileExists, "Test setup failed -- did not download plugin version 2.0.1")
 
 	// Require that the older version got removed from the fs
-	fileExists, _ = afero.Exists(fs, "/plugins/appA/0.0.1/stripe-cli-app-a")
+	fileExists, _ = afero.Exists(fs, file)
 	require.False(t, fileExists, "Expected the original version of the plugin to be deleted.")
 }
 
@@ -98,16 +104,18 @@ func TestInstallDoesNotCleanIfInstallFails(t *testing.T) {
 	plugin, _ := LookUpPlugin(context.Background(), config, fs, "appA")
 	err := plugin.Install(context.Background(), config, fs, "2.0.1", testServers.StripeServer.URL)
 	require.Nil(t, err)
-	fileExists, _ := afero.Exists(fs, "/plugins/appA/2.0.1/stripe-cli-app-a")
+	file := fmt.Sprintf("/plugins/appA/2.0.1/stripe-cli-app-a%s", GetBinaryExtension())
+	fileExists, _ := afero.Exists(fs, file)
 	require.True(t, fileExists, "Test setup failed -- did not download valid plugin")
 
 	// Install fails for the same plugin because the checksum could not be found in manifest
 	err = plugin.Install(context.Background(), config, fs, "0.0.0", testServers.StripeServer.URL)
 	require.EqualError(t, err, "Could not locate a valid checksum for appA version 0.0.0")
-	fileExists, _ = afero.Exists(fs, "/plugins/appA/0.0.1/stripe-cli-app-a")
+	failedFile := fmt.Sprintf("/plugins/appA/0.0.0/stripe-cli-app-a%s", GetBinaryExtension())
+	fileExists, _ = afero.Exists(fs, failedFile)
 	require.False(t, fileExists, "Test setup failed -- did not expect plugin to be downloaded")
 
 	// Require that we did not delete the initial version of the plugin
-	fileExists, _ = afero.Exists(fs, "/plugins/appA/2.0.1/stripe-cli-app-a")
+	fileExists, _ = afero.Exists(fs, file)
 	require.True(t, fileExists, "Did not expect the original version of the plugin to be deleted.")
 }

pkg/plugins/utilities.go

@@ -8,6 +8,7 @@ import (
 	"net/http/httptrace"
 	"os"
 	"path/filepath"
+	"runtime"
 
 	log "github.com/sirupsen/logrus"
 
@@ -22,6 +23,15 @@ import (
 	"github.com/stripe/stripe-cli/pkg/stripe"
 )
 
+// GetBinaryExtension returns the appropriate file extension for plugin binary
+func GetBinaryExtension() string {
+	if runtime.GOOS == "windows" {
+		return ".exe"
+	}
+
+	return ""
+}
+
 // getPluginsDir computes where plugins are installed locally
 func getPluginsDir(config config.IConfig) string {
 	var pluginsDir string