From 035c6f538e3a1b20f9e3a8823a9b41a0de7fcaec Mon Sep 17 00:00:00 2001 From: Suz Hinton <54524269+suz-stripe@users.noreply.github.com> Date: Tue, 29 Mar 2022 09:36:28 -0700 Subject: [PATCH] Suz stripe/plugins refactor (#844) * support awareness of installed plugins * suppress telemetry for plugin commands plugins will handle sending their own telemetry for now * trace install reqs; don't set opaque URL on redirect * improve plugin install outputs * lint * lint: the sequel * SyncConfig back to pkg level public method * migrate installed plugins getter to profile * migrate IsPluginCommand to plugins pkg --- go.mod | 2 +- go.sum | 4 +- pkg/cmd/plugin/upgrade.go | 3 +- pkg/cmd/plugin_cmds.go | 74 ++++++++++++++++++++++++++++++++++ pkg/cmd/root.go | 50 ++++++++++------------- pkg/config/config.go | 19 +++++++-- pkg/config/profile.go | 10 +++++ pkg/plugins/plugin.go | 48 +++++++++++++++++----- pkg/plugins/test_utils.go | 16 ++------ pkg/plugins/utilities.go | 36 +++++++++++++---- pkg/plugins/utilities_test.go | 12 ++++++ pkg/requests/request_tracer.go | 39 ++++++++++++++++++ 12 files changed, 247 insertions(+), 66 deletions(-) create mode 100644 pkg/cmd/plugin_cmds.go create mode 100644 pkg/requests/request_tracer.go diff --git a/go.mod b/go.mod index 2a1e574b..76115395 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.17 require ( github.com/BurntSushi/toml v0.4.1 - github.com/briandowns/spinner v1.16.0 + github.com/briandowns/spinner v1.17.0 github.com/fatih/color v1.13.0 // indirect github.com/felixge/httpsnoop v1.0.2 // indirect github.com/google/go-github/v28 v28.1.1 diff --git a/go.sum b/go.sum index 6d5d1c53..8231efb5 100644 --- a/go.sum +++ b/go.sum @@ -64,8 +64,8 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bketelsen/crypt v0.0.4/go.mod h1:aI6NrJ0pMGgvZKL1iVgXLnfIFJtfV+bKCoqOes/6LfM= -github.com/briandowns/spinner v1.16.0 h1:DFmp6hEaIx2QXXuqSJmtfSBSAjRmpGiKG6ip2Wm/yOs= -github.com/briandowns/spinner v1.16.0/go.mod h1:QOuQk7x+EaDASo80FEXwlwiA+j/PPIcX3FScO+3/ZPQ= +github.com/briandowns/spinner v1.17.0 h1:7HjHI07APcVZBT71J2UvJl3CAvYCnqqCrxW5gXSDOVA= +github.com/briandowns/spinner v1.17.0/go.mod h1:QOuQk7x+EaDASo80FEXwlwiA+j/PPIcX3FScO+3/ZPQ= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= diff --git a/pkg/cmd/plugin/upgrade.go b/pkg/cmd/plugin/upgrade.go index e1fa119e..11a7aa6b 100644 --- a/pkg/cmd/plugin/upgrade.go +++ b/pkg/cmd/plugin/upgrade.go @@ -59,7 +59,8 @@ func (uc *UpgradeCmd) runUpgradeCmd(cmd *cobra.Command, args []string) error { if err == nil { color := ansi.Color(os.Stdout) - fmt.Println(color.Green("✔ upgrade complete.")) + successMsg := fmt.Sprintf("✔ upgrade to v%s complete.", version) + fmt.Println(color.Green(successMsg)) } return err diff --git a/pkg/cmd/plugin_cmds.go b/pkg/cmd/plugin_cmds.go new file mode 100644 index 00000000..7ee24584 --- /dev/null +++ b/pkg/cmd/plugin_cmds.go @@ -0,0 +1,74 @@ +package cmd + +import ( + "errors" + "fmt" + + log "github.com/sirupsen/logrus" + "github.com/spf13/afero" + "github.com/spf13/cobra" + + "github.com/stripe/stripe-cli/pkg/config" + "github.com/stripe/stripe-cli/pkg/plugins" + "github.com/stripe/stripe-cli/pkg/validators" +) + +type pluginTemplateCmd struct { + cfg *config.Config + cmd *cobra.Command + fs afero.Fs +} + +// newPluginTemplateCmd is a generic plugin command template to dynamically use +// so that we can add any locally installed plugins as supported commands in the CLI +func newPluginTemplateCmd(config *config.Config, plugin *plugins.Plugin) *pluginTemplateCmd { + ptc := &pluginTemplateCmd{} + ptc.fs = afero.NewOsFs() + ptc.cfg = config + + ptc.cmd = &cobra.Command{ + 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 + ptc.cmd.SetHelpCommand(&cobra.Command{ + Use: "no-help", + Hidden: true, + }) + + return ptc +} + +// runPluginCmd hands off to the plugin itself to take over +func (ptc *pluginTemplateCmd) runPluginCmd(cmd *cobra.Command, args []string) error { + ctx := withSIGTERMCancel(cmd.Context(), func() { + log.WithFields(log.Fields{ + "prefix": "cmd.pluginCmd.runPluginCmd", + }).Debug("Ctrl+C received, cleaning up...") + }) + + 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) + plugins.CleanupAllClients() + + if err != nil { + if err == validators.ErrAPIKeyNotConfigured { + return errors.New("Install failed due to API key not configured. Please run `stripe login` or specify the `--api-key`") + } + + log.WithFields(log.Fields{ + "prefix": "pluginTemplateCmd.runPluginCmd", + }).Debug(fmt.Sprintf("Plugin command '%s' exited with error: %s", plugin.Shortname, err)) + } + + return nil +} diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 8472ac91..75684bc3 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -15,7 +15,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" - "github.com/stripe/stripe-cli/pkg/ansi" "github.com/stripe/stripe-cli/pkg/cmd/resource" "github.com/stripe/stripe-cli/pkg/config" "github.com/stripe/stripe-cli/pkg/login" @@ -31,7 +30,6 @@ import ( var Config config.Config var fs = afero.NewOsFs() -var color = ansi.Color(os.Stdout) // rootCmd represents the base command when called without any subcommands var rootCmd = &cobra.Command{ @@ -62,8 +60,11 @@ var rootCmd = &cobra.Command{ telemetryMetadata.SetMerchant(merchant) telemetryMetadata.SetUserAgent(useragent.GetEncodedUserAgent()) - // record command invocation - sendCommandInvocationEvent(cmd.Context()) + // plugins send their own telemetry due to having richer context than the CLI does + if !plugins.IsPluginCommand(cmd) { + // record command invocation + sendCommandInvocationEvent(cmd.Context()) + } }, } @@ -117,31 +118,7 @@ func Execute(ctx context.Context) { } case strings.Contains(errString, "unknown command"): - Config.InitConfig() - // first look for a plugin that matches the unknown command - plugin, err := plugins.LookUpPlugin(ctx, &Config, fs, os.Args[1]) - - if err != nil { - // no matches, show help and exit - showSuggestion() - } else { - // we found a plugin, so run it - err = plugin.Run(updatedCtx, &Config, fs, os.Args[2:]) - // ensure we fully tear down the plugin before exiting the CLI - plugins.CleanupAllClients() - if err != nil { - if err == validators.ErrAPIKeyNotConfigured { - fmt.Println(color.Red("Install failed due to API key not configured. Please run `stripe login` or specify the `--api-key`")) - } else { - log.WithFields(log.Fields{ - "prefix": "plugins.plugin.run", - }).Trace(fmt.Sprintf("Plugin command %s exited with error: %s", plugin.Shortname, err)) - } - os.Exit(1) - } - - os.Exit(0) - } + showSuggestion() default: fmt.Println(err) @@ -205,4 +182,19 @@ func init() { if err != nil { log.Fatal(err) } + + // config is not initialized by cobra at this point, so we need to temporarily initialize it + Config.InitConfig() + + // get a list of installed plugins, validate against the manifest + // and finally add each validated plugin as a command + nfs := afero.NewOsFs() + pluginList := Config.Profile.GetInstalledPlugins() + + for _, p := range pluginList { + plugin, err := plugins.LookUpPlugin(context.Background(), &Config, nfs, p) + if err == nil { + rootCmd.AddCommand(newPluginTemplateCmd(&Config, &plugin).cmd) + } + } } diff --git a/pkg/config/config.go b/pkg/config/config.go index 02de1e4e..1a634bfa 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -39,14 +39,16 @@ type IConfig interface { PrintConfig() error RemoveProfile(profileName string) error RemoveAllProfiles() error + WriteConfigField(field string, value interface{}) error } // Config handles all overall configuration for the CLI type Config struct { - Color string - LogLevel string - Profile Profile - ProfilesFile string + Color string + LogLevel string + Profile Profile + ProfilesFile string + InstalledPlugins []string } // GetProfile returns the Profile of the config @@ -253,6 +255,15 @@ func isProfile(value interface{}) bool { return ok } +// WriteConfigField updates a configuration field and writes the updated +// configuration to disk. +func (c *Config) WriteConfigField(field string, value interface{}) error { + runtimeViper := viper.GetViper() + runtimeViper.Set(field, value) + + return runtimeViper.WriteConfig() +} + // syncConfig merges a runtimeViper instance with the config file being used. func syncConfig(runtimeViper *viper.Viper) error { runtimeViper.MergeInConfig() diff --git a/pkg/config/profile.go b/pkg/config/profile.go index 7f66cdf5..9052eaa3 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -163,6 +163,16 @@ func (p *Profile) GetTerminalPOSDeviceID() string { return "" } +// GetInstalledPlugins returns a list of locally installed plugins. +// This does not vary by profile +func (p *Profile) GetInstalledPlugins() []string { + if err := viper.ReadInConfig(); err == nil { + return viper.GetStringSlice("installed_plugins") + } + + return []string{} +} + // GetConfigField returns the configuration field for the specific profile func (p *Profile) GetConfigField(field string) string { return p.ProfileName + "." + field diff --git a/pkg/plugins/plugin.go b/pkg/plugins/plugin.go index 71caf981..ead9c55e 100644 --- a/pkg/plugins/plugin.go +++ b/pkg/plugins/plugin.go @@ -5,6 +5,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "errors" "fmt" "io" "os" @@ -33,6 +34,7 @@ var ( // Plugin contains the plugin properties type Plugin struct { Shortname string + Shortdesc string Binary string Releases []Release `toml:"Release"` MagicCookieValue string @@ -152,37 +154,65 @@ func (p *Plugin) LookUpLatestVersion() string { } // Install installs the plugin of the given version -func (p *Plugin) Install(ctx context.Context, config config.IConfig, fs afero.Fs, version string, baseURL string) error { +func (p *Plugin) Install(ctx context.Context, cfg config.IConfig, fs afero.Fs, version string, baseURL string) error { spinner := ansi.StartNewSpinner(ansi.Faint(fmt.Sprintf("installing '%s' v%s...", p.Shortname, version)), os.Stdout) - apiKey, err := config.GetProfile().GetAPIKey(false) + apiKey, err := cfg.GetProfile().GetAPIKey(false) if err != nil { ansi.StopSpinner(spinner, ansi.Faint(fmt.Sprintf("could not install plugin '%s': missing API key", p.Shortname)), os.Stdout) return err } - pluginData, err := requests.GetPluginData(ctx, baseURL, stripe.APIVersion, apiKey, config.GetProfile()) + pluginData, err := requests.GetPluginData(ctx, baseURL, stripe.APIVersion, apiKey, cfg.GetProfile()) if err != nil { - ansi.StopSpinner(spinner, ansi.Faint(fmt.Sprintf("could not install plugin '%s': unauthorized", p.Shortname)), os.Stdout) - return err + ansi.StopSpinner(spinner, ansi.Faint(fmt.Sprintf("could not install plugin '%s'", p.Shortname)), os.Stdout) + + log.WithFields(log.Fields{ + "prefix": "plugins.plugin.Install", + }).Debugf("install error: %s", err) + + return errors.New("you don't seem to have access to this plugin") } pluginDownloadURL := fmt.Sprintf("%s/%s/%s/%s/%s/%s", pluginData.PluginBaseURL, p.Shortname, version, runtime.GOOS, runtime.GOARCH, p.Binary) // Pull down bin, verify, and save to disk - err = p.downloadAndSavePlugin(config, pluginDownloadURL, fs, version) + err = p.downloadAndSavePlugin(cfg, pluginDownloadURL, fs, version) if err != nil { - ansi.StopSpinner(spinner, ansi.Faint(fmt.Sprintf("could not install plugin '%s'", p.Shortname)), os.Stdout) + ansi.StopSpinner(spinner, ansi.Faint(fmt.Sprintf("could not install plugin '%s': %s", p.Shortname, err)), os.Stdout) + return err + } + + profile := cfg.GetProfile() + installedList := profile.GetInstalledPlugins() + + // check for plugin already in list (ie. in the case of an upgrade) + isInstalled := false + for _, name := range installedList { + if name == p.Shortname { + isInstalled = true + } + } + + if !isInstalled { + installedList = append(installedList, p.Shortname) + } + + // sync list of installed plugins to file + cfg.WriteConfigField("installed_plugins", installedList) + + if err != nil { + ansi.StopSpinner(spinner, ansi.Faint(fmt.Sprintf("could not install plugin '%s', %s", p.Shortname, err)), os.Stdout) return err } // Once the plugin is successfully downloaded, clean up other versions - p.cleanUpPluginPath(config, fs, version) + p.cleanUpPluginPath(cfg, fs, version) - ansi.StopSpinner(spinner, ansi.Faint(""), os.Stdout) + ansi.StopSpinner(spinner, "", os.Stdout) return nil } diff --git a/pkg/plugins/test_utils.go b/pkg/plugins/test_utils.go index 35f21703..6b63188c 100644 --- a/pkg/plugins/test_utils.go +++ b/pkg/plugins/test_utils.go @@ -14,7 +14,9 @@ import ( ) // TestConfig Implementation out the GetConfigFolder function -type TestConfig struct{} +type TestConfig struct { + config.Config +} // GetProfile returns the Mock Profile func (c *TestConfig) GetProfile() *config.Profile { @@ -31,18 +33,6 @@ func (c *TestConfig) GetConfigFolder(xdgPath string) string { // InitConfig is not implemented func (c *TestConfig) InitConfig() {} -// EditConfig is not implemented -func (c *TestConfig) EditConfig() error { return nil } - -// PrintConfig is not implemented -func (c *TestConfig) PrintConfig() error { return nil } - -// RemoveProfile is not implemented -func (c *TestConfig) RemoveProfile(profileName string) error { return nil } - -// RemoveAllProfiles is not implemented -func (c *TestConfig) RemoveAllProfiles() error { return nil } - // setUpFS Sets up a memMap that contains the manifest func setUpFS() afero.Fs { // test plugin manifest diff --git a/pkg/plugins/utilities.go b/pkg/plugins/utilities.go index 10514bfe..ebf70860 100644 --- a/pkg/plugins/utilities.go +++ b/pkg/plugins/utilities.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/http/httptrace" "os" "path/filepath" @@ -14,6 +15,7 @@ import ( hcplugin "github.com/hashicorp/go-plugin" "github.com/spf13/afero" + "github.com/spf13/cobra" "github.com/stripe/stripe-cli/pkg/config" "github.com/stripe/stripe-cli/pkg/requests" @@ -116,19 +118,25 @@ func RefreshPluginManifest(ctx context.Context, config config.IConfig, fs afero. // FetchRemoteResource returns the remote resource body func FetchRemoteResource(url string) ([]byte, error) { - client := http.Client{ - CheckRedirect: func(r *http.Request, via []*http.Request) error { - r.URL.Opaque = r.URL.Path - return nil - }, - } + t := &requests.TracedTransport{} + + req, err := http.NewRequest("GET", url, nil) - req, err := http.NewRequest(http.MethodGet, url, http.NoBody) if err != nil { return nil, err } + trace := &httptrace.ClientTrace{ + GotConn: t.GotConn, + DNSDone: t.DNSDone, + } + + req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace)) + + client := &http.Client{Transport: t} + resp, err := client.Do(req) + if err != nil { return nil, err } @@ -149,3 +157,17 @@ func CleanupAllClients() { log.Debug("Tearing down plugin before exit") hcplugin.CleanupClients() } + +// IsPluginCommand returns true if the command invoked is for a plugin +// false otherwise +func IsPluginCommand(cmd *cobra.Command) bool { + isPlugin := false + + for key, value := range cmd.Annotations { + if key == "scope" && value == "plugin" { + isPlugin = true + } + } + + return isPlugin +} diff --git a/pkg/plugins/utilities_test.go b/pkg/plugins/utilities_test.go index 75891d9b..0fef0a1c 100644 --- a/pkg/plugins/utilities_test.go +++ b/pkg/plugins/utilities_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/spf13/afero" + "github.com/spf13/cobra" "github.com/stretchr/testify/require" ) @@ -57,3 +58,14 @@ func TestRefreshPluginManifest(t *testing.T) { require.Nil(t, err) require.Equal(t, updatedManifestContent, pluginManifestContent) } + +func TestIsPluginCommand(t *testing.T) { + pluginCmd := &cobra.Command{ + Annotations: map[string]string{"scope": "plugin"}, + } + + notPluginCmd := &cobra.Command{} + + require.True(t, IsPluginCommand(pluginCmd)) + require.False(t, IsPluginCommand(notPluginCmd)) +} diff --git a/pkg/requests/request_tracer.go b/pkg/requests/request_tracer.go new file mode 100644 index 00000000..5d74496b --- /dev/null +++ b/pkg/requests/request_tracer.go @@ -0,0 +1,39 @@ +package requests + +import ( + "net/http" + "net/http/httptrace" + + log "github.com/sirupsen/logrus" +) + +// TracedTransport is an http.RoundTripper that keeps track of the in-flight +// request and implements hooks to report HTTP tracing events +// this is a different RoundTripper implementation to stripe.verboseTransport +// and is not designed for Stripe API requests +type TracedTransport struct { + current *http.Request +} + +// RoundTrip wraps http.DefaultTransport.RoundTrip to keep track +// of the current request +func (t *TracedTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.current = req + return http.DefaultTransport.RoundTrip(req) +} + +// GotConn will trace log each connection for the current request +func (t *TracedTransport) GotConn(connInfo httptrace.GotConnInfo) { + log.WithFields(log.Fields{ + "prefix": "requests.TracedTransport", + "connInfo": connInfo, + }).Tracef("Connection trace for %v: %v", t.current.URL, connInfo) +} + +// DNSDone will trace log each DNS lookup for the current request +func (t *TracedTransport) DNSDone(dnsInfo httptrace.DNSDoneInfo) { + log.WithFields(log.Fields{ + "prefix": "requests.TracedTransport", + "dnsInfo": dnsInfo, + }).Tracef("DNS trace for %v", t.current.URL) +}