From 52fd1be2b9166bcb6a0caa391006db6b481defbc Mon Sep 17 00:00:00 2001 From: wesleymccollam Date: Thu, 21 Nov 2024 11:46:34 -0500 Subject: [PATCH 1/3] PDI-2101: add fail flag to propagate non-zero exit code on custom request failure --- cmd/request/request.go | 1 + internal/commands/request/request_internal.go | 16 +++++++++++++ .../commands/request/request_internal_test.go | 12 ++++++++++ internal/configuration/options/options.go | 2 ++ internal/configuration/request/request.go | 24 ++++++++++++++++++- 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/cmd/request/request.go b/cmd/request/request.go index dc317639..385b640f 100644 --- a/cmd/request/request.go +++ b/cmd/request/request.go @@ -42,6 +42,7 @@ The command offers a cURL-like experience to interact with the Ping platform ser cmd.Flags().AddFlag(options.RequestHTTPMethodOption.Flag) cmd.Flags().AddFlag(options.RequestServiceOption.Flag) cmd.Flags().AddFlag(options.RequestDataOption.Flag) + cmd.Flags().AddFlag(options.RequestFailOption.Flag) return cmd } diff --git a/internal/commands/request/request_internal.go b/internal/commands/request/request_internal.go index b39662c5..3b8aeb0d 100644 --- a/internal/commands/request/request_internal.go +++ b/internal/commands/request/request_internal.go @@ -57,6 +57,11 @@ func runInternalPingOneRequest(uri string) (err error) { return err } + failOption, err := getFailOption() + if err != nil { + return err + } + apiURL := fmt.Sprintf("https://api.pingone.%s/v1/%s", topLevelDomain, uri) httpMethod, err := profiles.GetOptionValue(options.RequestHTTPMethodOption) @@ -108,6 +113,9 @@ func runInternalPingOneRequest(uri string) (err error) { } else { output.Success("Custom request successful", fields) } + if failOption == "true" { + return fmt.Errorf("custom request failed with --fail (-f) flag") + } return nil } @@ -297,3 +305,11 @@ func getData() (data string, err error) { return data, nil } + +func getFailOption() (fail string, err error) { + fail, err = profiles.GetOptionValue(options.RequestFailOption) + if err != nil { + return "", err + } + return fail, nil +} diff --git a/internal/commands/request/request_internal_test.go b/internal/commands/request/request_internal_test.go index b7e26cab..f0875528 100644 --- a/internal/commands/request/request_internal_test.go +++ b/internal/commands/request/request_internal_test.go @@ -21,6 +21,18 @@ func Test_RunInternalRequest(t *testing.T) { testutils.CheckExpectedError(t, err, nil) } +// Test RunInternalRequest function with fail +func Test_RunInternalRequestWithFail(t *testing.T) { + testutils_viper.InitVipers(t) + t.Setenv(options.RequestServiceOption.EnvVar, "pingone") + options.RequestFailOption.Flag.Changed = true + options.RequestFailOption.Flag.Value.Set("true") + + err := RunInternalRequest("environments/failTest") + expectedErrorPattern := `^failed to send custom request: custom request failed with --fail \(-f\) flag$` + testutils.CheckExpectedError(t, err, &expectedErrorPattern) +} + // Test RunInternalRequest function with empty service func Test_RunInternalRequest_EmptyService(t *testing.T) { testutils_viper.InitVipers(t) diff --git a/internal/configuration/options/options.go b/internal/configuration/options/options.go index 0cc98e6d..6bef857e 100644 --- a/internal/configuration/options/options.go +++ b/internal/configuration/options/options.go @@ -77,6 +77,7 @@ func Options() []Option { RequestServiceOption, RequestAccessTokenOption, RequestAccessTokenExpiryOption, + RequestFailOption, } } @@ -145,4 +146,5 @@ var ( RequestServiceOption Option RequestAccessTokenOption Option RequestAccessTokenExpiryOption Option + RequestFailOption Option ) diff --git a/internal/configuration/request/request.go b/internal/configuration/request/request.go index 7ff1e795..78098609 100644 --- a/internal/configuration/request/request.go +++ b/internal/configuration/request/request.go @@ -15,7 +15,7 @@ func InitRequestOptions() { initServiceOption() initAccessTokenOption() initAccessTokenExpiryOption() - + initFailOption() } func initDataOption() { @@ -123,3 +123,25 @@ func initAccessTokenExpiryOption() { ViperKey: "request.accessTokenExpiry", } } + +func initFailOption() { + cobraParamName := "fail" + cobraValue := new(customtypes.Bool) + defaultValue := customtypes.Bool(false) + + options.RequestFailOption = options.Option{ + CobraParamName: cobraParamName, + CobraParamValue: cobraValue, + DefaultValue: &defaultValue, + Flag: &pflag.Flag{ + Name: cobraParamName, + NoOptDefVal: "true", + Shorthand: "f", + Usage: "Return exit code when HTTP custom request returns a failure status code.", + Value: cobraValue, + }, + + Type: options.ENUM_BOOL, + ViperKey: "request.fail", + } +} From 4d8fc6833fbda3f54ea6a6fd7815758a6e0e7c29 Mon Sep 17 00:00:00 2001 From: wesleymccollam Date: Thu, 21 Nov 2024 13:35:13 -0500 Subject: [PATCH 2/3] update test, remove non-needed function, update usage message --- internal/commands/request/request_internal.go | 16 ++++------------ .../commands/request/request_internal_test.go | 7 +++++-- internal/configuration/request/request.go | 2 +- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/internal/commands/request/request_internal.go b/internal/commands/request/request_internal.go index 3b8aeb0d..4801eb81 100644 --- a/internal/commands/request/request_internal.go +++ b/internal/commands/request/request_internal.go @@ -57,7 +57,7 @@ func runInternalPingOneRequest(uri string) (err error) { return err } - failOption, err := getFailOption() + failOption, err := profiles.GetOptionValue(options.RequestFailOption) if err != nil { return err } @@ -110,12 +110,12 @@ func runInternalPingOneRequest(uri string) (err error) { // Note we don't os.Exit(1) here because pingcli has executed // without issue, despite a failed response to the custom request output.UserError("Failed Custom Request", fields) + if failOption == "true" { + return fmt.Errorf("custom request failed with --fail (-f) flag") + } } else { output.Success("Custom request successful", fields) } - if failOption == "true" { - return fmt.Errorf("custom request failed with --fail (-f) flag") - } return nil } @@ -305,11 +305,3 @@ func getData() (data string, err error) { return data, nil } - -func getFailOption() (fail string, err error) { - fail, err = profiles.GetOptionValue(options.RequestFailOption) - if err != nil { - return "", err - } - return fail, nil -} diff --git a/internal/commands/request/request_internal_test.go b/internal/commands/request/request_internal_test.go index f0875528..87793a64 100644 --- a/internal/commands/request/request_internal_test.go +++ b/internal/commands/request/request_internal_test.go @@ -26,9 +26,12 @@ func Test_RunInternalRequestWithFail(t *testing.T) { testutils_viper.InitVipers(t) t.Setenv(options.RequestServiceOption.EnvVar, "pingone") options.RequestFailOption.Flag.Changed = true - options.RequestFailOption.Flag.Value.Set("true") + err := options.RequestFailOption.Flag.Value.Set("true") + if err != nil { + t.Fatal(err) + } - err := RunInternalRequest("environments/failTest") + err = RunInternalRequest("environments/failTest") expectedErrorPattern := `^failed to send custom request: custom request failed with --fail \(-f\) flag$` testutils.CheckExpectedError(t, err, &expectedErrorPattern) } diff --git a/internal/configuration/request/request.go b/internal/configuration/request/request.go index 78098609..9b95e30b 100644 --- a/internal/configuration/request/request.go +++ b/internal/configuration/request/request.go @@ -137,7 +137,7 @@ func initFailOption() { Name: cobraParamName, NoOptDefVal: "true", Shorthand: "f", - Usage: "Return exit code when HTTP custom request returns a failure status code.", + Usage: "Return non-zero exit code when HTTP custom request returns a failure status code.", Value: cobraValue, }, From 986383adec31b7b7d86f78bc4c7465453f76f8a2 Mon Sep 17 00:00:00 2001 From: wesleymccollam Date: Thu, 21 Nov 2024 16:18:41 -0500 Subject: [PATCH 3/3] update output to remove usage info, os.exit 1 --- internal/commands/request/request_internal.go | 4 +-- .../commands/request/request_internal_test.go | 31 +++++++++++++------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/internal/commands/request/request_internal.go b/internal/commands/request/request_internal.go index 4801eb81..f36b2a2c 100644 --- a/internal/commands/request/request_internal.go +++ b/internal/commands/request/request_internal.go @@ -107,11 +107,9 @@ func runInternalPingOneRequest(uri string) (err error) { } if res.StatusCode < 200 || res.StatusCode >= 300 { - // Note we don't os.Exit(1) here because pingcli has executed - // without issue, despite a failed response to the custom request output.UserError("Failed Custom Request", fields) if failOption == "true" { - return fmt.Errorf("custom request failed with --fail (-f) flag") + os.Exit(1) } } else { output.Success("Custom request successful", fields) diff --git a/internal/commands/request/request_internal_test.go b/internal/commands/request/request_internal_test.go index 87793a64..476600dc 100644 --- a/internal/commands/request/request_internal_test.go +++ b/internal/commands/request/request_internal_test.go @@ -3,6 +3,7 @@ package request_internal import ( "fmt" "os" + "os/exec" "testing" "github.com/pingidentity/pingcli/internal/configuration/options" @@ -23,17 +24,27 @@ func Test_RunInternalRequest(t *testing.T) { // Test RunInternalRequest function with fail func Test_RunInternalRequestWithFail(t *testing.T) { - testutils_viper.InitVipers(t) - t.Setenv(options.RequestServiceOption.EnvVar, "pingone") - options.RequestFailOption.Flag.Changed = true - err := options.RequestFailOption.Flag.Value.Set("true") - if err != nil { - t.Fatal(err) - } - err = RunInternalRequest("environments/failTest") - expectedErrorPattern := `^failed to send custom request: custom request failed with --fail \(-f\) flag$` - testutils.CheckExpectedError(t, err, &expectedErrorPattern) + if os.Getenv("RUN_INTERNAL_FAIL_TEST") == "true" { + testutils_viper.InitVipers(t) + t.Setenv(options.RequestServiceOption.EnvVar, "pingone") + options.RequestFailOption.Flag.Changed = true + err := options.RequestFailOption.Flag.Value.Set("true") + if err != nil { + t.Fatal(err) + } + _ = RunInternalRequest("environments/failTest") + t.Fatal("This should never run due to internal request resulting in os.Exit(1)") + } else { + cmdName := os.Args[0] + cmd := exec.Command(cmdName, "-test.run=Test_RunInternalRequestWithFail") + cmd.Env = append(os.Environ(), "RUN_INTERNAL_FAIL_TEST=true") + err := cmd.Run() + if exitError, ok := err.(*exec.ExitError); ok && !exitError.Success() { + return + } + t.Fatalf("The process did not exit with a non-zero: %s", err) + } } // Test RunInternalRequest function with empty service