From 664588a830506b46647270f292c1d57e7049104f Mon Sep 17 00:00:00 2001 From: Erik Ostien Date: Tue, 29 Oct 2024 10:58:29 +0100 Subject: [PATCH 1/4] PDI-2056: Update Output and Add System Errors - Modify the output.go to have multiple functions for differing actions, and add the notion of system errors that are not the user's fault, but rather a bug to be reported. - Update all output usages. --- cmd/root.go | 75 ++------ .../commands/config/add_profile_internal.go | 15 +- internal/commands/config/config_internal.go | 10 +- .../config/delete_profile_internal.go | 15 +- internal/commands/config/get_internal.go | 5 +- .../commands/config/list_profiles_internal.go | 5 +- .../config/set_active_profile_internal.go | 10 +- internal/commands/config/set_internal.go | 11 +- internal/commands/config/unset_internal.go | 11 +- .../commands/config/view_profile_internal.go | 5 +- .../commands/feedback/feedback_internal.go | 5 +- internal/commands/platform/export_internal.go | 25 +-- internal/commands/request/request_internal.go | 28 +-- internal/output/output.go | 172 ++++++++---------- main.go | 7 +- 15 files changed, 123 insertions(+), 276 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index c1fa9ba5..d2d58dfa 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -63,11 +63,7 @@ func initViperProfile() { cfgFile, err := profiles.GetOptionValue(options.RootConfigOption) if err != nil { - output.Print(output.Opts{ - Message: "Failed to get configuration file location", - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.SystemError(fmt.Sprintf("Failed to get configuration file location: %v", err), nil) } l.Debug().Msgf("Using configuration file location for initialization: %s", cfgFile) @@ -82,19 +78,11 @@ func initViperProfile() { userDefinedProfile, err := profiles.GetOptionValue(options.RootProfileOption) if err != nil { - output.Print(output.Opts{ - Message: "Failed to get user-defined profile", - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.SystemError(fmt.Sprintf("Failed to get user-defined profile: %v", err), nil) } configFileActiveProfile, err := profiles.GetOptionValue(options.RootActiveProfileOption) if err != nil { - output.Print(output.Opts{ - Message: "Failed to get active profile", - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.SystemError(fmt.Sprintf("Failed to get active profile from configuration file: %v", err), nil) } if userDefinedProfile != "" { @@ -105,20 +93,12 @@ func initViperProfile() { // Configure the profile viper instance if err := profiles.GetMainConfig().ChangeActiveProfile(configFileActiveProfile); err != nil { - output.Print(output.Opts{ - Message: "Failed to set active profile viper", - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.UserFatal(fmt.Sprintf("Failed to set active profile: %v", err), nil) } // Validate the configuration if err := profiles.Validate(); err != nil { - output.Print(output.Opts{ - Message: "Failed to validate Ping CLI configuration", - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.UserFatal(fmt.Sprintf("Failed to validate Ping CLI configuration: %v", err), nil) } } @@ -128,43 +108,25 @@ func checkCfgFileLocation(cfgFile string) { if os.IsNotExist(err) { // Only create a new configuration file if it is the default configuration file location if cfgFile == options.RootConfigOption.DefaultValue.String() { - output.Print(output.Opts{ - Message: fmt.Sprintf("Ping CLI configuration file '%s' does not exist.", cfgFile), - Result: output.ENUM_RESULT_NOACTION_WARN, - }) + output.Warn(fmt.Sprintf("Ping CLI configuration file '%s' does not exist.", cfgFile), nil) createConfigFile(options.RootConfigOption.DefaultValue.String()) } else { - output.Print(output.Opts{ - Message: fmt.Sprintf("Configuration file '%s' does not exist.", cfgFile), - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: fmt.Sprintf("Configuration file '%s' does not exist. Use the default configuration file location or specify a valid configuration file location with the --config flag.", cfgFile), - }) + output.UserFatal(fmt.Sprintf("Configuration file '%s' does not exist. Use the default configuration file location or specify a valid configuration file location with the --config flag.", cfgFile), nil) } } else if err != nil { - output.Print(output.Opts{ - Message: fmt.Sprintf("Failed to check if configuration file '%s' exists", cfgFile), - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.SystemError(fmt.Sprintf("Failed to check if configuration file '%s' exists: %v", cfgFile, err), nil) } } func createConfigFile(cfgFile string) { - output.Print(output.Opts{ - Message: fmt.Sprintf("Creating new Ping CLI configuration file at: %s", cfgFile), - Result: output.ENUM_RESULT_NIL, - }) + output.Message(fmt.Sprintf("Creating new Ping CLI configuration file at: %s", cfgFile), nil) // MkdirAll does nothing if directories already exist. Create needed directories for config file location. err := os.MkdirAll(filepath.Dir(cfgFile), os.ModePerm) if err != nil { - output.Print(output.Opts{ - Message: fmt.Sprintf("Failed to make directories needed for new Ping CLI configuration file: %s", cfgFile), - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.SystemError(fmt.Sprintf("Failed to make the directory for the new configuration file '%s': %v", cfgFile, err), nil) } tempViper := viper.New() @@ -173,11 +135,7 @@ func createConfigFile(cfgFile string) { err = tempViper.WriteConfigAs(cfgFile) if err != nil { - output.Print(output.Opts{ - Message: fmt.Sprintf("Failed to create configuration file at: %s", cfgFile), - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.SystemError(fmt.Sprintf("Failed to create configuration file '%s': %v", cfgFile, err), nil) } } @@ -206,10 +164,7 @@ func initMainViper(cfgFile string) { } err := profiles.GetMainConfig().SaveProfile(pName, subViper) if err != nil { - output.Print(output.Opts{ - Message: fmt.Sprintf("Error: Failed to save profile %s.", pName), - Result: output.ENUM_RESULT_FAILURE, - }) + output.SystemError(fmt.Sprintf("Failed to save profile '%s': %v", pName, err), nil) } } } @@ -223,11 +178,7 @@ func loadMainViperConfig(cfgFile string) { // If a config file is found, read it in. if err := mainViper.ReadInConfig(); err != nil { - output.Print(output.Opts{ - Message: fmt.Sprintf("Failed to read configuration from file: %s", cfgFile), - Result: output.ENUM_RESULT_FAILURE, - FatalMessage: err.Error(), - }) + output.SystemError(fmt.Sprintf("Failed to read configuration from file '%s': %v", cfgFile, err), nil) } else { l.Info().Msgf("Using configuration file: %s", mainViper.ConfigFileUsed()) } diff --git a/internal/commands/config/add_profile_internal.go b/internal/commands/config/add_profile_internal.go index f2523dcc..7d2ecb33 100644 --- a/internal/commands/config/add_profile_internal.go +++ b/internal/commands/config/add_profile_internal.go @@ -23,10 +23,7 @@ func RunInternalConfigAddProfile(rc io.ReadCloser) (err error) { return fmt.Errorf("failed to add profile: %v", err) } - output.Print(output.Opts{ - Message: fmt.Sprintf("Adding new profile '%s'...", newProfileName), - Result: output.ENUM_RESULT_NIL, - }) + output.Message(fmt.Sprintf("Adding new profile '%s'...", newProfileName), nil) subViper := viper.New() subViper.Set(options.ProfileDescriptionOption.ViperKey, newDescription) @@ -35,20 +32,14 @@ func RunInternalConfigAddProfile(rc io.ReadCloser) (err error) { return fmt.Errorf("failed to add profile: %v", err) } - output.Print(output.Opts{ - Message: fmt.Sprintf("Profile created. Update additional profile attributes via 'pingcli config set' or directly within the config file at '%s'", profiles.GetMainConfig().ViperInstance().ConfigFileUsed()), - Result: output.ENUM_RESULT_SUCCESS, - }) + output.Success(fmt.Sprintf("Profile created. Update additional profile attributes via 'pingcli config set' or directly within the config file at '%s'", profiles.GetMainConfig().ViperInstance().ConfigFileUsed()), nil) if setActive { if err = profiles.GetMainConfig().ChangeActiveProfile(newProfileName); err != nil { return fmt.Errorf("failed to set active profile: %v", err) } - output.Print(output.Opts{ - Message: fmt.Sprintf("Profile '%s' set as active.", newProfileName), - Result: output.ENUM_RESULT_SUCCESS, - }) + output.Success(fmt.Sprintf("Profile '%s' set as active.", newProfileName), nil) } return nil diff --git a/internal/commands/config/config_internal.go b/internal/commands/config/config_internal.go index 9d7bf2f2..7bf6c450 100644 --- a/internal/commands/config/config_internal.go +++ b/internal/commands/config/config_internal.go @@ -16,10 +16,7 @@ func RunInternalConfig(rc io.ReadCloser) (err error) { return fmt.Errorf("failed to update profile. %v", err) } - output.Print(output.Opts{ - Message: fmt.Sprintf("Updating profile '%s'...", profileName), - Result: output.ENUM_RESULT_NIL, - }) + output.Message(fmt.Sprintf("Updating profile '%s'...", profileName), nil) if err = profiles.GetMainConfig().ChangeProfileName(profileName, newProfileName); err != nil { return fmt.Errorf("failed to update profile '%s' name to: %s. %v", profileName, newProfileName, err) @@ -29,10 +26,7 @@ func RunInternalConfig(rc io.ReadCloser) (err error) { return fmt.Errorf("failed to update profile '%s' description to: %s. %v", newProfileName, newDescription, err) } - output.Print(output.Opts{ - Message: fmt.Sprintf("Profile updated. Update additional profile attributes via 'pingcli config set' or directly within the config file at '%s'", profiles.GetMainConfig().ViperInstance().ConfigFileUsed()), - Result: output.ENUM_RESULT_SUCCESS, - }) + output.Success(fmt.Sprintf("Profile updated. Update additional profile attributes via 'pingcli config set' or directly within the config file at '%s'", profiles.GetMainConfig().ViperInstance().ConfigFileUsed()), nil) return nil } diff --git a/internal/commands/config/delete_profile_internal.go b/internal/commands/config/delete_profile_internal.go index ee6073e8..6573474c 100644 --- a/internal/commands/config/delete_profile_internal.go +++ b/internal/commands/config/delete_profile_internal.go @@ -27,10 +27,7 @@ func RunInternalConfigDeleteProfile(args []string, rc io.ReadCloser) (err error) } if !confirmed { - output.Print(output.Opts{ - Message: "Profile deletion cancelled.", - Result: output.ENUM_RESULT_NIL, - }) + output.Message("Profile deletion cancelled.", nil) return nil } @@ -70,19 +67,13 @@ func promptUserToConfirmDelete(pName string, rc io.ReadCloser) (confirmed bool, } func deleteProfile(pName string) (err error) { - output.Print(output.Opts{ - Message: fmt.Sprintf("Deleting profile '%s'...", pName), - Result: output.ENUM_RESULT_NIL, - }) + output.Message(fmt.Sprintf("Deleting profile '%s'...", pName), nil) if err = profiles.GetMainConfig().DeleteProfile(pName); err != nil { return err } - output.Print(output.Opts{ - Message: fmt.Sprintf("Profile '%s' deleted.", pName), - Result: output.ENUM_RESULT_SUCCESS, - }) + output.Success(fmt.Sprintf("Profile '%s' deleted.", pName), nil) return nil } diff --git a/internal/commands/config/get_internal.go b/internal/commands/config/get_internal.go index 6ee2beda..c6412c8c 100644 --- a/internal/commands/config/get_internal.go +++ b/internal/commands/config/get_internal.go @@ -24,10 +24,7 @@ func RunInternalConfigGet(viperKey string) (err error) { return fmt.Errorf("failed to get configuration: %v", err) } - output.Print(output.Opts{ - Message: yamlStr, - Result: output.ENUM_RESULT_NIL, - }) + output.Message(yamlStr, nil) return nil } diff --git a/internal/commands/config/list_profiles_internal.go b/internal/commands/config/list_profiles_internal.go index a8aa8c01..eddfafcc 100644 --- a/internal/commands/config/list_profiles_internal.go +++ b/internal/commands/config/list_profiles_internal.go @@ -37,8 +37,5 @@ func RunInternalConfigListProfiles() { } } - output.Print(output.Opts{ - Message: listStr, - Result: output.ENUM_RESULT_NIL, - }) + output.Message(listStr, nil) } diff --git a/internal/commands/config/set_active_profile_internal.go b/internal/commands/config/set_active_profile_internal.go index 45f4f33e..af590197 100644 --- a/internal/commands/config/set_active_profile_internal.go +++ b/internal/commands/config/set_active_profile_internal.go @@ -20,19 +20,13 @@ func RunInternalConfigSetActiveProfile(args []string, rc io.ReadCloser) (err err } } - output.Print(output.Opts{ - Message: fmt.Sprintf("Setting active profile to '%s'...", pName), - Result: output.ENUM_RESULT_NIL, - }) + output.Message(fmt.Sprintf("Setting active profile to '%s'...", pName), nil) if err = profiles.GetMainConfig().ChangeActiveProfile(pName); err != nil { return fmt.Errorf("failed to set active profile: %v", err) } - output.Print(output.Opts{ - Message: fmt.Sprintf("Active profile set to '%s'", pName), - Result: output.ENUM_RESULT_SUCCESS, - }) + output.Success(fmt.Sprintf("Active profile set to '%s'", pName), nil) return nil } diff --git a/internal/commands/config/set_internal.go b/internal/commands/config/set_internal.go index 39b35948..f32feaa3 100644 --- a/internal/commands/config/set_internal.go +++ b/internal/commands/config/set_internal.go @@ -51,15 +51,8 @@ func RunInternalConfigSet(kvPair string) (err error) { return fmt.Errorf("failed to set configuration: %v", err) } - output.Print(output.Opts{ - Message: "Configuration set successfully", - Result: output.ENUM_RESULT_SUCCESS, - }) - - output.Print(output.Opts{ - Message: yamlStr, - Result: output.ENUM_RESULT_NIL, - }) + output.Success("Configuration set successfully", nil) + output.Message(yamlStr, nil) return nil } diff --git a/internal/commands/config/unset_internal.go b/internal/commands/config/unset_internal.go index 9a2e72ad..3898ceb1 100644 --- a/internal/commands/config/unset_internal.go +++ b/internal/commands/config/unset_internal.go @@ -41,15 +41,8 @@ func RunInternalConfigUnset(viperKey string) (err error) { return fmt.Errorf("failed to unset configuration: %v", err) } - output.Print(output.Opts{ - Message: "Configuration unset successfully", - Result: output.ENUM_RESULT_SUCCESS, - }) - - output.Print(output.Opts{ - Message: yamlStr, - Result: output.ENUM_RESULT_NIL, - }) + output.Success("Configuration unset successfully", nil) + output.Message(yamlStr, nil) return nil } diff --git a/internal/commands/config/view_profile_internal.go b/internal/commands/config/view_profile_internal.go index 85d3225f..1cf374d4 100644 --- a/internal/commands/config/view_profile_internal.go +++ b/internal/commands/config/view_profile_internal.go @@ -26,10 +26,7 @@ func RunInternalConfigViewProfile(args []string) (err error) { profileStr = fmt.Sprintf("Profile: %s\n\n%s", pName, profileStr) - output.Print(output.Opts{ - Message: profileStr, - Result: output.ENUM_RESULT_NIL, - }) + output.Message(profileStr, nil) return nil } diff --git a/internal/commands/feedback/feedback_internal.go b/internal/commands/feedback/feedback_internal.go index 5393a6e9..132702a3 100644 --- a/internal/commands/feedback/feedback_internal.go +++ b/internal/commands/feedback/feedback_internal.go @@ -26,8 +26,5 @@ Open an issue on the project's GitHub repository's issue tracker: // Print the feedback message func PrintFeedbackMessage() { - output.Print(output.Opts{ - Message: FeedbackMessage, - Result: output.ENUM_RESULT_NIL, - }) + output.Message(FeedbackMessage, nil) } diff --git a/internal/commands/platform/export_internal.go b/internal/commands/platform/export_internal.go index c7669930..1d208677 100644 --- a/internal/commands/platform/export_internal.go +++ b/internal/commands/platform/export_internal.go @@ -92,10 +92,7 @@ func RunInternalExport(ctx context.Context, commandVersion string) (err error) { return err } - output.Print(output.Opts{ - Message: fmt.Sprintf("Export to directory '%s' complete.", outputDir), - Result: output.ENUM_RESULT_SUCCESS, - }) + output.Success(fmt.Sprintf("Export to directory '%s' complete.", outputDir), nil) return nil } @@ -366,20 +363,14 @@ func createOrValidateOutputDir(outputDir string, overwriteExport bool) (err erro l.Debug().Msgf("Validating export output directory '%s'", outputDir) _, err = os.Stat(outputDir) if err != nil { - output.Print(output.Opts{ - Message: fmt.Sprintf("failed to find 'platform export' output directory. creating new output directory at filepath '%s'", outputDir), - Result: output.ENUM_RESULT_NOACTION_WARN, - }) + output.Warn(fmt.Sprintf("failed to find 'platform export' output directory. creating new output directory at filepath '%s'", outputDir), nil) err = os.MkdirAll(outputDir, os.ModePerm) if err != nil { return fmt.Errorf("failed to create 'platform export' output directory '%s': %s", outputDir, err.Error()) } - output.Print(output.Opts{ - Message: fmt.Sprintf("new 'platform export' output directory '%s' created", outputDir), - Result: output.ENUM_RESULT_SUCCESS, - }) + output.Success(fmt.Sprintf("new 'platform export' output directory '%s' created", outputDir), nil) } else { // Check if the output directory is empty // If not, default behavior is to exit and not overwrite. @@ -414,10 +405,7 @@ func getPingOneExportEnvID() (err error) { return fmt.Errorf("failed to determine pingone export environment ID.") } - output.Print(output.Opts{ - Message: "No target PingOne export environment ID specified. Defaulting export environment ID to the Worker App environment ID.", - Result: output.ENUM_RESULT_NOACTION_WARN, - }) + output.Warn("No target PingOne export environment ID specified. Defaulting export environment ID to the Worker App environment ID.", nil) } return nil @@ -483,10 +471,7 @@ func exportConnectors(exportableConnectors *[]connector.Exportable, exportFormat // Loop through user defined exportable connectors and export them for _, connector := range *exportableConnectors { - output.Print(output.Opts{ - Message: fmt.Sprintf("Exporting %s service...", connector.ConnectorServiceName()), - Result: output.ENUM_RESULT_NIL, - }) + output.Message(fmt.Sprintf("Exporting %s service...", connector.ConnectorServiceName()), nil) err := connector.Export(exportFormat, outputDir, overwriteExport) if err != nil { diff --git a/internal/commands/request/request_internal.go b/internal/commands/request/request_internal.go index 822f2f3c..4eee42db 100644 --- a/internal/commands/request/request_internal.go +++ b/internal/commands/request/request_internal.go @@ -96,24 +96,15 @@ func runInternalPingOneRequest(uri string) (err error) { return err } + fields := map[string]any{ + "response": json.RawMessage(body), + "status": res.StatusCode, + } + if res.StatusCode < 200 || res.StatusCode >= 300 { - output.Print(output.Opts{ - Message: "Custom request", - Result: output.ENUM_RESULT_FAILURE, - Fields: map[string]any{ - "response": json.RawMessage(body), - "status": res.StatusCode, - }, - }) + output.UserError("Failed Custom Request", fields) } else { - output.Print(output.Opts{ - Message: "Custom request", - Result: output.ENUM_RESULT_SUCCESS, - Fields: map[string]any{ - "response": json.RawMessage(body), - "status": res.StatusCode, - }, - }) + output.Success("Custom request successful", fields) } return nil @@ -179,10 +170,7 @@ func pingoneAccessToken() (accessToken string, err error) { } } - output.Print(output.Opts{ - Message: "PingOne access token does not exist or is expired, requesting a new token...", - Result: output.ENUM_RESULT_NOACTION_WARN, - }) + output.Warn("PingOne access token does not exist or is expired, requesting a new token...", nil) // If no valid access token is available, login and get a new one return pingoneAuth() diff --git a/internal/output/output.go b/internal/output/output.go index f45a96a3..44780442 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -10,6 +10,7 @@ import ( "github.com/pingidentity/pingcli/internal/customtypes" "github.com/pingidentity/pingcli/internal/logger" "github.com/pingidentity/pingcli/internal/profiles" + "github.com/rs/zerolog" ) var ( @@ -21,24 +22,7 @@ var ( yellow = color.New(color.FgYellow).SprintfFunc() ) -type Result string - -type Opts struct { - Fields map[string]interface{} - Message string - ErrorMessage string - FatalMessage string - Result Result -} - -const ( - ENUM_RESULT_NIL Result = "" - ENUM_RESULT_SUCCESS Result = "Success" - ENUM_RESULT_NOACTION_OK Result = "No Action (OK)" - ENUM_RESULT_NOACTION_WARN Result = "No Action (Warning)" - ENUM_RESULT_FAILURE Result = "Failure" -) - +// Set the faith color option based on user configuration func SetColorize() { colorizeOutput, err := profiles.GetOptionValue(options.RootColorOption) if err != nil { @@ -53,8 +37,60 @@ func SetColorize() { } } -func Print(output Opts) { +// This function outputs white text to supply information to the user. +func Message(message string, fields map[string]interface{}) { + l := logger.Get() + + print(fmt.Sprintf("INFO: %s", message), fields, white, l.Info()) +} + +// This function outputs green text to inform the user of success +func Success(message string, fields map[string]interface{}) { + l := logger.Get() + + print(fmt.Sprintf("SUCCESS: %s", message), fields, green, l.Info()) +} + +// This function outputs yellow text to inform the user of a warning +func Warn(message string, fields map[string]interface{}) { + l := logger.Get() + + print(fmt.Sprintf("WARNING: %s", message), fields, yellow, l.Warn()) +} + +// This functions is used to inform the user their configuration +// or input to pingcli has caused an error. +func UserError(message string, fields map[string]interface{}) { + l := logger.Get() + + print(fmt.Sprintf("ERROR: %s", message), fields, red, l.Error()) +} + +// This functions is used to inform the user their configuration +// or input to pingcli has caused an fatal error that exits the program immediately. +func UserFatal(message string, fields map[string]interface{}) { + l := logger.Get() + + print(fmt.Sprintf("FATAL: %s", message), fields, boldRed, l.Fatal()) +} + +// This function is used to inform the user a system-level error +// has occurred. These errors should indicate a bug or bad behavior +// of the tool. +func SystemError(message string, fields map[string]interface{}) { + l := logger.Get() + + systemMsg := fmt.Sprintf(`FATAL: %s + +This is a bug in the Ping CLI and needs reporting to our team. + +Please raise an issue at https://github.com/pingidentity/pingcli`, + message) + print(systemMsg, fields, boldRed, l.Fatal()) +} + +func print(message string, fields map[string]interface{}, colorFunc func(format string, a ...interface{}) string, logEvent *zerolog.Event) { SetColorize() outputFormat, err := profiles.GetOptionValue(options.RootOutputFormatOption) @@ -64,107 +100,53 @@ func Print(output Opts) { switch outputFormat { case customtypes.ENUM_OUTPUT_FORMAT_TEXT: - printText(output) + printText(message, fields, colorFunc, logEvent) case customtypes.ENUM_OUTPUT_FORMAT_JSON: - printJson(output) - default: - printText(Opts{ - Message: fmt.Sprintf("Output format %q is not recognized. Defaulting to \"text\" output", outputFormat), - Result: ENUM_RESULT_NOACTION_WARN, - }) - printText(output) - } -} - -func printText(opts Opts) { - l := logger.Get() - - var resultFormat string - var resultColor func(format string, a ...interface{}) string - - // Determine message color and format based on status - switch opts.Result { - case ENUM_RESULT_SUCCESS: - resultFormat = "%s - %s" - resultColor = green - case ENUM_RESULT_NOACTION_OK: - resultFormat = "%s - %s" - resultColor = green - case ENUM_RESULT_NOACTION_WARN: - resultFormat = "%s - %s" - resultColor = yellow - case ENUM_RESULT_FAILURE: - resultFormat = "%s - %s" - resultColor = red - case ENUM_RESULT_NIL: - resultFormat = "%s%s" - resultColor = white + printJson(message, fields, logEvent) default: - resultFormat = "%s%s" - resultColor = white + l := logger.Get() + printText(fmt.Sprintf("Output format %q is not recognized. Defaulting to \"text\" output", outputFormat), nil, yellow, l.Warn()) + printText(message, fields, colorFunc, logEvent) } - // Supply the user a formatted message and a result status if any. - fmt.Println(resultColor(resultFormat, opts.Message, opts.Result)) - l.Info().Msg(resultColor(resultFormat, opts.Message, opts.Result)) +} - // Output and log any additional key/value pairs supplied to the user. - if opts.Fields != nil { +func printText(message string, fields map[string]interface{}, colorFunc func(format string, a ...interface{}) string, logEvent *zerolog.Event) { + if fields != nil { fmt.Println(cyan("Additional Information:")) - for k, v := range opts.Fields { + for k, v := range fields { + l := logger.Get() switch typedValue := v.(type) { // If the value is a json.RawMessage, print it as a string case json.RawMessage: fmt.Println(cyan("%s: %s", k, typedValue)) - l.Info().Msgf("%s: %s", k, typedValue) + l.Info().Msgf(cyan("%s: %s", k, typedValue)) default: fmt.Println(cyan("%s: %v", k, v)) - l.Info().Msgf("%s: %v", k, v) + l.Info().Msgf(cyan("%s: %v", k, v)) } } } - // Inform the user of an error and log the error - if opts.ErrorMessage != "" { - fmt.Println(red("Error: %s", opts.ErrorMessage)) - l.Error().Msg(opts.ErrorMessage) - } - - // Inform the user of a fatal error and log the fatal error. This exits the program. - if opts.FatalMessage != "" { - fmt.Println(boldRed("Fatal: %s", opts.FatalMessage)) - l.Fatal().Msg(opts.FatalMessage) - } - + fmt.Println(colorFunc(message)) + logEvent.Msg(colorFunc(message)) } -func printJson(opts Opts) { +func printJson(message string, fields map[string]interface{}, logEvent *zerolog.Event) { l := logger.Get() + if fields["message"] == nil { + fields["message"] = message + } + // Convert the CommandOutput struct to JSON - jsonOut, err := json.MarshalIndent(opts, "", " ") + jsonOut, err := json.MarshalIndent(fields, "", " ") if err != nil { l.Error().Err(err).Msgf("Failed to serialize output as JSON") + return } // Output the JSON as uncolored string fmt.Println(string(jsonOut)) - - switch opts.Result { - case ENUM_RESULT_NOACTION_WARN: - l.Warn().Msg(string(jsonOut)) - case ENUM_RESULT_FAILURE: - // Log the error if exists - if opts.ErrorMessage != "" { - l.Error().Msg(opts.ErrorMessage) - } - - // Log the fatal error if exists. This exits the program. - if opts.FatalMessage != "" { - l.Fatal().Msg(opts.FatalMessage) - } - default: //ENUM_RESULT_SUCCESS, ENUM_RESULT_NIL, ENUM_RESULT_NOACTION_OK - l.Info().Msg(string(jsonOut)) - } - + logEvent.Msg(string(jsonOut)) } diff --git a/main.go b/main.go index e54485a3..7048cbb9 100644 --- a/main.go +++ b/main.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "runtime/debug" "github.com/pingidentity/pingcli/cmd" @@ -31,10 +32,6 @@ func main() { err := rootCmd.Execute() if err != nil { - output.Print(output.Opts{ - ErrorMessage: err.Error(), - Message: "Failed to execute pingcli", - Result: output.ENUM_RESULT_FAILURE, - }) + output.UserError(fmt.Sprintf("Failed to execute pingcli: %v", err), nil) } } From 849d23f7a43cb5ad7da4d8b3c584db538822bc83 Mon Sep 17 00:00:00 2001 From: Erik Ostien Date: Tue, 29 Oct 2024 11:06:02 +0100 Subject: [PATCH 2/4] golangci-lint fixes --- internal/output/output.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/output/output.go b/internal/output/output.go index 44780442..47c5239b 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -120,10 +120,10 @@ func printText(message string, fields map[string]interface{}, colorFunc func(for // If the value is a json.RawMessage, print it as a string case json.RawMessage: fmt.Println(cyan("%s: %s", k, typedValue)) - l.Info().Msgf(cyan("%s: %s", k, typedValue)) + l.Info().Msg(cyan("%s: %s", k, typedValue)) default: fmt.Println(cyan("%s: %v", k, v)) - l.Info().Msgf(cyan("%s: %v", k, v)) + l.Info().Msg(cyan("%s: %v", k, v)) } } } From d75dac96c8554b0c72dcca546334d9b125a6e3a7 Mon Sep 17 00:00:00 2001 From: Erik Ostien Date: Wed, 30 Oct 2024 12:45:58 +0100 Subject: [PATCH 3/4] Fix fatal output exiting early --- internal/output/output.go | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/internal/output/output.go b/internal/output/output.go index 47c5239b..1f984869 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -69,17 +69,15 @@ func UserError(message string, fields map[string]interface{}) { // This functions is used to inform the user their configuration // or input to pingcli has caused an fatal error that exits the program immediately. func UserFatal(message string, fields map[string]interface{}) { - l := logger.Get() - - print(fmt.Sprintf("FATAL: %s", message), fields, boldRed, l.Fatal()) + // l.Fatal() exits the program prematurely before the message is printed + // pass nil to print the message before exiting + print(fmt.Sprintf("FATAL: %s", message), fields, boldRed, nil) } // This function is used to inform the user a system-level error // has occurred. These errors should indicate a bug or bad behavior // of the tool. func SystemError(message string, fields map[string]interface{}) { - l := logger.Get() - systemMsg := fmt.Sprintf(`FATAL: %s This is a bug in the Ping CLI and needs reporting to our team. @@ -87,7 +85,9 @@ This is a bug in the Ping CLI and needs reporting to our team. Please raise an issue at https://github.com/pingidentity/pingcli`, message) - print(systemMsg, fields, boldRed, l.Fatal()) + // l.Fatal() exits the program prematurely before the message is printed + // pass nil to print the message before exiting + print(systemMsg, fields, boldRed, nil) } func print(message string, fields map[string]interface{}, colorFunc func(format string, a ...interface{}) string, logEvent *zerolog.Event) { @@ -112,10 +112,11 @@ func print(message string, fields map[string]interface{}, colorFunc func(format } func printText(message string, fields map[string]interface{}, colorFunc func(format string, a ...interface{}) string, logEvent *zerolog.Event) { + l := logger.Get() + if fields != nil { fmt.Println(cyan("Additional Information:")) for k, v := range fields { - l := logger.Get() switch typedValue := v.(type) { // If the value is a json.RawMessage, print it as a string case json.RawMessage: @@ -129,7 +130,11 @@ func printText(message string, fields map[string]interface{}, colorFunc func(for } fmt.Println(colorFunc(message)) - logEvent.Msg(colorFunc(message)) + if logEvent != nil { + logEvent.Msg(colorFunc(message)) + } else { + l.Fatal().Msg(colorFunc(message)) + } } func printJson(message string, fields map[string]interface{}, logEvent *zerolog.Event) { @@ -148,5 +153,9 @@ func printJson(message string, fields map[string]interface{}, logEvent *zerolog. // Output the JSON as uncolored string fmt.Println(string(jsonOut)) - logEvent.Msg(string(jsonOut)) + if logEvent != nil { + logEvent.Msg(string(jsonOut)) + } else { + l.Fatal().Msg(string(jsonOut)) + } } From 137b456d789386bb10644757fb50080e72cad9b8 Mon Sep 17 00:00:00 2001 From: Erik Ostien Date: Wed, 30 Oct 2024 13:16:35 +0100 Subject: [PATCH 4/4] Fix logevent function calls, and pass the right functions to the print logic --- .../commands/config/list_profiles_internal.go | 2 +- internal/output/output.go | 51 +++++++++---------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/internal/commands/config/list_profiles_internal.go b/internal/commands/config/list_profiles_internal.go index eddfafcc..3a768adc 100644 --- a/internal/commands/config/list_profiles_internal.go +++ b/internal/commands/config/list_profiles_internal.go @@ -33,7 +33,7 @@ func RunInternalConfigListProfiles() { } if description != "" { - listStr += " " + description + "\n" + listStr += " " + description } } diff --git a/internal/output/output.go b/internal/output/output.go index e3b786a8..47b14d6a 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -41,43 +41,42 @@ func SetColorize() { func Message(message string, fields map[string]interface{}) { l := logger.Get() - print(fmt.Sprintf("INFO: %s", message), fields, white, l.Info()) + print(fmt.Sprintf("INFO: %s", message), fields, white, l.Info) } // This function outputs green text to inform the user of success func Success(message string, fields map[string]interface{}) { l := logger.Get() - print(fmt.Sprintf("SUCCESS: %s", message), fields, green, l.Info()) + print(fmt.Sprintf("SUCCESS: %s", message), fields, green, l.Info) } // This function outputs yellow text to inform the user of a warning func Warn(message string, fields map[string]interface{}) { l := logger.Get() - print(fmt.Sprintf("WARNING: %s", message), fields, yellow, l.Warn()) + print(fmt.Sprintf("WARNING: %s", message), fields, yellow, l.Warn) } // This functions is used to inform the user their configuration // or input to pingcli has caused an error. func UserError(message string, fields map[string]interface{}) { l := logger.Get() - - print(fmt.Sprintf("ERROR: %s", message), fields, red, l.Error()) + print(fmt.Sprintf("ERROR: %s", message), fields, red, l.Error) } // This functions is used to inform the user their configuration // or input to pingcli has caused an fatal error that exits the program immediately. func UserFatal(message string, fields map[string]interface{}) { - // l.Fatal() exits the program prematurely before the message is printed - // pass nil to print the message before exiting - print(fmt.Sprintf("FATAL: %s", message), fields, boldRed, nil) + l := logger.Get() + print(fmt.Sprintf("FATAL: %s", message), fields, boldRed, l.Fatal) } // This function is used to inform the user a system-level error // has occurred. These errors should indicate a bug or bad behavior // of the tool. func SystemError(message string, fields map[string]interface{}) { + l := logger.Get() systemMsg := fmt.Sprintf(`FATAL: %s This is a bug in the Ping CLI and needs reporting to our team. @@ -87,10 +86,13 @@ Please raise an issue at https://github.com/pingidentity/pingcli`, // l.Fatal() exits the program prematurely before the message is printed // pass nil to print the message before exiting - print(systemMsg, fields, boldRed, nil) + print(systemMsg, fields, boldRed, l.Fatal) } -func print(message string, fields map[string]interface{}, colorFunc func(format string, a ...interface{}) string, logEvent *zerolog.Event) { +func print(message string, + fields map[string]interface{}, + colorFunc func(format string, a ...interface{}) string, + logEventFunc func() *zerolog.Event) { SetColorize() outputFormat, err := profiles.GetOptionValue(options.RootOutputFormatOption) @@ -100,18 +102,21 @@ func print(message string, fields map[string]interface{}, colorFunc func(format switch outputFormat { case customtypes.ENUM_OUTPUT_FORMAT_TEXT: - printText(message, fields, colorFunc, logEvent) + printText(message, fields, colorFunc, logEventFunc) case customtypes.ENUM_OUTPUT_FORMAT_JSON: - printJson(message, fields, logEvent) + printJson(message, fields, logEventFunc) default: l := logger.Get() - printText(fmt.Sprintf("Output format %q is not recognized. Defaulting to \"text\" output", outputFormat), nil, yellow, l.Warn()) - printText(message, fields, colorFunc, logEvent) + printText(fmt.Sprintf("Output format %q is not recognized. Defaulting to \"text\" output", outputFormat), nil, yellow, l.Warn) + printText(message, fields, colorFunc, logEventFunc) } } -func printText(message string, fields map[string]interface{}, colorFunc func(format string, a ...interface{}) string, logEvent *zerolog.Event) { +func printText(message string, + fields map[string]interface{}, + colorFunc func(format string, a ...interface{}) string, + logEventFunc func() *zerolog.Event) { l := logger.Get() if fields != nil { @@ -130,14 +135,12 @@ func printText(message string, fields map[string]interface{}, colorFunc func(for } fmt.Println(colorFunc(message)) - if logEvent != nil { - logEvent.Msg(colorFunc(message)) - } else { - l.Fatal().Msg(colorFunc(message)) - } + logEventFunc().Msg(colorFunc(message)) } -func printJson(message string, fields map[string]interface{}, logEvent *zerolog.Event) { +func printJson(message string, + fields map[string]interface{}, + logEventFunc func() *zerolog.Event) { l := logger.Get() if fields["message"] == nil { @@ -153,9 +156,5 @@ func printJson(message string, fields map[string]interface{}, logEvent *zerolog. // Output the JSON as uncolored string fmt.Println(string(jsonOut)) - if logEvent != nil { - logEvent.Msg(string(jsonOut)) - } else { - l.Fatal().Msg(string(jsonOut)) - } + logEventFunc().Msg(string(jsonOut)) }