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

fix(apiaccess): fix to the error thrown when API Access Key creation fails #1015

Merged
merged 1 commit into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 30 additions & 10 deletions pkg/apiaccess/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package apiaccess
import (
"context"
"errors"

"fmt"
"github.com/newrelic/newrelic-client-go/v2/internal/http"
)

Expand Down Expand Up @@ -145,27 +145,45 @@ func (a *APIAccess) DeleteAPIAccessKeyWithContext(ctx context.Context, keys APIA
return resp.APIAccessDeleteKeys.DeletedKeys, nil
}

func formatAPIAccessKeyErrors(errs []APIAccessKeyErrorInterface) string {
errorString := ""
var AccessKeyErrorPrefix = "The following errors have been thrown.\n"

func formatAPIAccessKeyErrors(errs []APIAccessKeyErrorResponse) string {
errorString := AccessKeyErrorPrefix
for _, e := range errs {
errorString += e.GetError().Error() + "\n"
IDAsString := ""
if len(e.Id) != 0 {
// Id is returned in the 'error' block only in the case of update and delete but not with create.
// So; in the case of create, it is made an empty string to generalize the usage of IDAsString.
IDAsString = fmt.Sprintf("%s: ", e.Id)
}
if e.Type == "USER" {
errorString += fmt.Sprintf("%s: %s%s\n", e.UserKeyErrorType, IDAsString, e.Message)
} else if e.Type == "INGEST" {
errorString += fmt.Sprintf("%s: %s%s\n", e.IngestKeyErrorType, IDAsString, e.Message)
} else if len(e.Type) == 0 {
// When Ingest Keys are deleted, the "type" attribute is currently null in the response sent,
// in the "errors" attribute - hence, this condition. However, this is not the case with User Keys.
errorString += fmt.Sprintf("%s: %s%s\n", e.IngestKeyErrorType, IDAsString, e.Message)
} else {
errorString += e.Message
}
}
return errorString
}

// apiAccessKeyCreateResponse represents the JSON response returned from creating key(s).
type apiAccessKeyCreateResponse struct {
APIAccessCreateKeys struct {
CreatedKeys []APIKey `json:"createdKeys"`
Errors []APIAccessKeyErrorInterface `json:"errors"`
CreatedKeys []APIKey `json:"createdKeys"`
Errors []APIAccessKeyErrorResponse `json:"errors,omitempty"`
} `json:"apiAccessCreateKeys"`
}

// apiAccessKeyUpdateResponse represents the JSON response returned from updating key(s).
type apiAccessKeyUpdateResponse struct {
APIAccessUpdateKeys struct {
UpdatedKeys []APIKey `json:"updatedKeys"`
Errors []APIAccessKeyErrorInterface `json:"errors"`
UpdatedKeys []APIKey `json:"updatedKeys"`
Errors []APIAccessKeyErrorResponse `json:"errors,omitempty"`
} `json:"apiAccessUpdateKeys"`
}

Expand All @@ -190,9 +208,11 @@ type apiAccessKeySearchResponse struct {
http.GraphQLErrorResponse
}

// apiAccessKeyDeleteResponse represents the JSON response returned from creating key(s).
type apiAccessKeyDeleteResponse struct {
APIAccessDeleteKeys APIAccessDeleteKeyResponse `json:"apiAccessDeleteKeys"`
APIAccessDeleteKeys struct {
DeletedKeys []APIAccessDeletedKey `json:"deletedKeys,omitempty"`
Errors []APIAccessKeyErrorResponse `json:"errors,omitempty"`
} `json:"apiAccessDeleteKeys"`
}

const (
Expand Down
116 changes: 112 additions & 4 deletions pkg/apiaccess/keys_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
package apiaccess

import (
"testing"

"github.com/stretchr/testify/require"

mock "github.com/newrelic/newrelic-client-go/v2/pkg/testhelpers"
"github.com/stretchr/testify/require"
"regexp"
"strings"
"testing"
)

func TestIntegrationAPIAccess_IngestKeys(t *testing.T) {
Expand Down Expand Up @@ -133,3 +133,111 @@ func TestIntegrationAPIAccess_UserKeys(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, deleteResult)
}

func TestIntegrationAPIAccess_UpdateIngestKeyError(t *testing.T) {
t.Parallel()
client := newIntegrationTestClient(t)
mockIngestKeyID, _ := mock.GetNonExistentIDs()
_, err := client.UpdateAPIAccessKeys(APIAccessUpdateInput{
Ingest: []APIAccessUpdateIngestKeyInput{
{
KeyID: mockIngestKeyID,
Name: "Lorem Ipsum",
Notes: "Lorem Ipsum",
},
},
})
require.Error(t, err)
require.Equal(t, validateAPIAccessKeyError(err, "INGEST"), true)
}

func TestIntegrationAPIAccess_UpdateUserKeyError(t *testing.T) {
t.Parallel()
client := newIntegrationTestClient(t)
_, mockUserKeyID := mock.GetNonExistentIDs()
_, err := client.UpdateAPIAccessKeys(APIAccessUpdateInput{
User: []APIAccessUpdateUserKeyInput{
{
KeyID: mockUserKeyID,
Name: "Lorem Ipsum",
Notes: "Lorem Ipsum",
},
},
})
require.Error(t, err)
require.Equal(t, validateAPIAccessKeyError(err, "USER"), true)
}

func TestIntegrationAPIAccess_DeleteIngestKeyError(t *testing.T) {
t.Parallel()
client := newIntegrationTestClient(t)
mockIngestKeyIDOne, mockIngestKeyIDTwo := mock.GetNonExistentIDs()
_, err := client.DeleteAPIAccessKey(APIAccessDeleteInput{
IngestKeyIDs: []string{
mockIngestKeyIDOne,
mockIngestKeyIDTwo,
},
})
require.Error(t, err)
require.Equal(t, validateAPIAccessKeyError(err, "INGEST"), true)
}

func TestIntegrationAPIAccess_DeleteUserKeyError(t *testing.T) {
t.Parallel()
client := newIntegrationTestClient(t)
_, mockUserKeyID := mock.GetNonExistentIDs()
_, err := client.DeleteAPIAccessKey(APIAccessDeleteInput{
UserKeyIDs: []string{
mockUserKeyID,
},
})
require.Error(t, err)
require.Equal(t, validateAPIAccessKeyError(err, "USER"), true)
}

var possibleIngestKeyErrors = []string{
string(APIAccessIngestKeyErrorTypeTypes.NOT_FOUND),
string(APIAccessIngestKeyErrorTypeTypes.INVALID),
string(APIAccessIngestKeyErrorTypeTypes.FORBIDDEN),
}

var possibleUserKeyErrors = []string{
string(APIAccessUserKeyErrorTypeTypes.NOT_FOUND),
string(APIAccessUserKeyErrorTypeTypes.INVALID),
string(APIAccessUserKeyErrorTypeTypes.FORBIDDEN),
}

func validateAPIAccessKeyError(err error, errorType string) (status bool) {
errorMessage := err.Error()
listOfErrorMessages := strings.Split(errorMessage, "\n")
listOfErrorMessages = listOfErrorMessages[1 : len(listOfErrorMessages)-1]
validatedErrorMessages := 0

if errorType == "USER" {
for errorIndex := 0; errorIndex < len(listOfErrorMessages); errorIndex++ {
for listIndex := 0; listIndex < len(possibleUserKeyErrors); listIndex++ {
match, _ := regexp.MatchString(possibleUserKeyErrors[listIndex], listOfErrorMessages[errorIndex])
if match {
validatedErrorMessages += 1
break
}
}
}
} else if errorType == "INGEST" {
for errorIndex := 0; errorIndex < len(listOfErrorMessages); errorIndex++ {
for listIndex := 0; listIndex < len(possibleIngestKeyErrors); listIndex++ {
match, _ := regexp.MatchString(possibleIngestKeyErrors[listIndex], listOfErrorMessages[errorIndex])
if match {
validatedErrorMessages += 1
break
}
}
}
}

if validatedErrorMessages == len(listOfErrorMessages) {
return true
}

return false
}
10 changes: 10 additions & 0 deletions pkg/apiaccess/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,16 @@ type APIAccessKeyErrorInterface interface {
GetError() error
}

type APIAccessKeyErrorResponse struct {
// The message with the error cause.
Message string `json:"message,omitempty"`
// Type of error.
Type string `json:"type,omitempty"`
UserKeyErrorType APIAccessUserKeyErrorType `json:"userErrorType,omitempty"`
IngestKeyErrorType APIAccessIngestKeyErrorType `json:"ingestErrorType,omitempty"`
Id string `json:"id,omitempty"`
}

// UnmarshalAPIAccessKeyErrorInterface unmarshals the interface into the correct type
// based on __typename provided by GraphQL
func UnmarshalAPIAccessKeyErrorInterface(b []byte) (*APIAccessKeyErrorInterface, error) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/testhelpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ func GetTestAccountID() (int, error) {
return getEnvInt("NEW_RELIC_ACCOUNT_ID")
}

// GetNonExistentIDs returns two non-existent IDs that can be used to test errors raised
// when Ingest/User keys which do not exist are updated or deleted.
func GetNonExistentIDs() (string, string) {
return "00000FF000F0FFFF000F0F0000FF0FF0000000F000F00F0FF000FFFFF0FF00F0", "00000HH000H0HHHH000H0H0000HH0HH0000000H000H00H0HH000HHHHH0HH00H0"
}

// getEnvInt helper to DRY up the other env get calls for integers
func getEnvInt(name string) (int, error) {
if name == "" {
Expand Down