Skip to content

Commit

Permalink
fix(konnect): fix handling conflicts (#1176)
Browse files Browse the repository at this point in the history
* fix(konnect): fix handling conflicts

* chore: refactor SDKErrorIsConflict and add exemplary body

* fix: handle already existing Certificate when calling create
  • Loading branch information
pmalek authored Feb 19, 2025
1 parent a8590e8 commit 87923c4
Show file tree
Hide file tree
Showing 6 changed files with 299 additions and 105 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@
[#1115](https://github.com/Kong/gateway-operator/pull/1115)
- Fix setting `DataPlane`'s readiness probe using `GatewayConfiguration`.
[#1118](https://github.com/Kong/gateway-operator/pull/1118)
- Fix handling Konnect API conflicts.
[#1176](https://github.com/Kong/gateway-operator/pull/1176)

## [v1.4.2]

Expand Down
1 change: 1 addition & 0 deletions controller/konnect/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func Create[
entityType = e.GetTypeName()
statusCode int
)

switch ent := any(e).(type) {
case *konnectv1alpha1.KonnectGatewayControlPlane:
err = createControlPlane(ctx, sdk.GetControlPlaneSDK(), sdk.GetControlPlaneGroupSDK(), cl, ent)
Expand Down
42 changes: 36 additions & 6 deletions controller/konnect/ops/ops_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"slices"
"strings"

sdkkonnecterrs "github.com/Kong/sdk-konnect-go/models/sdkerrors"
"github.com/go-logr/logr"
Expand Down Expand Up @@ -94,7 +95,6 @@ func ParseSDKErrorBody(body string) (sdkErrorBody, error) {
}

const (
dataConstraintMesasge = "data constraint error"
validationErrorMessage = "validation error"
apiErrorOccurredMessage = "API error occurred"
)
Expand Down Expand Up @@ -196,20 +196,50 @@ func ErrorIsCreateConflict(err error) bool {
}

// SDKErrorIsConflict returns true if the provided SDKError indicates a conflict.
// We currently handle two codes (as mapped in
// https://grpc.io/docs/guides/status-codes/#the-full-list-of-status-codes)
// from SDK error:
// - 3: INVALID_ARGUMENT
// - 6: ALREADY_EXISTS
//
// Exemplary body:
//
// {
// "code": 3,
// "message": "name (type: unique) constraint failed",
// "details": [
// {
// "@type": "type.googleapis.com/kong.admin.model.v1.ErrorDetail",
// "type": "ERROR_TYPE_REFERENCE",
// "field": "name",
// "messages": [
// "name (type: unique) constraint failed"
// ]
// }
// ]
// }
func SDKErrorIsConflict(sdkError *sdkkonnecterrs.SDKError) bool {
sdkErrorBody, err := ParseSDKErrorBody(sdkError.Body)
if err != nil {
return false
}

if sdkErrorBody.Message != dataConstraintMesasge {
return false
}

switch sdkErrorBody.Code {
case 3, 6:
case 3: // INVALID_ARGUMENT
const (
dataConstraintMesasge = "data constraint error"
typeUniqueConstraintFailed = "(type: unique) constraint failed"
)

if sdkErrorBody.Message == dataConstraintMesasge ||
strings.Contains(sdkErrorBody.Message, typeUniqueConstraintFailed) {
return true
}

case 6: // ALREADY_EXISTS
return true
}

return false
}

Expand Down
100 changes: 100 additions & 0 deletions controller/konnect/ops/ops_errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,28 @@ func TestErrorIsCreateConflict(t *testing.T) {
},
want: false,
},
{
name: "error is SDKError with code 6",
err: &sdkkonnecterrs.SDKError{
Body: `{
"code": 6,
"message": "already exists",
"details": []
}`,
},
want: true,
},
{
name: "error is SDKError with code 7",
err: &sdkkonnecterrs.SDKError{
Body: `{
"code": 7,
"message": "other error",
"details": []
}`,
},
want: false,
},
{
name: "error is not ConflictError or SDKError",
err: errors.New("some other error"),
Expand All @@ -140,3 +162,81 @@ func TestErrorIsCreateConflict(t *testing.T) {
})
}
}

func TestSDKErrorIsConflict(t *testing.T) {
tests := []struct {
name string
err *sdkkonnecterrs.SDKError
want bool
}{
{
name: "SDKError with data constraint error message and code 3",
err: &sdkkonnecterrs.SDKError{
Body: `{
"code": 3,
"message": "data constraint error",
"details": []
}`,
},
want: true,
},
{
name: "SDKError with data constraint error message and code 6",
err: &sdkkonnecterrs.SDKError{
Body: `{
"code": 6,
"message": "data constraint error",
"details": []
}`,
},
want: true,
},
{
name: "SDKError with unique constraint failed message",
err: &sdkkonnecterrs.SDKError{
Body: `{
"code": 3,
"message": "name (type: unique) constraint failed",
"details": []
}`,
},
want: true,
},
{
name: "SDKError with non-conflict message",
err: &sdkkonnecterrs.SDKError{
Body: `{
"code": 3,
"message": "some other error",
"details": []
}`,
},
want: false,
},
{
name: "SDKError with conflict message but different code",
err: &sdkkonnecterrs.SDKError{
Body: `{
"code": 4,
"message": "data constraint error",
"details": []
}`,
},
want: false,
},
{
name: "SDKError with invalid JSON body",
err: &sdkkonnecterrs.SDKError{
Body: `invalid json`,
},
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := SDKErrorIsConflict(tt.err)
require.Equal(t, tt.want, got)
})
}
}
28 changes: 28 additions & 0 deletions controller/konnect/ops/ops_kongcertificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ops
import (
"context"
"fmt"
"strings"

sdkkonnectcomp "github.com/Kong/sdk-konnect-go/models/components"
sdkkonnectops "github.com/Kong/sdk-konnect-go/models/operations"
Expand All @@ -25,6 +26,33 @@ func createCertificate(
return CantPerformOperationWithoutControlPlaneIDError{Entity: cert, Op: CreateOp}
}

// NOTE: This is a workaround for the fact that the Konnect SDK does not
// return a conflict error when creating a Certificate as there are no criteria
// that would prevent the creation of a Certificate with the same spec fields.
// This can be expanded to other entities by changing the order of get -> list
// to list -> get in ops.go's Create() function.

// Check if the Certificate already exists in Konnect: it is possible that
// the Certificate was already created and cache the controller is using
// is outdated, so we can't rely on the status.konnect.id.
// If it does, set the KonnectID in the KongCertificate status and return.

respList, err := sdk.ListCertificate(ctx, sdkkonnectops.ListCertificateRequest{
ControlPlaneID: cpID,
Tags: lo.ToPtr(strings.Join(GenerateTagsForObject(cert, cert.Spec.Tags...), ",")),
})
if err != nil {
return fmt.Errorf("failed to list: %w", err)
}
if respList.Object != nil && len(respList.Object.Data) > 0 {
certList := respList.Object.Data[0]
if certList.ID == nil {
return fmt.Errorf("failed listing: found a cert without ID")
}
cert.SetKonnectID(*certList.ID)
return nil
}

resp, err := sdk.CreateCertificate(ctx,
cpID,
kongCertificateToCertificateInput(cert),
Expand Down
Loading

0 comments on commit 87923c4

Please sign in to comment.