Skip to content

Commit

Permalink
fix(konnect): handle KongRoute conflict on creation (#752)
Browse files Browse the repository at this point in the history
* fix(konnect): handle KongRoute conflict on creation

* refactor: change Konnect conflict error handling

* fix: make type conversion safe with generatic in sliceToEntityWithIDSlice

* refactor: make error handling consistent
  • Loading branch information
pmalek authored Oct 18, 2024
1 parent a41fdc1 commit dcfe0ce
Show file tree
Hide file tree
Showing 13 changed files with 473 additions and 168 deletions.
1 change: 1 addition & 0 deletions controller/konnect/ops/kongroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type RoutesSDK interface {
CreateRoute(ctx context.Context, controlPlaneID string, route sdkkonnectcomp.RouteInput, opts ...sdkkonnectops.Option) (*sdkkonnectops.CreateRouteResponse, error)
UpsertRoute(ctx context.Context, req sdkkonnectops.UpsertRouteRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.UpsertRouteResponse, error)
DeleteRoute(ctx context.Context, controlPlaneID, routeID string, opts ...sdkkonnectops.Option) (*sdkkonnectops.DeleteRouteResponse, error)
ListRoute(ctx context.Context, request sdkkonnectops.ListRouteRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.ListRouteResponse, error)
}
74 changes: 74 additions & 0 deletions controller/konnect/ops/kongroute_mock.go

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

73 changes: 55 additions & 18 deletions controller/konnect/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"fmt"
"time"

sdkkonnecterrs "github.com/Kong/sdk-konnect-go/models/sdkerrors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -68,6 +67,10 @@ func Create[

case *configurationv1.KongConsumer:
err = createConsumer(ctx, sdk.GetConsumersSDK(), sdk.GetConsumerGroupsSDK(), 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 *configurationv1beta1.KongConsumerGroup:
err = createConsumerGroup(ctx, sdk.GetConsumerGroupsSDK(), ent)
case *configurationv1alpha1.KongPluginBinding:
Expand Down Expand Up @@ -106,24 +109,9 @@ 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)
}

var errRelationsFailed KonnectEntityCreatedButRelationsFailedError
switch {
case errors.As(err, &errConflict) ||
// Service API returns 400 for conflicts
errSdkBody.Code == 3 && errSdkBody.Message == "data constraint error":
case ErrorIsCreateConflict(err):
// 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
Expand All @@ -132,6 +120,8 @@ func Create[
id, err = getControlPlaneForUID(ctx, sdk.GetControlPlaneSDK(), ent)
case *configurationv1alpha1.KongService:
id, err = getKongServiceForUID(ctx, sdk.GetServicesSDK(), ent)
case *configurationv1alpha1.KongRoute:
id, err = getKongRouteForUID(ctx, sdk.GetRoutesSDK(), ent)
case *configurationv1alpha1.KongSNI:
id, err = getSNIForUID(ctx, sdk.GetSNIsSDK(), ent)
case *configurationv1.KongConsumer:
Expand Down Expand Up @@ -417,3 +407,50 @@ func logEntityNotFoundRecreating[
"id", id,
)
}

type entityWithID interface {
GetID() *string
}

// sliceToEntityWithIDSlice converts a slice of entities to a slice of entityWithID.
func sliceToEntityWithIDSlice[
T any,
TPtr interface {
*T
GetID() *string
},
](
slice []T,
) []entityWithID {
result := make([]entityWithID, 0, len(slice))
for _, item := range slice {
result = append(result, TPtr(&item))
}
return result
}

// getMatchingEntryFromListResponseData returns the ID of the first entry in the list response data.
// It returns an error if no entry with a non-empty ID was found.
// It is used in conjunction with the list operation to get the ID of the entity that matches the UID
// hence no filtering is done here because it is assumed that the provided list response data is already filtered.
func getMatchingEntryFromListResponseData(
data []entityWithID,
entity entity,
) (string, error) {
var id string
for _, entry := range data {
entryID := entry.GetID()
if entryID != nil && *entryID != "" {
id = *entryID
break
}
}

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

return id, nil
}
66 changes: 26 additions & 40 deletions controller/konnect/ops/ops_controlplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,46 +19,6 @@ import (
konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1"
)

// getControlPlaneForUID returns the Konnect ID of the Konnect ControlPlane
// that matches the UID of the provided KonnectGatewayControlPlane.
func getControlPlaneForUID(
ctx context.Context,
sdk ControlPlaneSDK,
cp *konnectv1alpha1.KonnectGatewayControlPlane,
) (string, error) {
reqList := sdkkonnectops.ListControlPlanesRequest{
// 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.
Labels: lo.ToPtr(UIDLabelForObject(cp)),
}

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

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

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

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

return id, nil
}

// createControlPlane creates the ControlPlane as specified in provided ControlPlane's
// spec. Besides creating the ControlPlane, it also creates the group membership if the
// ControlPlane is a group. If the group membership creation fails, KonnectEntityCreatedButRelationsFailedError
Expand Down Expand Up @@ -254,3 +214,29 @@ type membersByID []sdkkonnectcomp.Members
func (m membersByID) Len() int { return len(m) }
func (m membersByID) Less(i, j int) bool { return *m[i].ID < *m[j].ID }
func (m membersByID) Swap(i, j int) { m[i], m[j] = m[j], m[i] }

// getControlPlaneForUID returns the Konnect ID of the Konnect ControlPlane
// that matches the UID of the provided KonnectGatewayControlPlane.
func getControlPlaneForUID(
ctx context.Context,
sdk ControlPlaneSDK,
cp *konnectv1alpha1.KonnectGatewayControlPlane,
) (string, error) {
reqList := sdkkonnectops.ListControlPlanesRequest{
// 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.
Labels: lo.ToPtr(UIDLabelForObject(cp)),
}

resp, err := sdk.ListControlPlanes(ctx, reqList)
if err != nil {
return "", fmt.Errorf("failed listing %s: %w", cp.GetTypeName(), err)
}

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

return getMatchingEntryFromListResponseData(sliceToEntityWithIDSlice(resp.ListControlPlanesResponse.Data), cp)
}
43 changes: 37 additions & 6 deletions controller/konnect/ops/ops_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,22 @@ import (
"errors"
"fmt"

"k8s.io/apimachinery/pkg/types"
sdkkonnecterrs "github.com/Kong/sdk-konnect-go/models/sdkerrors"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// 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")

type entity interface {
client.Object
GetTypeName() string
}

// 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
}
Entity entity
}

// Error implements the error interface.
Expand Down Expand Up @@ -65,3 +67,32 @@ func ParseSDKErrorBody(body string) (sdkErrorBody, error) {

return sdkErr, nil
}

// ErrorIsCreateConflict returns true if the provided error is a Konnect conflict error.
//
// NOTE: Konnect APIs specific for Konnect only APIs like Gateway Control Planes
// return 409 conflict for already existing entities and return ConflictError.
// APIs that are shared with Kong Admin API return 400 for conflicts and return SDKError.
func ErrorIsCreateConflict(err error) bool {
var (
errConflict *sdkkonnecterrs.ConflictError
sdkError *sdkkonnecterrs.SDKError
)
if errors.As(err, &errConflict) {
return true
}
if errors.As(err, &sdkError) {
return SDKErrorIsConflict(sdkError)
}
return false
}

// SDKErrorIsConflict returns true if the provided SDKError indicates a conflict.
func SDKErrorIsConflict(sdkError *sdkkonnecterrs.SDKError) bool {
sdkErrorBody, err := ParseSDKErrorBody(sdkError.Body)
if err != nil {
return false
}

return sdkErrorBody.Code == 3 && sdkErrorBody.Message == "data constraint error"
}
19 changes: 6 additions & 13 deletions controller/konnect/ops/ops_kongconsumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,24 +361,17 @@ func getConsumerForUID(
Tags: lo.ToPtr(UIDLabelForObject(consumer)),
}

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

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

for _, entry := range respList.Object.Data {
if entry.ID != nil && *entry.ID != "" {
// return the ID if we found a non-empty one with the given k8s uid
return *entry.ID, nil
}
if err != nil {
return "", fmt.Errorf("failed listing %s: %w", consumer.GetTypeName(), err)
}
// return UIDNotFound error if no such entry found
return "", EntityWithMatchingUIDNotFoundError{
Entity: consumer,
if resp == nil || resp.Object == nil {
return "", fmt.Errorf("failed listing %s: %w", consumer.GetTypeName(), ErrNilResponse)
}

return getMatchingEntryFromListResponseData(sliceToEntityWithIDSlice(resp.Object.Data), consumer)
}
Loading

0 comments on commit dcfe0ce

Please sign in to comment.