Skip to content

Commit

Permalink
fix: handle KongService conflict on creation (#749)
Browse files Browse the repository at this point in the history
  • Loading branch information
pmalek authored Oct 17, 2024
1 parent ebfa7ed commit 3c2e736
Show file tree
Hide file tree
Showing 16 changed files with 431 additions and 71 deletions.
1 change: 1 addition & 0 deletions controller/konnect/ops/kongservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type ServicesSDK interface {
CreateService(ctx context.Context, controlPlaneID string, service sdkkonnectcomp.ServiceInput, opts ...sdkkonnectops.Option) (*sdkkonnectops.CreateServiceResponse, error)
UpsertService(ctx context.Context, req sdkkonnectops.UpsertServiceRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.UpsertServiceResponse, error)
DeleteService(ctx context.Context, controlPlaneID, serviceID string, opts ...sdkkonnectops.Option) (*sdkkonnectops.DeleteServiceResponse, error)
ListService(ctx context.Context, request sdkkonnectops.ListServiceRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.ListServiceResponse, error)
}
74 changes: 74 additions & 0 deletions controller/konnect/ops/kongservice_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 29 additions & 3 deletions controller/konnect/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ func Create[
switch ent := any(e).(type) {
case *konnectv1alpha1.KonnectGatewayControlPlane:
err = createControlPlane(ctx, sdk.GetControlPlaneSDK(), sdk.GetControlPlaneGroupSDK(), cl, ent)
// TODO: modify the create* operation wrappers to not set Programmed conditions and return
// a KonnectEntityCreatedButRelationsFailedError if the entity was created but its relations assignment failed.
case *configurationv1alpha1.KongService:
err = createService(ctx, sdk.GetServicesSDK(), ent)

// TODO: modify the create* operation wrappers to not set Programmed conditions and return
// a KonnectEntityCreatedButRelationsFailedError if the entity was created but its relations assignment failed.

case *configurationv1alpha1.KongRoute:
err = createRoute(ctx, sdk.GetRoutesSDK(), ent)
case *configurationv1.KongConsumer:
Expand Down Expand Up @@ -104,19 +106,32 @@ func Create[
return nil, fmt.Errorf("unsupported entity type %T", ent)
}

// NOTE: Konnect APIs specific for Konnect only APIs like Gateway Control Planes
// return 409 conflict for already existing entities. APIs that are shared with
// Kong Admin API return 400 for conflicts.
var (
errConflict *sdkkonnecterrs.ConflictError
errRelationsFailed KonnectEntityCreatedButRelationsFailedError
errSdk *sdkkonnecterrs.SDKError
errSdkBody sdkErrorBody
)

if err != nil && (errors.As(err, &errSdk)) {
errSdkBody, _ = ParseSDKErrorBody(errSdk.Body)
}

switch {
case errors.As(err, &errConflict):
case errors.As(err, &errConflict) ||
// Service API returns 400 for conflicts
errSdkBody.Code == 3 && errSdkBody.Message == "data constraint error":
// If there was a conflict on the create request, we can assume the entity already exists.
// We'll get its Konnect ID by listing all entities of its type filtered by the Kubernetes object UID.
var id string
switch ent := any(e).(type) {
case *konnectv1alpha1.KonnectGatewayControlPlane:
id, err = getControlPlaneForUID(ctx, sdk.GetControlPlaneSDK(), ent)
case *configurationv1alpha1.KongService:
id, err = getKongServiceForUID(ctx, sdk.GetServicesSDK(), ent)
// ---------------------------------------------------------------------
// TODO: add other Konnect types
default:
Expand Down Expand Up @@ -388,3 +403,14 @@ func wrapErrIfKonnectOpFailed[
}
return nil
}

func logEntityNotFoundRecreating[
T constraints.SupportedKonnectEntityType,
](ctx context.Context, _ *T, id string) {
ctrllog.FromContext(ctx).
Info(
"entity not found in Konnect, trying to recreate",
"type", constraints.EntityTypeName[T](),
"id", id,
)
}
23 changes: 11 additions & 12 deletions controller/konnect/ops/ops_controlplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,17 @@ func getControlPlaneForUID(
}

var id string
for _, listCP := range respList.ListControlPlanesResponse.Data {
if listCP.ID != nil && *listCP.ID != "" {
id = *listCP.ID
for _, entry := range respList.ListControlPlanesResponse.Data {
if entry.ID != nil && *entry.ID != "" {
id = *entry.ID
break
}
}

if id == "" {
return "", fmt.Errorf("control plane %s (matching UID %s) not found", cp.Name, cp.UID)
return "", EntityWithMatchingUIDNotFoundError{
Entity: cp,
}
}

return id, nil
Expand All @@ -72,16 +74,16 @@ func createControlPlane(
req.Labels = WithKubernetesMetadataLabels(cp, req.Labels)

resp, err := sdk.CreateControlPlane(ctx, req)
// TODO: handle already exists

// Can't adopt it as it will cause conflicts between the controller
// that created that entity and already manages it, hm
// that created that entity and already manages it.
// TODO: implement entity adoption https://github.com/Kong/gateway-operator/issues/460
if errWrap := wrapErrIfKonnectOpFailed(err, CreateOp, cp); errWrap != nil {
return errWrap
}

if resp == nil || resp.ControlPlane == nil {
return fmt.Errorf("failed creating ControlPlane: %w", ErrNilResponse)
if resp == nil || resp.ControlPlane == nil || resp.ControlPlane.ID == nil {
return fmt.Errorf("failed creating %s: %w", cp.GetTypeName(), ErrNilResponse)
}

// At this point, the ControlPlane has been created in Konnect.
Expand Down Expand Up @@ -159,10 +161,7 @@ func updateControlPlane(
resp, err := sdk.UpdateControlPlane(ctx, id, req)
var sdkError *sdkkonnecterrs.NotFoundError
if errors.As(err, &sdkError) {
ctrllog.FromContext(ctx).
Info("entity not found in Konnect, trying to recreate",
"type", cp.GetTypeName(), "id", id,
)
logEntityNotFoundRecreating(ctx, cp, id)
if err := createControlPlane(ctx, sdk, sdkGroups, cl, cp); err != nil {
return FailedKonnectOpError[konnectv1alpha1.KonnectGatewayControlPlane]{
Op: UpdateOp,
Expand Down
2 changes: 1 addition & 1 deletion controller/konnect/ops/ops_controlplane_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func TestCreateControlPlane(t *testing.T) {
err := createControlPlane(ctx, sdk, sdkGroups, fakeClient, cp)
if tc.expectedErrContains != "" {
if tc.expectedErrType != nil {
assert.IsType(t, err, tc.expectedErrType)
assert.ErrorAs(t, err, &tc.expectedErrType)
}
assert.ErrorContains(t, err, tc.expectedErrContains)
return
Expand Down
62 changes: 61 additions & 1 deletion controller/konnect/ops/ops_errors.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,67 @@
package ops

import "errors"
import (
"encoding/json"
"errors"
"fmt"

"k8s.io/apimachinery/pkg/types"
)

// ErrNilResponse is an error indicating that a Konnect operation returned an empty response.
// This can sometimes happen regardless of the err being nil.
var ErrNilResponse = errors.New("nil response received")

// EntityWithMatchingUIDNotFoundError is an error indicating that an entity with a matching UID was not found on Konnect.
type EntityWithMatchingUIDNotFoundError struct {
Entity interface {
GetTypeName() string
GetName() string
GetUID() types.UID
}
}

// Error implements the error interface.
func (e EntityWithMatchingUIDNotFoundError) Error() string {
return fmt.Sprintf(
"%s %s (matching UID %s) not found on Konnect",
e.Entity.GetTypeName(), e.Entity.GetName(), e.Entity.GetUID(),
)
}

type sdkErrorBody struct {
Code int `json:"code"`
Message string `json:"message"`
Details []struct {
TypeAt string `json:"@type"`
Type string `json:"type"`
Field string `json:"field"`
Messages []string `json:"messages"`
} `json:"details"`
}

// ParseSDKErrorBody parses the body of an SDK error response.
// Exemplary body:
//
// {
// "code": 3,
// "message": "data constraint error",
// "details": [
// {
// "@type": "type.googleapis.com/kong.admin.model.v1.ErrorDetail",
// "type": "ERROR_TYPE_REFERENCE",
// "field": "name",
// "messages": [
// "name (type: unique) constraint failed"
// ]
// }
// ]
// }
func ParseSDKErrorBody(body string) (sdkErrorBody, error) {
var sdkErr sdkErrorBody
if err := json.Unmarshal([]byte(body), &sdkErr); err != nil {
return sdkErr, err
}

return sdkErr, nil
}
65 changes: 54 additions & 11 deletions controller/konnect/ops/ops_kongservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,54 @@ import (
sdkkonnectcomp "github.com/Kong/sdk-konnect-go/models/components"
sdkkonnectops "github.com/Kong/sdk-konnect-go/models/operations"
sdkkonnecterrs "github.com/Kong/sdk-konnect-go/models/sdkerrors"
"github.com/samber/lo"
"sigs.k8s.io/controller-runtime/pkg/client"
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"

configurationv1alpha1 "github.com/kong/kubernetes-configuration/api/configuration/v1alpha1"
)

// getKongServiceForUID returns the Konnect ID of the KongService
// that matches the UID of the provided KongService.
func getKongServiceForUID(
ctx context.Context,
sdk ServicesSDK,
svc *configurationv1alpha1.KongService,
) (string, error) {
reqList := sdkkonnectops.ListServiceRequest{
// NOTE: only filter on object's UID.
// Other fields like name might have changed in the meantime but that's OK.
// Those will be enforced via subsequent updates.
Tags: lo.ToPtr(UIDLabelForObject(svc)),
ControlPlaneID: svc.GetControlPlaneID(),
}

respList, err := sdk.ListService(ctx, reqList)
if err != nil {
return "", err
}

if respList == nil || respList.Object == nil {
return "", fmt.Errorf("failed listing KongServices: %w", ErrNilResponse)
}

var id string
for _, entry := range respList.Object.Data {
if entry.ID != nil && *entry.ID != "" {
id = *entry.ID
break
}
}

if id == "" {
return "", EntityWithMatchingUIDNotFoundError{
Entity: svc,
}
}

return id, nil
}

func createService(
ctx context.Context,
sdk ServicesSDK,
Expand All @@ -31,16 +73,20 @@ func createService(
kongServiceToSDKServiceInput(svc),
)

// TODO: handle already exists
// Can't adopt it as it will cause conflicts between the controller
// that created that entity and already manages it, hm
// that created that entity and already manages it.
// TODO: implement entity adoption https://github.com/Kong/gateway-operator/issues/460
if errWrap := wrapErrIfKonnectOpFailed(err, CreateOp, svc); errWrap != nil {
SetKonnectEntityProgrammedConditionFalse(svc, "FailedToCreate", errWrap.Error())
return errWrap
}

svc.Status.Konnect.SetKonnectID(*resp.Service.ID)
SetKonnectEntityProgrammedCondition(svc)
if resp == nil || resp.Service == nil || resp.Service.ID == nil {
return fmt.Errorf("failed creating %s: %w", svc.GetTypeName(), ErrNilResponse)
}

// At this point, the ControlPlane has been created in Konnect.
id := *resp.Service.ID
svc.SetKonnectID(id)

return nil
}
Expand All @@ -55,7 +101,7 @@ func updateService(
svc *configurationv1alpha1.KongService,
) error {
if svc.GetControlPlaneID() == "" {
return fmt.Errorf("can't update %T %s without a Konnect ControlPlane ID",
return fmt.Errorf("can't update %T %s without a Konnect ControlPlane ID",
svc, client.ObjectKeyFromObject(svc),
)
}
Expand All @@ -78,8 +124,8 @@ func updateService(
if errors.As(errWrap, &sdkError) {
switch sdkError.StatusCode {
case 404:
err := createService(ctx, sdk, svc)
if err != nil {
logEntityNotFoundRecreating(ctx, svc, id)
if err := createService(ctx, sdk, svc); err != nil {
return FailedKonnectOpError[configurationv1alpha1.KongService]{
Op: UpdateOp,
Err: err,
Expand All @@ -95,12 +141,9 @@ func updateService(
}
}

SetKonnectEntityProgrammedConditionFalse(svc, "FailedToUpdate", errWrap.Error())
return errWrap
}

SetKonnectEntityProgrammedCondition(svc)

return nil
}

Expand Down
Loading

0 comments on commit 3c2e736

Please sign in to comment.