From 61a35e05b195ce8ac6339ce15193fea538cb1989 Mon Sep 17 00:00:00 2001 From: Panchajanya Mysarla Date: Fri, 23 Feb 2024 10:34:09 -0600 Subject: [PATCH] Implement global verbose flag to set logger verbosity and if plugin root command already implement a verbose flag pass the flag value down to plugin --- docs/plugindev/style_guide.md | 5 +- pkg/cli/plugin_cmd.go | 98 ++++++++++++++++++++++++- pkg/command/discovery_source_test.go | 2 + pkg/command/root.go | 104 ++++++++++++++++++++++++--- 4 files changed, 197 insertions(+), 12 deletions(-) diff --git a/docs/plugindev/style_guide.md b/docs/plugindev/style_guide.md index 10602f48a..27ef4b535 100644 --- a/docs/plugindev/style_guide.md +++ b/docs/plugindev/style_guide.md @@ -208,8 +208,9 @@ Available input components: ### Feedback principles -Useful for everyone, critical to the usability of screen-readers and automation -Support verbosity flags (`-v`, `--verbose`). +`--verbose` The global --verbose flag is meant to be automatically honoured by plugins when called through the tanzu CLI. + +For this to work plugins are expected to use the `github.com/vmware-tanzu/tanzu-plugin-runtime/log` package. `--format` to define preferred output diff --git a/pkg/cli/plugin_cmd.go b/pkg/cli/plugin_cmd.go index 8fda3b24a..35025d612 100644 --- a/pkg/cli/plugin_cmd.go +++ b/pkg/cli/plugin_cmd.go @@ -75,7 +75,12 @@ func getCmdForPluginEx(p *PluginInfo, cmdGroupName string) *cobra.Command { Use: cmdGroupName, Short: p.Description, RunE: func(cmd *cobra.Command, args []string) error { - runner := NewRunner(p.Name, p.InstallationPath, args) + // Process the args to determine to pass the supported flags + pluginArgs, err := processArgs(p, args) + if err != nil { + return err + } + runner := NewRunner(p.Name, p.InstallationPath, pluginArgs) ctx := context.Background() setupPluginEnv() return runner.Run(ctx) @@ -148,6 +153,97 @@ func getCmdForPluginEx(p *PluginInfo, cmdGroupName string) *cobra.Command { return cmd } +// processArgs retrieves all plugin flags and filters out the verbose flag if the plugin doesn't support it. +// It returns a slice of known arguments and an error if any occurred during the process. +func processArgs(p *PluginInfo, args []string) (known []string, err error) { + // Retrieve all plugin flags + pluginFlags, err := getPluginFlags(p) + if err != nil { + // If an error occurs while retrieving the plugin flags, return the error. + return known, err + } + + // Iterate over all the arguments. + for i := 0; i < len(args); i++ { + // If the current argument is the verbose flag... + if args[i] == "--verbose" { + // ...check if the plugin supports the verbose flag. + if _, ok := pluginFlags[args[i]]; ok { + // If the plugin supports the verbose flag, add it to the known arguments. + known = append(known, args[i]) + i++ + // If there are more arguments, add the flag value to the known arguments. + if i < len(args) { + known = append(known, args[i]) + } + } else { + // If the plugin does not support the verbose flag, skip it. + i++ + } + } else { + // If the current argument is not the verbose flag, add it to the known arguments. + known = append(known, args[i]) + } + } + + // Return the known arguments and any error that occurred. + return known, err +} + +// getPluginFlags parses the plugin's help command and returns all supported flags. +// It returns a map where the keys are the supported flags and the values are all set to true. +func getPluginFlags(plugin *PluginInfo) (map[string]bool, error) { + // Create a new runner with the plugin's name, installation path, and the help command. + runner := NewRunner(plugin.Name, plugin.InstallationPath, []string{"-h"}) + ctx := context.Background() + + // Run the help command and capture the output. + stdout, _, err := runner.RunOutput(ctx) + if err != nil { + // If an error occurs while running the help command, return the error. + return nil, err + } + + // Split the output into lines. + lines := strings.Split(stdout, "\n") + + // Initialize a boolean to track when to start capturing flags. + start := false + + // Initialize a map to store the flags. + flags := make(map[string]bool) + + // Iterate over each line of the output. + for _, line := range lines { + // If the line starts with "Flags:", start capturing flags on the next line. + if strings.HasPrefix(line, "Flags:") { + start = true + continue + } + + // If the line starts with "Use", stop capturing flags. + if start && strings.HasPrefix(line, "Use") { + break + } + + // If capturing flags, split the line into parts. + if start { + parts := strings.Fields(line) + for i := 0; i < len(parts); i++ { + // Trim any trailing commas from each part. + part := strings.Trim(parts[i], ",") + // If the part is a flag (starts with "-") and is not just "-", add it to the flags map. + if strings.HasPrefix(part, "-") && part != "-" { + flags[part] = true + } + } + } + } + + // Return the flags map and any error that occurred. + return flags, nil +} + // getHelpArguments extracts the command line to pass along to help calls. // The help function is only ever called for help commands in the format of // "tanzu help cmd", so we can assume anything two after "help" should get diff --git a/pkg/command/discovery_source_test.go b/pkg/command/discovery_source_test.go index 38db5955b..5aa90883e 100644 --- a/pkg/command/discovery_source_test.go +++ b/pkg/command/discovery_source_test.go @@ -145,6 +145,8 @@ func Test_createAndListDiscoverySources(t *testing.T) { testSource2 := "localhost:9876/tanzu-cli/plugins/sandbox1:small" os.Setenv(constants.ConfigVariableAdditionalDiscoveryForTesting, testSource1+","+testSource2) + rootCmd, err = NewRootCmd() + assert.Nil(err) rootCmd.SetArgs([]string{"plugin", "source", "list"}) b = bytes.NewBufferString("") rootCmd.SetOut(b) diff --git a/pkg/command/root.go b/pkg/command/root.go index 87243dc0d..44f1d43d1 100644 --- a/pkg/command/root.go +++ b/pkg/command/root.go @@ -33,6 +33,9 @@ import ( "github.com/vmware-tanzu/tanzu-plugin-runtime/plugin" ) +// verbose flag to set the log level +var verbose int + // NewRootCmd creates a root command. func NewRootCmd() (*cobra.Command, error) { //nolint: gocyclo,funlen var rootCmd = newRootCmd() @@ -42,6 +45,9 @@ func NewRootCmd() (*cobra.Command, error) { //nolint: gocyclo,funlen // Configure defined environment variables found in the config file cliconfig.ConfigureEnvVariables() + // Initialize the global verbose flag to set the log level verbosity (0 - 9) + rootCmd.PersistentFlags().IntVar(&verbose, "verbose", 0, "number for the log level verbosity (0 - 9)") + rootCmd.AddCommand( newVersionCmd(), newPluginCmd(), @@ -58,6 +64,7 @@ func NewRootCmd() (*cobra.Command, error) { //nolint: gocyclo,funlen newCEIPParticipationCmd(), newGenAllDocsCmd(), ) + if _, err := ensureCLIInstanceID(); err != nil { return nil, errors.Wrap(err, "failed to ensure CLI ID") } @@ -240,8 +247,42 @@ func newRootCmd() *cobra.Command { // silencing usage for now as we are getting double usage from plugins on errors SilenceUsage: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - // Sets the verbosity of the logger if TANZU_CLI_LOG_LEVEL is set - setLoggerVerbosity() + // Validate the verbose flag + if verbose < 0 || verbose > 9 { + return errors.New("Invalid value for verbose flag. It should be between 0 and 9") + } + + // For CLI commands: Default the verbose value to -1 If the flag is not set by user + // For Plugin commands: This won't be executed since flags are not parsed by cli but send to plugin as args + if !cmd.Flags().Changed("verbose") { + verbose = -1 + } + + /** + Manually parse the flags when plugin command is triggered + 1. For CLI commands; all flags have been parsed and removed from the list of args and this loop will not run + 2. For Plugin commands; Since CLI doesn't parse args and sends all args to plugin this loop will run to verify if verbose flag is set by the user and then passes to plugin. + */ + if cmd.DisableFlagParsing { + err := determineVerbosityFromPluginCommandArgs(args) + if err != nil { + return err + } + } + + // Sets the verbosity of the logger + setLoggerVerbosity(verbose) + + //TODO: Remove test logs + + log.Info("I am level default") + log.V(0).Info("I am level 0") + log.V(1).Info("I am level 1") + log.V(2).Info("I am level 2") + log.V(3).Info("I am level 3") + log.V(4).Info("I am level 4") + log.V(5).Info("I am level 5") + log.V(6).Info("I am level 6") // Ensure mutual exclusion in current contexts just in case if any plugins with old // plugin-runtime sets k8s context as current when tanzu context is already set as current @@ -290,14 +331,59 @@ func newRootCmd() *cobra.Command { return rootCmd } +// determineVerbosityFromPluginCommandArgs processes the command line arguments for plugin commands. +// It specifically looks for the '--verbose' flag and validates its value. +// The function returns an error if the verbose flag is missing its value or the value is not an integer between 0 and 9. +func determineVerbosityFromPluginCommandArgs(args []string) error { + // Iterate over all command line arguments. + for i := 0; i < len(args); i++ { + // If the current argument is the verbose flag... + if args[i] == "--verbose" { + // ...check if there is a next argument that could be the value for the verbose flag. + if i+1 < len(args) { + nextArg := args[i+1] + // If the next argument is another flag (starts with '-' or '--'), return an error because the verbose flag is missing its value. + if strings.HasPrefix(nextArg, "-") || strings.HasPrefix(nextArg, "--") { + return errors.New("Missing value for verbose flag") + } + // Try to convert the next argument to an integer. + if v, err := strconv.Atoi(nextArg); err == nil { + // If the conversion is successful but the value is not between 0 and 9, return an error. + if v < 0 || v > 9 { + return errors.New("Invalid value for verbose flag. It should be between 0 and 9") + } + // If the value is valid, set the global verbose variable to this value. + verbose = v + } + // Skip the next argument because it has been processed as the value for the verbose flag. + i++ + } else { + // If there is no next argument, return an error because the verbose flag is missing its value. + return errors.New("Missing value for verbose flag") + } + } + } + // If all arguments have been processed without errors, return nil. + return nil +} + // setLoggerVerbosity sets the verbosity of the logger if TANZU_CLI_LOG_LEVEL is set -func setLoggerVerbosity() { - // Configure the log level if env variable TANZU_CLI_LOG_LEVEL is set - logLevel := os.Getenv(log.EnvTanzuCLILogLevel) - if logLevel != "" { - logValue, err := strconv.ParseInt(logLevel, 10, 32) - if err == nil { - log.SetVerbosity(int32(logValue)) +func setLoggerVerbosity(verbosity int) { + // If verbose global flag is passed set the log level verbosity if not then check if env variable TANZU_CLI_LOG_LEVEL is set + if verbosity >= 0 { + // Set the log level verbosity with the verbosity value + log.SetVerbosity(int32(verbosity)) + + // Set the TANZU_CLI_LOG_LEVEL env with the verbosity value + _ = os.Setenv(log.EnvTanzuCLILogLevel, strconv.Itoa(verbosity)) + } else { + // Configure the log level if env variable TANZU_CLI_LOG_LEVEL is set + logLevel := os.Getenv(log.EnvTanzuCLILogLevel) + if logLevel != "" { + logValue, err := strconv.ParseInt(logLevel, 10, 32) + if err == nil { + log.SetVerbosity(int32(logValue)) + } } } }