Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AV-66756] Code Health - Extract API error handling #78

Merged
merged 89 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
89 commits
Select commit Hold shift + click to select a range
9f19daf
Merge branch 'main' into AV-63471_Implement_update_user_using_PATCH_r…
matty271828 Oct 20, 2023
e4e5159
Add logic to patch user
matty271828 Oct 20, 2023
c2458c5
Add method structure to construct patch
matty271828 Oct 20, 2023
8dd6d71
Add function to construct patch entries
matty271828 Oct 23, 2023
3125c20
Convert roles and resources to type string
matty271828 Oct 23, 2023
7bced3e
Add constructors for patch entries
matty271828 Oct 23, 2023
7bc60d0
Add nil pointer check to constructPatch
matty271828 Oct 23, 2023
572fd78
Use variable for email in example
matty271828 Oct 24, 2023
07415e7
Put roles for same project into one array
matty271828 Oct 24, 2023
f5fcafe
Fix userID retrieval from state
matty271828 Oct 24, 2023
b7c0091
Merge branch 'main' into AV-63471_Implement_update_user_using_PATCH_r…
matty271828 Oct 27, 2023
9cf8b60
Directly pass patch to user request
matty271828 Oct 27, 2023
b535f6b
Make path of type ValueString
matty271828 Oct 27, 2023
40937e3
Compare organizationRoles for patch request
matty271828 Oct 30, 2023
7142ab1
Make resource object optional
matty271828 Nov 1, 2023
cd43439
Generalise contains function logic
matty271828 Nov 1, 2023
ab140d7
Compare projectRoles and Resources
matty271828 Nov 1, 2023
e25b4cc
Switch order of remove and add resources
matty271828 Nov 1, 2023
c28b5a2
Make patch single valued
matty271828 Nov 1, 2023
2ac301e
Add test for construct patch
matty271828 Nov 1, 2023
edce15d
Extract patch comparison logic
matty271828 Nov 2, 2023
6e46868
Format strings over multiple lines
matty271828 Nov 2, 2023
8627ca6
Merge branch 'main' into AV-63471_Implement_update_user_using_PATCH_r…
matty271828 Nov 2, 2023
5f25a32
Remove unused getUsers struct
matty271828 Nov 2, 2023
ddc9a31
Use maps within handleResources
matty271828 Nov 2, 2023
5ee8997
Use maps in handleProjectRoles
matty271828 Nov 2, 2023
51008ae
Change break to continue
matty271828 Nov 3, 2023
690cb58
Handle optional type for resource
matty271828 Nov 3, 2023
b11d67d
Use variadic appends to patch slice
matty271828 Nov 3, 2023
2d59cf5
Change projectRoles to resources in docstring
matty271828 Nov 3, 2023
54aff4b
Make type optional and computed
matty271828 Nov 3, 2023
c46312c
Use slices.Contains function
matty271828 Nov 3, 2023
adec1d1
Use tools common for slice comparison
matty271828 Nov 3, 2023
438bb5b
Convert projectroles to strings
matty271828 Nov 3, 2023
6ae54d7
Remove import alias for slices
matty271828 Nov 3, 2023
0cf6f14
Use constant types for operations
matty271828 Nov 3, 2023
cc4a788
Handle resource not found error for user
matty271828 Nov 3, 2023
999bbf9
Extract API error handling
matty271828 Nov 3, 2023
c411648
Add function to CheckApiError
matty271828 Nov 3, 2023
536fd1e
Add docstring to CheckApiError
matty271828 Nov 3, 2023
4705ca9
Make CheckApiError two valued
matty271828 Nov 3, 2023
1c0de5a
Remove nested if statement
matty271828 Nov 3, 2023
25658d5
Rename checkApiError to ParseApiError
matty271828 Nov 3, 2023
b7a6ff6
Use resourcenotfoundcheck in allowlist read
matty271828 Nov 3, 2023
ac46eee
Rename ParseApiError to ParseError
matty271828 Nov 6, 2023
591c6f8
Refactor use of resourcenotfound in resources
matty271828 Nov 6, 2023
5da8704
Merge branch 'main' into AV-66756_Extract_API_error_handling
matty271828 Nov 6, 2023
c84575d
Move error parsers to api package
matty271828 Nov 6, 2023
95483ec
Use api parser functions in datasources
matty271828 Nov 6, 2023
0df4f14
Add unit tests for error parsers
matty271828 Nov 6, 2023
cc305e0
Use type assertion to retrieve error
matty271828 Nov 6, 2023
85d4c61
Use completeError in test case
matty271828 Nov 6, 2023
6308f92
Use variables for 500 and 404 errors
matty271828 Nov 6, 2023
8c4e5bf
Remove outdated error block
matty271828 Nov 6, 2023
080f880
Move resource removal block before error
matty271828 Nov 7, 2023
05d5b04
Remove resource not found checks from creates
matty271828 Nov 7, 2023
8704184
Remove resourcenotfound check from delete
matty271828 Nov 7, 2023
81ee128
Check ResourceNotFound in users datasource
matty271828 Nov 7, 2023
84a19f1
Remove resourcenotfound check from apikey delete
matty271828 Nov 7, 2023
5a97aab
Remove ResourceNotFound check bucket delete
matty271828 Nov 7, 2023
f1538ea
Add ResourceNotFound check to update bucket
matty271828 Nov 7, 2023
de62c4f
Check ResourceNotFound cluster update
matty271828 Nov 7, 2023
3758fcc
Remove ResourceNotFound check cluster delete
matty271828 Nov 7, 2023
3f45dcb
Remove ResourceNotFound checks from deletes
matty271828 Nov 7, 2023
e64d436
Remove datasources ResourceNotFound handling
matty271828 Nov 7, 2023
d0f079a
Add ResourceNotFound checks to deletes
matty271828 Nov 7, 2023
d556f3d
Parse allowlist create error
matty271828 Nov 7, 2023
fdbfe50
Pass in sort by parameter to pagination function
matty271828 Nov 8, 2023
5032242
Merge branch 'main' into AV-66756_Extract_API_error_handling
matty271828 Nov 9, 2023
dcf3706
Use error parsers in appservices
matty271828 Nov 9, 2023
bb8c0ee
Merge branch 'main' into AV-66756_Extract_API_error_handling
matty271828 Nov 10, 2023
4897fea
Change err to ErrExecutingRequest
matty271828 Nov 13, 2023
13e257c
Remove ResourceNotFound handling from certificate
matty271828 Nov 13, 2023
aea7879
Remove redundant error wrapping
matty271828 Nov 13, 2023
49dd7fa
Use parser functions in datasources
matty271828 Nov 14, 2023
2be0624
Use parser function in appservice read
matty271828 Nov 14, 2023
f42b1fc
Use errors.As in parser functions
matty271828 Nov 14, 2023
8d4029d
Revert "Remove redundant error wrapping"
matty271828 Nov 14, 2023
b32410a
Use pointer for Error receiver
matty271828 Nov 14, 2023
c0fa77c
Remove statement from debugging
matty271828 Nov 14, 2023
1f1b45a
Use %w to wrap errors
matty271828 Nov 14, 2023
5c085bf
Wrap errors throughout the provider
matty271828 Nov 14, 2023
fd19308
Check resourcenotfound during status check
matty271828 Nov 14, 2023
7f8455b
Merge branch 'main' into AV-66756_Extract_API_error_handling
matty271828 Nov 14, 2023
56cd3e1
Remove not found logging during metadatacheck
matty271828 Nov 15, 2023
105365b
Return early when resourcenotfound
matty271828 Nov 15, 2023
d20c83c
Merge branch 'main' into AV-66756_Extract_API_error_handling
matty271828 Nov 17, 2023
0bf17c2
Use api parsers in backup
matty271828 Nov 17, 2023
f4eff87
Merge branch 'main' into AV-66756_Extract_API_error_handling
matty271828 Nov 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions examples/allowlist/create_allowlist.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ output "new_allowlist" {
value = capella_allowlist.new_allowlist
}

output "allowlist_id" {
value = capella_allowlist.new_allowlist.id
}

resource "capella_allowlist" "new_allowlist" {
organization_id = var.organization_id
project_id = var.project_id
Expand Down
4 changes: 4 additions & 0 deletions examples/apikey/create_apikey.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ output "new_apikey" {
sensitive = true
}

output "apikey_id" {
value = capella_apikey.new_apikey.id
}

resource "capella_apikey" "new_apikey" {
organization_id = var.organization_id
name = var.apikey.name
Expand Down
6 changes: 5 additions & 1 deletion examples/appservice/create_app_service.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ output "new_app_service" {
value = capella_app_service.new_app_service
}

output "appservice_id" {
value = capella_app_service.new_app_service.id
}

resource "capella_app_service" "new_app_service" {
organization_id = var.organization_id
project_id = var.project_id
Expand All @@ -14,4 +18,4 @@ resource "capella_app_service" "new_app_service" {
ram = var.app_service.compute.ram
}
version = var.app_service.version
}
}
6 changes: 5 additions & 1 deletion examples/bucket/create_bucket.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ output "new_bucket" {
value = capella_bucket.new_bucket
}

output "bucket_id" {
value = capella_bucket.new_bucket.id
}

resource "capella_bucket" "new_bucket" {
name = var.bucket.name
organization_id = var.organization_id
Expand All @@ -16,4 +20,4 @@ resource "capella_bucket" "new_bucket" {
flush = var.bucket.flush
time_to_live_in_seconds = var.bucket.time_to_live_in_seconds
eviction_policy = var.bucket.eviction_policy
}
}
4 changes: 4 additions & 0 deletions examples/cluster/create_cluster.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ output "new_cluster" {
value = capella_cluster.new_cluster
}

output "cluster_id" {
value = capella_cluster.new_cluster.id
}

resource "capella_cluster" "new_cluster" {
organization_id = var.organization_id
project_id = var.project_id
Expand Down
4 changes: 4 additions & 0 deletions examples/database_credential/create_database_credential.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ output "new_database_credential" {
sensitive = true
}

output "database_credential_id" {
value = capella_database_credential.new_database_credential.id
}

resource "capella_database_credential" "new_database_credential" {
name = var.database_credential_name
organization_id = var.organization_id
Expand Down
4 changes: 4 additions & 0 deletions examples/project/create_project.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ output "new_project" {
value = capella_project.new_project
}

output "project_id" {
value = capella_project.new_project.id
}

resource "capella_project" "new_project" {
organization_id = var.organization_id
name = var.project_name
Expand Down
4 changes: 4 additions & 0 deletions examples/user/create_user.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ output "new_user" {
value = capella_user.new_user
}

output "user_id" {
value = capella_user.new_user.id
}

resource "capella_user" "new_user" {
organization_id = var.organization_id

Expand Down
6 changes: 3 additions & 3 deletions internal/api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (c *Client) Execute(

req, err := http.NewRequest(endpointCfg.Method, endpointCfg.Url, bytes.NewReader(requestBody))
if err != nil {
return nil, fmt.Errorf("%s: %v", errors.ErrConstructingRequest, err)
return nil, fmt.Errorf("%s: %w", errors.ErrConstructingRequest, err)
}

req.Header.Set("Authorization", "Bearer "+authToken)
Expand All @@ -72,7 +72,7 @@ func (c *Client) Execute(

apiRes, err := c.Do(req)
if err != nil {
return nil, fmt.Errorf("%s: %v", errors.ErrExecutingRequest, err)
return nil, fmt.Errorf("%s: %w", errors.ErrExecutingRequest, err)
}
defer apiRes.Body.Close()

Expand All @@ -88,7 +88,7 @@ func (c *Client) Execute(
"unexpected code: %d, expected: %d, body: %s",
apiRes.StatusCode, endpointCfg.SuccessStatus, responseBody)
}
return nil, apiError
return nil, &apiError
}

return &Response{
Expand Down
42 changes: 39 additions & 3 deletions internal/api/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package api

import (
"encoding/json"
"errors"
"fmt"
"net/http"
)

// Error tracks the error structure received from Capella V4 APIs
Expand All @@ -19,14 +21,48 @@ type Error struct {
Message string `json:"message"`
}

func (e Error) Error() string {
return fmt.Sprintf("%s", e.Message)
func (e *Error) Error() string {
return fmt.Sprintf(`{"code":%d,"hint":"%s","httpStatusCode":%d,"message":"%s"}`,
e.Code, e.Hint, e.HttpStatusCode, e.Message,
)
}

func (e Error) CompleteError() string {
jsonData, err := json.Marshal(e)
if err != nil {
return e.Message
return fmt.Sprintf(`{"code":%d,"hint":"%s","httpStatusCode":%d,"message":"%s"}`,
e.Code, e.Hint, e.HttpStatusCode, e.Message,
)
}
return string(jsonData)
}

// ParseError is used to check if an error is of type
// api.Error error and return it as a string.
func ParseError(err error) string {
var apiError *Error
switch {
case errors.As(err, &apiError):
return apiError.CompleteError()
default:
return err.Error()
}
}

// CheckResourceNotFoundError is used to check if an error is of
// type api.Error and whether the error is resource not found.
//
// Note: If the error is other than not found, the error string
// will be returned along with a bool value of false.
func CheckResourceNotFoundError(err error) (bool, string) {
var apiError *Error
switch {
case errors.As(err, &apiError):
if apiError.HttpStatusCode != http.StatusNotFound {
return false, apiError.CompleteError()
}
return true, apiError.CompleteError()
default:
return false, err.Error()
}
}
112 changes: 112 additions & 0 deletions internal/api/error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package api

import (
"fmt"
"net/http"
internalerrors "terraform-provider-capella/internal/errors"
"testing"

"github.com/stretchr/testify/assert"
)

func Test_ParseError(t *testing.T) {
var (
errMessage = "Error received from Capella V4 Api"
apiError = Error{Message: errMessage}
)

type test struct {
name string
err error
expOutput interface{}
}

tests := []test{
{
name: "Error received from Capella V4 Api",
err: &Error{Message: errMessage},
expOutput: Error{Message: errMessage}.CompleteError(),
},
{
name: "Wrapped error received from Capella V4 Api",
err: fmt.Errorf("received error: %w", &apiError),
expOutput: Error{Message: errMessage}.CompleteError(),
},
{
name: "Error other than received from Capella V4 Api",
err: internalerrors.ErrAllowListIdCannotBeEmpty,
expOutput: internalerrors.ErrAllowListIdCannotBeEmpty.Error(),
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
errString := ParseError(test.err)

assert.Equal(t, test.expOutput, errString)
})
}
}

func Test_CheckResourceNotFound(t *testing.T) {
var (
errMessage = "example error message"
err500 = Error{
HttpStatusCode: http.StatusInternalServerError,
Message: errMessage,
}
err404 = Error{
HttpStatusCode: http.StatusNotFound,
Message: errMessage,
}
)

type test struct {
name string
err error
expOutput interface{}
expBool bool
}

tests := []test{
{
name: "Error received from Capella V4 Api",
err: &err500,
expOutput: err500.CompleteError(),
expBool: false,
},
{
name: "Error received from Capella V4 Api - Resource Not Found",
err: &err404,
expOutput: err404.CompleteError(),
expBool: true,
},
{
name: "Wrapped 500 error received from Capella V4 Api",
err: fmt.Errorf("received error: %w", &err500),
expOutput: err500.CompleteError(),
expBool: false,
},
{
name: "Wrapped 404 error received from Capella V4 Api",
err: fmt.Errorf("received error: %w", &err404),
expOutput: err404.CompleteError(),
expBool: true,
},
{
name: "Error other than received from Capella V4 Api",
err: internalerrors.ErrAllowListIdCannotBeEmpty,
expOutput: internalerrors.ErrAllowListIdCannotBeEmpty.Error(),
expBool: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
resourceNotFound, errString := CheckResourceNotFoundError(test.err)

assert.Equal(t, test.expOutput, errString)
assert.Equal(t, test.expBool, resourceNotFound)
})
}
}
6 changes: 4 additions & 2 deletions internal/api/pagination.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package api

import (
"terraform-provider-capella/internal/errors"

"context"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -92,13 +94,13 @@ func GetPaginated[DataSchema ~[]T, T any](
nil,
)
if err != nil {
return nil, err
return nil, fmt.Errorf("%s: %w", errors.ErrExecutingRequest, err)
}

var decoded overlay
err = json.Unmarshal(response.Body, &decoded)
if err != nil {
return nil, err
return nil, fmt.Errorf("%s: %w", errors.ErrUnmarshallingResponse, err)
}

responses = append(responses, decoded.Data...)
Expand Down
22 changes: 4 additions & 18 deletions internal/datasources/allowlists.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-log/tflog"
)

// Ensure the implementation satisfies the expected interfaces.
Expand Down Expand Up @@ -87,23 +86,10 @@ func (d *AllowLists) Read(ctx context.Context, req datasource.ReadRequest, resp
)

allowLists, err := d.listAllowLists(ctx, organizationId, projectId, clusterId)
switch err := err.(type) {
case nil:
case api.Error:
if err.HttpStatusCode != http.StatusNotFound {
resp.Diagnostics.AddError(
"Error Reading Capella AllowLists",
"Could not read allow lists in cluster "+state.ClusterId.String()+": "+err.CompleteError(),
)
return
}
tflog.Info(ctx, "resource doesn't exist in remote server removing resource from state file")
resp.State.RemoveResource(ctx)
return
default:
if err != nil {
resp.Diagnostics.AddError(
"Error Reading AllowLists",
"Could not read allow lists in cluster "+state.ClusterId.String()+": "+err.Error(),
"Error Reading Capella AllowLists",
"Could not read allow lists in cluster "+state.ClusterId.String()+": "+api.ParseError(err),
)
return
}
Expand All @@ -112,7 +98,7 @@ func (d *AllowLists) Read(ctx context.Context, req datasource.ReadRequest, resp
if err != nil {
resp.Diagnostics.AddError(
"Error reading allowlist",
"Could not create allowlist, unexpected error: "+err.Error(),
"Could not read allowlist, unexpected error: "+err.Error(),
)
return
}
Expand Down
12 changes: 2 additions & 10 deletions internal/datasources/apikeys.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,10 @@ func (d *ApiKeys) Read(ctx context.Context, req datasource.ReadRequest, resp *da
cfg := api.EndpointCfg{Url: url, Method: http.MethodGet, SuccessStatus: http.StatusOK}

response, err := api.GetPaginated[[]api.GetApiKeyResponse](ctx, d.Client, d.Token, cfg, api.SortByName)
switch err := err.(type) {
case nil:
case api.Error:
resp.Diagnostics.AddError(
"Error Reading Capella ApiKeys",
"Could not read api keys in organization "+organizationId+": "+err.CompleteError(),
)
return
default:
if err != nil {
resp.Diagnostics.AddError(
"Error Reading Capella ApiKeys",
"Could not read api keys in organization "+organizationId+": "+err.Error(),
"Could not read api keys in organization "+organizationId+": "+api.ParseError(err),
)
return
}
Expand Down
Loading