From 336270f7a799891ca05b073a66ddd73f3a7d1b70 Mon Sep 17 00:00:00 2001 From: Tao Yi Date: Thu, 25 Jan 2024 18:14:58 +0800 Subject: [PATCH] feat: include CRUD action message from go-database-reconciler in logging error details from Konnect (#5453) * extract APIErrors * log CRUD action error from Konnect * update go.mod and changelog * address comment --- CHANGELOG.md | 3 ++ internal/dataplane/deckerrors/api.go | 29 +++++++++--- internal/dataplane/deckerrors/api_test.go | 56 ++++++++++++++++++++++- internal/dataplane/kong_client.go | 24 ++++++++++ 4 files changed, 104 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb78680aa2..53600ac5f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -157,6 +157,9 @@ Adding a new version? You'll need three changes: It's only possible to set the same timeout for each rule in a single `HTTPRoute`. Other settings will be rejected by the admission webhook validation. [#5243](https://github.com/Kong/kubernetes-ingress-controller/pull/5243) +- Log the details in response fron Konnect when failed to push configuration + to Konnect. + [#5453](https://github.com/Kong/kubernetes-ingress-controller/pull/5453) ### Fixed diff --git a/internal/dataplane/deckerrors/api.go b/internal/dataplane/deckerrors/api.go index 35bd7efcf6..4a9e579550 100644 --- a/internal/dataplane/deckerrors/api.go +++ b/internal/dataplane/deckerrors/api.go @@ -3,8 +3,10 @@ package deckerrors import ( "errors" + "github.com/kong/go-database-reconciler/pkg/crud" deckutils "github.com/kong/go-database-reconciler/pkg/utils" "github.com/kong/go-kong/kong" + "github.com/samber/lo" ) // ExtractAPIErrors tries to extract kong.APIErrors from the generic error. @@ -18,13 +20,26 @@ func ExtractAPIErrors(err error) []*kong.APIError { // It might be either a deckutils.ErrArray with APIErrors inside. var deckErrArray deckutils.ErrArray if errors.As(err, &deckErrArray) { - var apiErrs []*kong.APIError - for _, err := range deckErrArray.Errors { - if apiErr, ok := castAsErr[*kong.APIError](err); ok { - apiErrs = append(apiErrs, apiErr) - } - } - return apiErrs + return lo.FilterMap(deckErrArray.Errors, func(e error, _ int) (*kong.APIError, bool) { + return castAsErr[*kong.APIError](e) + }) + } + + return nil +} + +func ExtractCRUDActionErrors(err error) []*crud.ActionError { + // It might be a single crud.ActionError. + if actionErr, ok := castAsErr[*crud.ActionError](err); ok { + return []*crud.ActionError{actionErr} + } + + // It might be either a deckutils.ErrArray with ActionErrors inside. + var deckErrArray deckutils.ErrArray + if errors.As(err, &deckErrArray) { + return lo.FilterMap(deckErrArray.Errors, func(e error, _ int) (*crud.ActionError, bool) { + return castAsErr[*crud.ActionError](e) + }) } return nil diff --git a/internal/dataplane/deckerrors/api_test.go b/internal/dataplane/deckerrors/api_test.go index 3144e525b1..0b24e629db 100644 --- a/internal/dataplane/deckerrors/api_test.go +++ b/internal/dataplane/deckerrors/api_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/kong/go-database-reconciler/pkg/crud" deckutils "github.com/kong/go-database-reconciler/pkg/utils" "github.com/kong/go-kong/kong" "github.com/stretchr/testify/require" @@ -41,7 +42,7 @@ func TestExtractAPIErrors(t *testing.T) { { name: "deck array of errors with no api error", input: deckutils.ErrArray{Errors: []error{genericErr, genericErr}}, - expected: nil, + expected: []*kong.APIError{}, }, { name: "deck array of errors with an api error among generic ones", @@ -51,9 +52,62 @@ func TestExtractAPIErrors(t *testing.T) { } for _, tc := range testCases { + tc := tc t.Run(tc.name, func(t *testing.T) { out := deckerrors.ExtractAPIErrors(tc.input) require.Equal(t, tc.expected, out) }) } } + +func TestExtractCRUDActionErrors(t *testing.T) { + var ( + genericErr = errors.New("not an api error") + actionErr = &crud.ActionError{ + OperationType: crud.Create, + Kind: crud.Kind("service"), + Name: "badservice", + Err: errors.New("something wrong"), + } + ) + + testCases := []struct { + name string + input error + expected []*crud.ActionError + }{ + { + name: "nil", + input: nil, + expected: nil, + }, + { + name: "single generic error", + input: genericErr, + expected: nil, + }, + { + name: "single action error", + input: actionErr, + expected: []*crud.ActionError{actionErr}, + }, + { + name: "deck array of errors with no action errors", + input: deckutils.ErrArray{Errors: []error{genericErr, genericErr}}, + expected: []*crud.ActionError{}, + }, + { + name: "deck array of errors with an action error among generic ones", + input: deckutils.ErrArray{Errors: []error{genericErr, actionErr, genericErr}}, + expected: []*crud.ActionError{actionErr}, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + out := deckerrors.ExtractCRUDActionErrors(tc.input) + require.Equal(t, tc.expected, out) + }) + } +} diff --git a/internal/dataplane/kong_client.go b/internal/dataplane/kong_client.go index b5da32826f..4fe828f1fc 100644 --- a/internal/dataplane/kong_client.go +++ b/internal/dataplane/kong_client.go @@ -27,6 +27,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v3/internal/clients" dpconf "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/config" "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/configfetcher" + "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/deckerrors" "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/deckgen" "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/failures" "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate" @@ -511,6 +512,7 @@ func (c *KongClient) maybeSendOutToKonnectClient(ctx context.Context, s *kongsta c.logger.Error(err, "Skipped pushing configuration to Konnect") } else { c.logger.Error(err, "Failed pushing configuration to Konnect") + logKonnectErrors(c.logger, err) } return err } @@ -518,6 +520,28 @@ func (c *KongClient) maybeSendOutToKonnectClient(ctx context.Context, s *kongsta return nil } +// logKonnectErrors logs details of each error response returned from Konnect API. +func logKonnectErrors(logger logr.Logger, err error) { + if crudActionErrors := deckerrors.ExtractCRUDActionErrors(err); len(crudActionErrors) > 0 { + for _, actionErr := range crudActionErrors { + apiErr := &kong.APIError{} + if errors.As(actionErr.Err, &apiErr) { + logger.Error(actionErr, "Failed to send request to Konnect", + "operation_type", actionErr.OperationType.String(), + "entity_kind", actionErr.Kind, + "entity_name", actionErr.Name, + "details", apiErr.Details()) + } else { + logger.Error(actionErr, "Failed to send request to Konnect", + "operation_type", actionErr.OperationType.String(), + "entity_kind", actionErr.Kind, + "entity_name", actionErr.Name, + ) + } + } + } +} + func (c *KongClient) sendToClient( ctx context.Context, client sendconfig.AdminAPIClient,