From a42ba6dfaf2a12a02a3f9f5bcc3a5e1b04930ff7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A9sz=C3=A1ros=20Gergely?= Date: Tue, 23 Jul 2024 15:04:26 +0200 Subject: [PATCH] CDPCP-12279 - Client Error should show failure reason --- .github/workflows/test.yml | 2 +- .../environments/resource_aws_credential.go | 11 +- utils/errors.go | 122 +++++++++- utils/errors_test.go | 215 ++++++++++++++++++ utils/queue.go | 50 ---- utils/utils.go | 34 --- 6 files changed, 337 insertions(+), 97 deletions(-) create mode 100644 utils/errors_test.go delete mode 100644 utils/queue.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 882bf779..ff10cb7b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -100,7 +100,7 @@ jobs: - name: Go Coverage uses: gwatts/go-coverage-action@v2.0.0 with: - coverage-threshold: 29.6 + coverage-threshold: 32.0 cover-pkg: ./... ignore-pattern: | /cdp-sdk-go/ diff --git a/resources/environments/resource_aws_credential.go b/resources/environments/resource_aws_credential.go index 516c512a..43c80a22 100644 --- a/resources/environments/resource_aws_credential.go +++ b/resources/environments/resource_aws_credential.go @@ -14,10 +14,6 @@ import ( "context" "time" - "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/cdp" - "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/environments/client/operations" - environmentsmodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/environments/models" - "github.com/cloudera/terraform-provider-cdp/utils" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" @@ -25,6 +21,11 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" + + "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/cdp" + "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/environments/client/operations" + environmentsmodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/environments/models" + "github.com/cloudera/terraform-provider-cdp/utils" ) // Ensure the implementation satisfies the expected interfaces. @@ -121,7 +122,7 @@ func (r *awsCredentialResource) Create(ctx context.Context, req resource.CreateR responseOk, err := client.Operations.CreateAWSCredential(params) if err != nil { if envErr, ok := err.(*operations.CreateAWSCredentialDefault); ok { - if envErr.Code() == 403 { + if utils.IsRetryableError(envErr.Code()) { return retry.RetryableError(err) } } diff --git a/utils/errors.go b/utils/errors.go index 93fbfa79..db565a1d 100644 --- a/utils/errors.go +++ b/utils/errors.go @@ -24,14 +24,44 @@ import ( opdbmodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/opdb/models" ) +const authFailMsg = "authentication failure, no access key provided or the key is no longer valid" + +var NonRetryableErrorCodes = [...]int{403} + +func IsNonRetryableError(code int) bool { + for _, nonRetryableCode := range NonRetryableErrorCodes { + if nonRetryableCode == code { + return true + } + } + return false +} + +func IsRetryableError(code int) bool { + return !IsNonRetryableError(code) +} + type EnvironmentErrorPayload interface { GetPayload() *environmentsmodels.Error } +func decorateEnvironmentUnauthorizedErrorIfMessageNotExists(err *environmentsmodels.Error) *environmentsmodels.Error { + if err != nil && len(err.Message) == 0 { + return &environmentsmodels.Error{ + Message: authFailMsg, + } + } + return err +} + func AddEnvironmentDiagnosticsError(err error, diagnostics *diag.Diagnostics, errMsg string) { msg := err.Error() if d, ok := err.(EnvironmentErrorPayload); ok && d.GetPayload() != nil { - msg = d.GetPayload().Message + if d.GetPayload().Code == "401" { + msg = decorateEnvironmentUnauthorizedErrorIfMessageNotExists(d.GetPayload()).Message + } else { + msg = d.GetPayload().Message + } } caser := cases.Title(language.English) diagnostics.AddError( @@ -44,10 +74,23 @@ type IamErrorPayload interface { GetPayload() *iammodels.Error } +func decorateIamUnauthorizedErrorIfMessageNotExists(err *iammodels.Error) *iammodels.Error { + if err != nil && len(err.Message) == 0 { + return &iammodels.Error{ + Message: authFailMsg, + } + } + return err +} + func AddIamDiagnosticsError(err error, diagnostics *diag.Diagnostics, errMsg string) { msg := err.Error() if d, ok := err.(IamErrorPayload); ok && d.GetPayload() != nil { - msg = d.GetPayload().Message + if d.GetPayload().Code == "401" { + msg = decorateIamUnauthorizedErrorIfMessageNotExists(d.GetPayload()).Message + } else { + msg = d.GetPayload().Message + } } caser := cases.Title(language.English) diagnostics.AddError( @@ -60,10 +103,23 @@ type MlErrorPayload interface { GetPayload() *mlmodels.Error } +func decorateMlUnauthorizedErrorIfMessageNotExists(err *mlmodels.Error) *mlmodels.Error { + if err != nil && len(err.Message) == 0 { + return &mlmodels.Error{ + Message: authFailMsg, + } + } + return err +} + func AddMlDiagnosticsError(err error, diagnostics *diag.Diagnostics, errMsg string) { msg := err.Error() if d, ok := err.(MlErrorPayload); ok && d.GetPayload() != nil { - msg = d.GetPayload().Message + if d.GetPayload().Code == "401" { + msg = decorateMlUnauthorizedErrorIfMessageNotExists(d.GetPayload()).Message + } else { + msg = d.GetPayload().Message + } } caser := cases.Title(language.English) diagnostics.AddError( @@ -76,10 +132,23 @@ type DeErrorPayload interface { GetPayload() *demodels.Error } +func decorateDeUnauthorizedErrorIfMessageNotExists(err *demodels.Error) *demodels.Error { + if err != nil && len(err.Message) == 0 { + return &demodels.Error{ + Message: authFailMsg, + } + } + return err +} + func AddDeDiagnosticsError(err error, diagnostics *diag.Diagnostics, errMsg string) { msg := err.Error() if d, ok := err.(DeErrorPayload); ok && d.GetPayload() != nil { - msg = d.GetPayload().Message + if d.GetPayload().Code == "401" { + msg = decorateDeUnauthorizedErrorIfMessageNotExists(d.GetPayload()).Message + } else { + msg = d.GetPayload().Message + } } caser := cases.Title(language.English) diagnostics.AddError( @@ -92,10 +161,23 @@ type DatalakeErrorPayload interface { GetPayload() *datalakemodels.Error } +func decorateDatalakeUnauthorizedErrorIfMessageNotExists(err *datalakemodels.Error) *datalakemodels.Error { + if err != nil && len(err.Message) == 0 { + return &datalakemodels.Error{ + Message: authFailMsg, + } + } + return err +} + func AddDatalakeDiagnosticsError(err error, diagnostics *diag.Diagnostics, errMsg string) { msg := err.Error() if d, ok := err.(DatalakeErrorPayload); ok && d.GetPayload() != nil { - msg = d.GetPayload().Message + if d.GetPayload().Code == "401" { + msg = decorateDatalakeUnauthorizedErrorIfMessageNotExists(d.GetPayload()).Message + } else { + msg = d.GetPayload().Message + } } caser := cases.Title(language.English) diagnostics.AddError( @@ -108,10 +190,23 @@ type DatahubErrorPayload interface { GetPayload() *datahubmodels.Error } +func decorateDatahubUnauthorizedErrorIfMessageNotExists(err *datahubmodels.Error) *datahubmodels.Error { + if err != nil && len(err.Message) == 0 { + return &datahubmodels.Error{ + Message: authFailMsg, + } + } + return err +} + func AddDatahubDiagnosticsError(err error, diagnostics *diag.Diagnostics, errMsg string) { msg := err.Error() if d, ok := err.(DatahubErrorPayload); ok && d.GetPayload() != nil { - msg = d.GetPayload().Message + if d.GetPayload().Code == "401" { + msg = decorateDatahubUnauthorizedErrorIfMessageNotExists(d.GetPayload()).Message + } else { + msg = d.GetPayload().Message + } } caser := cases.Title(language.English) diagnostics.AddError( @@ -124,10 +219,23 @@ type DatabaseErrorPayload interface { GetPayload() *opdbmodels.Error } +func decorateDatabaseUnauthorizedErrorIfMessageNotExists(err *opdbmodels.Error) *opdbmodels.Error { + if err != nil && len(err.Message) == 0 { + return &opdbmodels.Error{ + Message: authFailMsg, + } + } + return err +} + func AddDatabaseDiagnosticsError(err error, diagnostics *diag.Diagnostics, errMsg string) { msg := err.Error() if d, ok := err.(DatabaseErrorPayload); ok && d.GetPayload() != nil { - msg = d.GetPayload().Message + if d.GetPayload().Code == "401" { + msg = decorateDatabaseUnauthorizedErrorIfMessageNotExists(d.GetPayload()).Message + } else { + msg = d.GetPayload().Message + } } caser := cases.Title(language.English) diagnostics.AddError( diff --git a/utils/errors_test.go b/utils/errors_test.go new file mode 100644 index 00000000..23442cf6 --- /dev/null +++ b/utils/errors_test.go @@ -0,0 +1,215 @@ +// Copyright 2024 Cloudera. All Rights Reserved. +// +// This file is licensed under the Apache License Version 2.0 (the "License"). +// You may not use this file except in compliance with the License. +// You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0. +// +// This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS +// OF ANY KIND, either express or implied. Refer to the License for the specific +// permissions and limitations governing your use of the file. + +package utils + +import ( + "testing" + + datahubmodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/datahub/models" + datalakemodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/datalake/models" + demodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/de/models" + environmentsmodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/environments/models" + iammodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/iam/models" + mlmodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/ml/models" + opdbmodels "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/opdb/models" +) + +var testNonRetryableCodes = []int{403} + +func TestIsNonRetryableErrorWhenCodeIsNonRetryable(t *testing.T) { + for _, code := range testNonRetryableCodes { + if !IsNonRetryableError(code) { + t.Errorf("Code %d is non-retryable but the function returned otherwise!", code) + } + } +} + +func TestIsNonRetryableErrorWhenCodeIsRetryable(t *testing.T) { + if IsNonRetryableError(500) { + t.Error("Code 500 is retryable but the function returned otherwise!") + } +} + +func TestIsRetryableErrorWhenCodeIsRetryable(t *testing.T) { + for _, code := range testNonRetryableCodes { + if IsRetryableError(code) { + t.Errorf("Code %d is non-retryable but the function returned otherwise!", code) + } + } +} + +func TestIsRetryableErrorWhenCodeIsNonRetryable(t *testing.T) { + if !IsRetryableError(500) { + t.Error("Code 500 is retryable but the function returned otherwise!") + } + +} + +func TestDecorateEnvironmentUnAuthWhenErrIsNil(t *testing.T) { + if decorateEnvironmentUnauthorizedErrorIfMessageNotExists(nil) != nil { + t.Error("Result is not nil but it should be!") + } +} + +func TestDecorateEnvironmentUnAuthWhenErrMessageIsNotEmpty(t *testing.T) { + msg := "something" + result := decorateEnvironmentUnauthorizedErrorIfMessageNotExists(&environmentsmodels.Error{ + Message: msg, + }) + if result.Message != msg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", msg, result.Message) + } +} + +func TestDecorateEnvironmentUnAuthWhenErrMessageIsEmpty(t *testing.T) { + result := decorateEnvironmentUnauthorizedErrorIfMessageNotExists(&environmentsmodels.Error{}) + if result.Message != authFailMsg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", authFailMsg, result.Message) + } +} + +func TestDecorateIamUnAuthWhenErrIsNil(t *testing.T) { + if decorateIamUnauthorizedErrorIfMessageNotExists(nil) != nil { + t.Error("Result is not nil but it should be!") + } +} + +func TestDecorateIamUnAuthWhenErrMessageIsNotEmpty(t *testing.T) { + msg := "something" + result := decorateIamUnauthorizedErrorIfMessageNotExists(&iammodels.Error{ + Message: msg, + }) + if result.Message != msg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", msg, result.Message) + } +} + +func TestDecorateIamUnAuthWhenErrMessageIsEmpty(t *testing.T) { + result := decorateIamUnauthorizedErrorIfMessageNotExists(&iammodels.Error{}) + if result.Message != authFailMsg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", authFailMsg, result.Message) + } +} + +func TestDecorateMlUnAuthWhenErrIsNil(t *testing.T) { + if decorateMlUnauthorizedErrorIfMessageNotExists(nil) != nil { + t.Error("Result is not nil but it should be!") + } +} + +func TestDecorateMlUnAuthWhenErrMessageIsNotEmpty(t *testing.T) { + msg := "something" + result := decorateMlUnauthorizedErrorIfMessageNotExists(&mlmodels.Error{ + Message: msg, + }) + if result.Message != msg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", msg, result.Message) + } +} + +func TestDecorateMlUnAuthWhenErrMessageIsEmpty(t *testing.T) { + result := decorateMlUnauthorizedErrorIfMessageNotExists(&mlmodels.Error{}) + if result.Message != authFailMsg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", authFailMsg, result.Message) + } +} + +func TestDecorateDeUnAuthWhenErrIsNil(t *testing.T) { + if decorateDeUnauthorizedErrorIfMessageNotExists(nil) != nil { + t.Error("Result is not nil but it should be!") + } +} + +func TestDecorateDeUnAuthWhenErrMessageIsNotEmpty(t *testing.T) { + msg := "something" + result := decorateDeUnauthorizedErrorIfMessageNotExists(&demodels.Error{ + Message: msg, + }) + if result.Message != msg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", msg, result.Message) + } +} + +func TestDecorateDeUnAuthWhenErrMessageIsEmpty(t *testing.T) { + result := decorateDeUnauthorizedErrorIfMessageNotExists(&demodels.Error{}) + if result.Message != authFailMsg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", authFailMsg, result.Message) + } +} + +func TestDecorateDatalakeUnAuthWhenErrIsNil(t *testing.T) { + if decorateDatalakeUnauthorizedErrorIfMessageNotExists(nil) != nil { + t.Error("Result is not nil but it should be!") + } +} + +func TestDecorateDatalakeUnAuthWhenErrMessageIsNotEmpty(t *testing.T) { + msg := "something" + result := decorateDatalakeUnauthorizedErrorIfMessageNotExists(&datalakemodels.Error{ + Message: msg, + }) + if result.Message != msg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", msg, result.Message) + } +} + +func TestDecorateDatalakeUnAuthWhenErrMessageIsEmpty(t *testing.T) { + result := decorateDatalakeUnauthorizedErrorIfMessageNotExists(&datalakemodels.Error{}) + if result.Message != authFailMsg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", authFailMsg, result.Message) + } +} + +func TestDecorateDatahubUnAuthWhenErrIsNil(t *testing.T) { + if decorateDatahubUnauthorizedErrorIfMessageNotExists(nil) != nil { + t.Error("Result is not nil but it should be!") + } +} + +func TestDecorateDatahubUnAuthWhenErrMessageIsNotEmpty(t *testing.T) { + msg := "something" + result := decorateDatahubUnauthorizedErrorIfMessageNotExists(&datahubmodels.Error{ + Message: msg, + }) + if result.Message != msg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", msg, result.Message) + } +} + +func TestDecorateDatahubUnAuthWhenErrMessageIsEmpty(t *testing.T) { + result := decorateDatahubUnauthorizedErrorIfMessageNotExists(&datahubmodels.Error{}) + if result.Message != authFailMsg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", authFailMsg, result.Message) + } +} + +func TestDecorateDatabaseUnAuthWhenErrIsNil(t *testing.T) { + if decorateDatabaseUnauthorizedErrorIfMessageNotExists(nil) != nil { + t.Error("Result is not nil but it should be!") + } +} + +func TestDecorateDatabaseUnAuthWhenErrMessageIsNotEmpty(t *testing.T) { + msg := "something" + result := decorateDatabaseUnauthorizedErrorIfMessageNotExists(&opdbmodels.Error{ + Message: msg, + }) + if result.Message != msg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", msg, result.Message) + } +} + +func TestDecorateDatabaseUnAuthWhenErrMessageIsEmpty(t *testing.T) { + result := decorateDatabaseUnauthorizedErrorIfMessageNotExists(&opdbmodels.Error{}) + if result.Message != authFailMsg { + t.Errorf("Result message is not the expected one! Expected: '%s', got: '%s'.", authFailMsg, result.Message) + } +} diff --git a/utils/queue.go b/utils/queue.go deleted file mode 100644 index 7ca8bb08..00000000 --- a/utils/queue.go +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2023 Cloudera. All Rights Reserved. -// -// This file is licensed under the Apache License Version 2.0 (the "License"). -// You may not use this file except in compliance with the License. -// You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0. -// -// This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS -// OF ANY KIND, either express or implied. Refer to the License for the specific -// permissions and limitations governing your use of the file. - -package utils - -import "fmt" - -type Queue[T any] interface { - Enqueue(T) error - Dequeue() (T, error) - Len() int -} - -var ( - ErrQueueEmpty = fmt.Errorf("QUEUE EMPTY") -) - -type SliceQueue[T any] []T - -func NewSliceQueue[T any]() Queue[T] { - return &SliceQueue[T]{} -} - -func (q *SliceQueue[T]) Len() int { - return len(*q) -} - -func (q *SliceQueue[T]) Enqueue(value T) error { - *q = append(*q, value) - return nil -} - -func (q *SliceQueue[T]) Dequeue() (T, error) { - queue := *q - if len(*q) > 0 { - card := queue[0] - *q = queue[1:] - return card, nil - } - - var empty T - return empty, ErrQueueEmpty -} diff --git a/utils/utils.go b/utils/utils.go index cc3bc980..fb979bc2 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -20,7 +20,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/cdp" ) @@ -57,31 +56,6 @@ func GetCdpClientForDataSource(req datasource.ConfigureRequest, resp *datasource return client } -func GetMapFromSingleItemList(d *schema.ResourceData, name string) (map[string]interface{}, error) { - if l := d.Get(name).([]interface{}); len(l) == 1 && l[0] != nil { - return l[0].(map[string]interface{}), nil - } - return nil, fmt.Errorf("error getting %s", name) -} - -func GetMapFromSingleItemListInMap(d map[string]interface{}, name string) (map[string]interface{}, error) { - if l := d[name].([]interface{}); len(l) == 1 && l[0] != nil { - return l[0].(map[string]interface{}), nil - } - return nil, fmt.Errorf("error getting %s", name) -} - -func ToStringList(configured []interface{}) []string { - vs := make([]string, 0, len(configured)) - for _, v := range configured { - val, ok := v.(string) - if ok && val != "" { - vs = append(vs, v.(string)) - } - } - return vs -} - func CalculateTimeoutOrDefault(ctx context.Context, options *PollingOptions, fallback time.Duration) (*time.Duration, error) { tflog.Debug(ctx, fmt.Sprintf("About to calculate polling timeout using the desired timeout (%+v) and the given fallback timeout (%+v)", options, fallback)) var timeout time.Duration @@ -118,14 +92,6 @@ func CalculateCallFailureThresholdOrDefault(ctx context.Context, options *Pollin return threshold, nil } -func ToBaseTypesStringMap(in map[string]string) map[string]types.String { - res := map[string]types.String{} - for k, v := range in { - res[k] = types.StringValue(v) - } - return res -} - func FromListValueToStringList(tl types.List) []string { if tl.IsNull() { return []string{}