fix(apiaccess): corrects incorrect errors thrown by failing API Acces…

…s Key creation
newrelic · Apr 4, 2023 · 336d48c · 336d48c
1 parent f2ea03b
commit 336d48c
Show file tree

Hide file tree

Showing 4 changed files with 158 additions and 14 deletions.
diff --git a/pkg/apiaccess/keys.go b/pkg/apiaccess/keys.go
@@ -3,7 +3,7 @@ package apiaccess
 import (
 	"context"
 	"errors"
-
+	"fmt"
 	"github.com/newrelic/newrelic-client-go/v2/internal/http"
 )
 
@@ -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"`
 }
 
@@ -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 (

diff --git a/pkg/apiaccess/keys_integration_test.go b/pkg/apiaccess/keys_integration_test.go
@@ -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) {
@@ -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
+}
diff --git a/pkg/apiaccess/types.go b/pkg/apiaccess/types.go
@@ -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) {

diff --git a/pkg/testhelpers/helpers.go b/pkg/testhelpers/helpers.go
@@ -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 == "" {