Skip to content

Commit

Permalink
fix(konnect): handle CACertificate creation conflicts (#769)
Browse files Browse the repository at this point in the history
Signed-off-by: Jintao Zhang <[email protected]>
  • Loading branch information
tao12345666333 authored Oct 22, 2024
1 parent d9646df commit 32bbcd9
Show file tree
Hide file tree
Showing 6 changed files with 191 additions and 5 deletions.
2 changes: 2 additions & 0 deletions controller/konnect/ops/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ func Create[
id, err = getKongCredentialACLForUID(ctx, sdk.GetACLCredentialsSDK(), ent)
case *configurationv1alpha1.KongCertificate:
id, err = getKongCertificateForUID(ctx, sdk.GetCertificatesSDK(), ent)
case *configurationv1alpha1.KongCACertificate:
id, err = getKongCACertificateForUID(ctx, sdk.GetCACertificatesSDK(), ent)
// ---------------------------------------------------------------------
// TODO: add other Konnect types
default:
Expand Down
31 changes: 28 additions & 3 deletions controller/konnect/ops/ops_kongcacertificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package ops

import (
"context"
"fmt"

sdkkonnectcomp "github.com/Kong/sdk-konnect-go/models/components"
sdkkonnectops "github.com/Kong/sdk-konnect-go/models/operations"
"github.com/samber/lo"

sdkops "github.com/kong/gateway-operator/controller/konnect/ops/sdk"

Expand Down Expand Up @@ -32,12 +34,15 @@ func createCACertificate(
// Can't adopt it as it will cause conflicts between the controller
// that created that entity and already manages it, hm
if errWrap := wrapErrIfKonnectOpFailed(err, CreateOp, cert); errWrap != nil {
SetKonnectEntityProgrammedConditionFalse(cert, "FailedToCreate", errWrap.Error())
return errWrap
}

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

// At this point, the CACertificate has been created successfully.
cert.SetKonnectID(*resp.CACertificate.ID)

return nil
}
Expand Down Expand Up @@ -100,3 +105,23 @@ func kongCACertificateToCACertificateInput(cert *configurationv1alpha1.KongCACer
Tags: GenerateTagsForObject(cert, cert.Spec.Tags...),
}
}

func getKongCACertificateForUID(
ctx context.Context,
sdk sdkops.CACertificatesSDK,
cert *configurationv1alpha1.KongCACertificate,
) (string, error) {
resp, err := sdk.ListCaCertificate(ctx, sdkkonnectops.ListCaCertificateRequest{
ControlPlaneID: cert.GetControlPlaneID(),
Tags: lo.ToPtr(UIDLabelForObject(cert)),
})
if err != nil {
return "", fmt.Errorf("failed to list %s: %w", cert.GetTypeName(), err)
}

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

return getMatchingEntryFromListResponseData(sliceToEntityWithIDSlice(resp.Object.Data), cert)
}
1 change: 1 addition & 0 deletions controller/konnect/ops/sdk/kongcacertificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type CACertificatesSDK interface {
CreateCaCertificate(ctx context.Context, controlPlaneID string, caCertificate sdkkonnectcomp.CACertificateInput, opts ...sdkkonnectops.Option) (*sdkkonnectops.CreateCaCertificateResponse, error)
UpsertCaCertificate(ctx context.Context, request sdkkonnectops.UpsertCaCertificateRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.UpsertCaCertificateResponse, error)
DeleteCaCertificate(ctx context.Context, controlPlaneID string, caCertificateID string, opts ...sdkkonnectops.Option) (*sdkkonnectops.DeleteCaCertificateResponse, error)
ListCaCertificate(ctx context.Context, request sdkkonnectops.ListCaCertificateRequest, opts ...sdkkonnectops.Option) (*sdkkonnectops.ListCaCertificateResponse, error)
}

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

84 changes: 82 additions & 2 deletions test/envtest/konnect_entities_kongcacertificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package envtest

import (
"context"
"slices"
"testing"

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"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand All @@ -28,6 +30,10 @@ func TestKongCACertificate(t *testing.T) {
ctx, cancel := Context(t, context.Background())
defer cancel()
cfg, ns := Setup(t, ctx, scheme.Get())
const (
tagName = "tag1"
conflictingTagName = "xconflictx"
)

t.Log("Setting up the manager with reconcilers")
mgr, logs := NewManager(t, ctx, cfg, scheme.Get())
Expand All @@ -53,7 +59,8 @@ func TestKongCACertificate(t *testing.T) {
t.Log("Setting up SDK expectations on KongCACertificate creation")
sdk.CACertificatesSDK.EXPECT().CreateCaCertificate(mock.Anything, cp.GetKonnectStatus().GetKonnectID(),
mock.MatchedBy(func(input sdkkonnectcomp.CACertificateInput) bool {
return input.Cert == deploy.TestValidCACertPEM
return input.Cert == deploy.TestValidCACertPEM &&
slices.Contains(input.Tags, tagName)
}),
).Return(&sdkkonnectops.CreateCaCertificateResponse{
CACertificate: &sdkkonnectcomp.CACertificate{
Expand All @@ -65,7 +72,12 @@ func TestKongCACertificate(t *testing.T) {
w := setupWatch[configurationv1alpha1.KongCACertificateList](t, ctx, cl, client.InNamespace(ns.Name))

t.Log("Creating KongCACertificate")
createdCert := deploy.KongCACertificateAttachedToCP(t, ctx, clientNamespaced, cp)
createdCert := deploy.KongCACertificateAttachedToCP(t, ctx, clientNamespaced, cp,
func(obj client.Object) {
cert := obj.(*configurationv1alpha1.KongCACertificate)
cert.Spec.Tags = []string{tagName}
},
)

t.Log("Waiting for KongCACertificate to be programmed")
watchFor(t, ctx, w, watch.Modified, func(c *configurationv1alpha1.KongCACertificate) bool {
Expand Down Expand Up @@ -110,4 +122,72 @@ func TestKongCACertificate(t *testing.T) {
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assert.True(c, factory.SDK.CACertificatesSDK.AssertExpectations(t))
}, waitTime, tickTime)

t.Run("should handle conflict in creation correctly", func(t *testing.T) {
const (
certID = "id-conflict"
)
t.Log("Setup mock SDK for creating CA certificate and listing CA certificates by UID")
cpID := cp.GetKonnectStatus().GetKonnectID()
sdk.CACertificatesSDK.EXPECT().
CreateCaCertificate(mock.Anything, cpID,
mock.MatchedBy(func(input sdkkonnectcomp.CACertificateInput) bool {
return input.Cert == deploy.TestValidCACertPEM &&
slices.Contains(input.Tags, conflictingTagName)
}),
).
Return(nil,
&sdkkonnecterrs.SDKError{
StatusCode: 400,
Body: ErrBodyDataConstraintError,
},
)

sdk.CACertificatesSDK.EXPECT().
ListCaCertificate(
mock.Anything,
mock.MatchedBy(func(req sdkkonnectops.ListCaCertificateRequest) bool {
return req.ControlPlaneID == cpID
}),
).
Return(
&sdkkonnectops.ListCaCertificateResponse{
Object: &sdkkonnectops.ListCaCertificateResponseBody{
Data: []sdkkonnectcomp.CACertificate{
{
ID: lo.ToPtr(certID),
},
},
},
}, nil,
)

t.Log("Creating a KongCACertificate")
createdCert := deploy.KongCACertificateAttachedToCP(t, ctx, clientNamespaced, cp,
func(obj client.Object) {
cert := obj.(*configurationv1alpha1.KongCACertificate)
cert.Spec.Tags = []string{conflictingTagName}
},
)

t.Log("Watching for KongCACertificates to verify the created KongCACertificate gets programmed")
watchFor(t, ctx, w, watch.Modified, func(c *configurationv1alpha1.KongCACertificate) bool {
if c.GetName() != createdCert.GetName() {
return false
}
if !slices.Equal(c.Spec.Tags, createdCert.Spec.Tags) {
return false
}

return c.GetKonnectID() == certID && lo.ContainsBy(c.Status.Conditions, func(condition metav1.Condition) bool {
return condition.Type == konnectv1alpha1.KonnectEntityProgrammedConditionType &&
condition.Status == metav1.ConditionTrue
})
}, "KongCACertificate should be programmed and have ID in status after handling conflict")

t.Log("Ensuring that the SDK's create and list methods are called")
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assert.True(c, sdk.CACertificatesSDK.AssertExpectations(t))
}, waitTime, tickTime)
})
}
4 changes: 4 additions & 0 deletions test/helpers/deploy/deploy_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ func KongCACertificateAttachedToCP(
ctx context.Context,
cl client.Client,
cp *konnectv1alpha1.KonnectGatewayControlPlane,
opts ...objOption,
) *configurationv1alpha1.KongCACertificate {
t.Helper()

Expand All @@ -503,6 +504,9 @@ func KongCACertificateAttachedToCP(
},
},
}
for _, opt := range opts {
opt(cert)
}
require.NoError(t, cl.Create(ctx, cert))
logObjectCreate(t, cert)

Expand Down

0 comments on commit 32bbcd9

Please sign in to comment.