Skip to content

Commit

Permalink
fix(konnect): do not set owner relationship between ControlPlane and …
Browse files Browse the repository at this point in the history
…its config entities
  • Loading branch information
pmalek committed Feb 5, 2025
1 parent d1e88da commit 848ac5a
Show file tree
Hide file tree
Showing 25 changed files with 827 additions and 177 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@
have any effect.
They will be removed in next major release.
[#1100](https://github.com/Kong/gateway-operator/pull/1100)
- Konnect entities that are attached to a Konnect CP through a `ControlPlaneRef`
do not get an owner relationship set to the `ControlPlane` anymore hence
they are not deleted when the `ControlPlane` is deleted.
[#1099](https://github.com/Kong/gateway-operator/pull/1099)

[kubebuilder_3907]: https://github.com/kubernetes-sigs/kubebuilder/discussions/3907

Expand Down
25 changes: 0 additions & 25 deletions controller/konnect/reconciler_controlplaneref.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
ctrllog "sigs.k8s.io/controller-runtime/pkg/log"

"github.com/kong/gateway-operator/controller/konnect/constraints"
Expand Down Expand Up @@ -152,30 +151,6 @@ func handleControlPlaneRef[T constraints.SupportedKonnectEntityType, TEnt constr
return ctrl.Result{Requeue: true}, nil
}

var (
old = ent.DeepCopyObject().(TEnt)

// A cluster scoped object cannot set a namespaced object as its owner, and also we cannot set cross namespaced owner reference.
// So we skip setting owner reference for cluster scoped resources (KongVault).
// TODO: handle cross namespace refs
isNamespaceScoped = ent.GetNamespace() != ""

// If an entity has another owner, we should not set the owner reference as that would prevent the entity from being deleted.
hasNoOwners = len(ent.GetOwnerReferences()) == 0
)
if isNamespaceScoped && hasNoOwners {
if err := controllerutil.SetOwnerReference(cp, ent, cl.Scheme(), controllerutil.WithBlockOwnerDeletion(true)); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to set owner reference: %w", err)
}
}

if err := cl.Patch(ctx, ent, client.MergeFrom(old)); err != nil {
if k8serrors.IsConflict(err) {
return ctrl.Result{Requeue: true}, nil
}
return ctrl.Result{}, fmt.Errorf("failed to update status: %w", err)
}

if resource, ok := any(ent).(EntityWithControlPlaneRef); ok {
old := ent.DeepCopyObject().(TEnt)
resource.SetControlPlaneID(cp.Status.ID)
Expand Down
5 changes: 2 additions & 3 deletions controller/konnect/reconciler_controlplaneref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,8 @@ func TestHandleControlPlaneRef(t *testing.T) {
}), "service should get control plane ID"
},
func(svc *configurationv1alpha1.KongService) (bool, string) {
return lo.ContainsBy(svc.OwnerReferences, func(o metav1.OwnerReference) bool {
return o.Kind == "KonnectGatewayControlPlane" && o.Name == "cp-ok"
}), "service should have owner reference set to CP"
return len(svc.OwnerReferences) == 0,
"service should have 0 owner references"
},
func(svc *configurationv1alpha1.KongService) (bool, string) {
return lo.ContainsBy(svc.Status.Conditions, func(c metav1.Condition) bool {
Expand Down
38 changes: 35 additions & 3 deletions controller/konnect/reconciler_generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,10 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
// If a type has a ControlPlane ref, handle it.
res, err := handleControlPlaneRef(ctx, r.Client, ent)
if err != nil || !res.IsZero() {
// If the referenced ControlPlane is not found and the object is deleted,
// remove the finalizer and update the status.
// If the referenced ControlPlane is not found, remove the finalizer and update the status.
// There's no need to remove the entity on Konnect because the ControlPlane
// does not exist anymore.
if !ent.GetDeletionTimestamp().IsZero() && errors.As(err, &ReferencedControlPlaneDoesNotExistError{}) {
if errors.As(err, &ReferencedControlPlaneDoesNotExistError{}) {
if controllerutil.RemoveFinalizer(ent, KonnectCleanupFinalizer) {
if err := r.Client.Update(ctx, ent); err != nil {
if k8serrors.IsConflict(err) {
Expand All @@ -174,6 +173,13 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
}
}
}

if res, err := setProgrammedStatusConditionBasedOnOtherConditions(ctx, r.Client, ent); err != nil {
return res, err
} else if !res.IsZero() {
return res, nil
}

// Status update will requeue the entity.
return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -1009,3 +1015,29 @@ func handleKongConsumerRef[T constraints.SupportedKonnectEntityType, TEnt constr

return ctrl.Result{}, nil
}

func setProgrammedStatusConditionBasedOnOtherConditions[
T interface {
client.Object
k8sutils.ConditionsAware
},
](
ctx context.Context,
cl client.Client,
ent T,
) (ctrl.Result, error) {
if k8sutils.AreAllConditionsHaveTrueStatus(ent) {
return ctrl.Result{}, nil
}

if res, errStatus := patch.StatusWithCondition(
ctx, cl, ent,
konnectv1alpha1.KonnectEntityProgrammedConditionType,
metav1.ConditionFalse,
konnectv1alpha1.KonnectEntityProgrammedReasonExistsConditionWithStatusFalse,
"Some conditions have status set to False",
); errStatus != nil || !res.IsZero() {
return res, errStatus
}
return ctrl.Result{}, nil
}
2 changes: 1 addition & 1 deletion docs/api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ _Appears in:_
#### KongServiceSpec


KongServiceSpec defines specification of a Kong Route.
KongServiceSpec defines specification of a Kong Service.



Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ require (
github.com/google/go-containerregistry v0.20.3
github.com/google/uuid v1.6.0
github.com/gruntwork-io/terratest v0.48.2
github.com/kong/kubernetes-configuration v1.1.0
github.com/kong/kubernetes-configuration v1.1.1-0.20250127162809-96d322b98f26
github.com/kong/kubernetes-telemetry v0.1.8
github.com/kong/kubernetes-testing-framework v0.47.2
github.com/kong/semver/v4 v4.0.1
Expand Down Expand Up @@ -223,7 +223,7 @@ require (
k8s.io/apiserver v0.32.1 // indirect
k8s.io/component-helpers v0.0.0 // indirect
k8s.io/controller-manager v0.0.0 // indirect
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.0 // indirect
)

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,8 @@ github.com/klauspost/compress v1.17.11 h1:In6xLpyWOi1+C7tXUUWv2ot1QvBjxevKAaI6IX
github.com/klauspost/compress v1.17.11/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0=
github.com/kong/go-kong v0.63.0 h1:8ECLgkgDqON61qCMq/M0gTwZKYxg55Oy692dRDOOBiU=
github.com/kong/go-kong v0.63.0/go.mod h1:ma9GWnhkxtrXZlLFfED955HjVzmUojYEHet3lm+PDik=
github.com/kong/kubernetes-configuration v1.1.0 h1:zCj7QlOiZgwQ2wSNlVNR0K6Z+plXTalfihtDzLXQWzs=
github.com/kong/kubernetes-configuration v1.1.0/go.mod h1:Z9Yo8DyBe/4zw/cgoSYafqHzuviKINVSq1b8D60F0u8=
github.com/kong/kubernetes-configuration v1.1.1-0.20250127162809-96d322b98f26 h1:PpedMiMqEEOigPJnRkhkxf/dwqESsYNCbQbrCiGjPzY=
github.com/kong/kubernetes-configuration v1.1.1-0.20250127162809-96d322b98f26/go.mod h1:B2OJE9gfwSTzr52YNZVsMuI7Ie8wxEVukJCWgc7R1Ao=
github.com/kong/kubernetes-telemetry v0.1.8 h1:nbtUmXW9xkzRO7dgvrgVrJZiRksATk4XHrqX+78g/5k=
github.com/kong/kubernetes-telemetry v0.1.8/go.mod h1:ZEQY/4DddKoe5XA7UTOIbdI/4d6ZRcrzh2ezRxnuyl0=
github.com/kong/kubernetes-testing-framework v0.47.2 h1:+2Z9anTpbV/hwNeN+NFQz53BMU+g3QJydkweBp3tULo=
Expand Down
9 changes: 6 additions & 3 deletions pkg/utils/kubernetes/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func SetReadyWithGeneration(resource ConditionsAndGenerationAware, generation in
ObservedGeneration: generation,
}

if areAllConditionsHaveTrueStatus(resource) {
if AreAllConditionsHaveTrueStatus(resource) {
ready.Status = metav1.ConditionTrue
ready.Reason = string(consts.ResourceReadyReason)
} else {
Expand All @@ -138,7 +138,7 @@ func SetProgrammed(resource ConditionsAndGenerationAware) {
ObservedGeneration: resource.GetGeneration(),
}

if areAllConditionsHaveTrueStatus(resource) {
if AreAllConditionsHaveTrueStatus(resource) {
programmed.Status = metav1.ConditionTrue
programmed.Reason = string(gatewayv1.GatewayReasonProgrammed)
} else {
Expand Down Expand Up @@ -195,7 +195,10 @@ func SetAcceptedConditionOnGateway(resource ConditionsAndListenerConditionsAndGe
}
}

func areAllConditionsHaveTrueStatus(resource ConditionsAware) bool {
// AreAllConditionsHaveTrueStatus checks if all the conditions on the resource are in the True state.
// It skips the Programmed condition as that particular condition will be set based on
// the return value of this function.
func AreAllConditionsHaveTrueStatus(resource ConditionsAware) bool {
for _, condition := range resource.GetConditions() {
if condition.Type == string(gatewayv1.GatewayConditionProgrammed) {
continue
Expand Down
53 changes: 53 additions & 0 deletions test/envtest/condition_asserts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package envtest

import (
"sigs.k8s.io/controller-runtime/pkg/client"

k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"

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

func conditionsAreSetWhenReferencedControlPlaneIsMissing[
T interface {
client.Object
k8sutils.ConditionsAware
GetControlPlaneRef() *configurationv1alpha1.ControlPlaneRef
},
](objToMatch T) func(obj T) bool {
return func(obj T) bool {
if obj.GetName() != objToMatch.GetName() {
return false
}
if obj.GetControlPlaneRef().Type != configurationv1alpha1.ControlPlaneRefKonnectID {
return false
}
condCpRef, okCPRef := k8sutils.GetCondition(konnectv1alpha1.ControlPlaneRefValidConditionType, obj)
condProgrammed, okProgrammed := k8sutils.GetCondition(konnectv1alpha1.KonnectEntityProgrammedConditionType, obj)
return okCPRef && okProgrammed &&
condCpRef.Status == "False" &&
condProgrammed.Status == "False" &&
condCpRef.Reason == konnectv1alpha1.ControlPlaneRefReasonInvalid &&
condProgrammed.Reason == konnectv1alpha1.KonnectEntityProgrammedReasonExistsConditionWithStatusFalse
}
}

func conditionProgrammedIsSetToTrue[
T interface {
client.Object
k8sutils.ConditionsAware
GetControlPlaneRef() *configurationv1alpha1.ControlPlaneRef
GetKonnectID() string
},
](objToMatch T, id string) func(T) bool {
return func(obj T) bool {
if obj.GetName() != objToMatch.GetName() {
return false
}
if obj.GetControlPlaneRef().Type != configurationv1alpha1.ControlPlaneRefKonnectID {
return false
}
return obj.GetKonnectID() == id && k8sutils.IsProgrammed(obj)
}
}
22 changes: 15 additions & 7 deletions test/envtest/kongpluginbinding_managed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ func TestKongPluginBindingManaged(t *testing.T) {
wKongPluginBinding := setupWatch[configurationv1alpha1.KongPluginBindingList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
wKongPlugin := setupWatch[configurationv1.KongPluginList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))

kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp,
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
deploy.WithAnnotation(metadata.AnnotationKeyPlugins, rateLimitingkongPlugin.Name),
)
t.Cleanup(func() {
Expand Down Expand Up @@ -211,7 +212,9 @@ func TestKongPluginBindingManaged(t *testing.T) {

wKongPluginBinding := setupWatch[configurationv1alpha1.KongPluginBindingList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
wKongPlugin := setupWatch[configurationv1.KongPluginList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp)
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)
t.Cleanup(func() {
require.NoError(t, clientNamespaced.Delete(ctx, kongService))
})
Expand Down Expand Up @@ -317,7 +320,8 @@ func TestKongPluginBindingManaged(t *testing.T) {

wKongPluginBinding := setupWatch[configurationv1alpha1.KongPluginBindingList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
wKongPlugin := setupWatch[configurationv1.KongPluginList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp,
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
deploy.WithAnnotation(metadata.AnnotationKeyPlugins, rateLimitingkongPlugin.Name),
)
t.Cleanup(func() {
Expand Down Expand Up @@ -445,7 +449,8 @@ func TestKongPluginBindingManaged(t *testing.T) {

wKongPluginBinding := setupWatch[configurationv1alpha1.KongPluginBindingList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
wKongPlugin := setupWatch[configurationv1.KongPluginList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp,
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
deploy.WithAnnotation(metadata.AnnotationKeyPlugins, rateLimitingkongPlugin.Name),
)
t.Cleanup(func() {
Expand All @@ -459,7 +464,8 @@ func TestKongPluginBindingManaged(t *testing.T) {
require.NoError(t, clientNamespaced.Delete(ctx, kongRoute))
})
updateKongRouteStatusWithProgrammed(t, ctx, clientNamespaced, kongRoute, routeID, cp.GetKonnectStatus().GetKonnectID(), serviceID)
kongConsumer := deploy.KongConsumerAttachedToCP(t, ctx, clientNamespaced, "username-1", cp,
kongConsumer := deploy.KongConsumerAttachedToCP(t, ctx, clientNamespaced, "username-1",
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
deploy.WithAnnotation(metadata.AnnotationKeyPlugins, rateLimitingkongPlugin.Name),
)
t.Cleanup(func() {
Expand Down Expand Up @@ -644,7 +650,8 @@ func TestKongPluginBindingManaged(t *testing.T) {

wKongPluginBinding := setupWatch[configurationv1alpha1.KongPluginBindingList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
wKongPlugin := setupWatch[configurationv1.KongPluginList](t, ctx, clientWithWatch, client.InNamespace(ns.Name))
kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp,
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
deploy.WithAnnotation(metadata.AnnotationKeyPlugins, rateLimitingkongPlugin.Name),
)
t.Cleanup(func() {
Expand All @@ -658,7 +665,8 @@ func TestKongPluginBindingManaged(t *testing.T) {
require.NoError(t, clientNamespaced.Delete(ctx, kongRoute))
})
updateKongRouteStatusWithProgrammed(t, ctx, clientNamespaced, kongRoute, routeID, cp.GetKonnectStatus().GetKonnectID(), serviceID)
kongConsumerGroup := deploy.KongConsumerGroupAttachedToCP(t, ctx, clientNamespaced, cp,
kongConsumerGroup := deploy.KongConsumerGroupAttachedToCP(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
deploy.WithAnnotation(metadata.AnnotationKeyPlugins, rateLimitingkongPlugin.Name),
)
t.Cleanup(func() {
Expand Down
32 changes: 25 additions & 7 deletions test/envtest/kongpluginbinding_unmanaged_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ func TestKongPluginBindingUnmanaged(t *testing.T) {
)
defer createCall.Unset()

kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp)
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)
updateKongServiceStatusWithProgrammed(t, ctx, clientNamespaced, kongService, serviceID, cp.GetKonnectStatus().GetKonnectID())

kpb := deploy.KongPluginBinding(t, ctx, clientNamespaced,
Expand Down Expand Up @@ -141,7 +143,10 @@ func TestKongPluginBindingUnmanaged(t *testing.T) {
)
defer createCall.Unset()

kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp)
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)

updateKongServiceStatusWithProgrammed(t, ctx, clientNamespaced, kongService, serviceID, cp.GetKonnectStatus().GetKonnectID())
kongRoute := deploy.KongRouteAttachedToService(t, ctx, clientNamespaced, kongService)
updateKongRouteStatusWithProgrammed(t, ctx, clientNamespaced, kongRoute, routeID, cp.GetKonnectStatus().GetKonnectID(), serviceID)
Expand Down Expand Up @@ -205,7 +210,10 @@ func TestKongPluginBindingUnmanaged(t *testing.T) {
routeID := uuid.NewString()
pluginID := uuid.NewString()

kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp)
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)

updateKongServiceStatusWithProgrammed(t, ctx, clientNamespaced, kongService, serviceID, cp.GetKonnectStatus().GetKonnectID())
kongRoute := deploy.KongRouteAttachedToService(t, ctx, clientNamespaced, kongService)
updateKongRouteStatusWithProgrammed(t, ctx, clientNamespaced, kongRoute, routeID, cp.GetKonnectStatus().GetKonnectID(), serviceID)
Expand Down Expand Up @@ -287,12 +295,17 @@ func TestKongPluginBindingUnmanaged(t *testing.T) {
pluginID := uuid.NewString()
username := "test-user" + uuid.NewString()

kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp)
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)

t.Cleanup(func() {
require.NoError(t, client.IgnoreNotFound(clientNamespaced.Delete(ctx, kongService)))
})
updateKongServiceStatusWithProgrammed(t, ctx, clientNamespaced, kongService, serviceID, cp.GetKonnectStatus().GetKonnectID())
kongConsumer := deploy.KongConsumerAttachedToCP(t, ctx, clientNamespaced, username, cp)
kongConsumer := deploy.KongConsumerAttachedToCP(t, ctx, clientNamespaced, username,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)
t.Cleanup(func() {
require.NoError(t, client.IgnoreNotFound(clientNamespaced.Delete(ctx, kongConsumer)))
})
Expand Down Expand Up @@ -374,9 +387,14 @@ func TestKongPluginBindingUnmanaged(t *testing.T) {
consumerGroupID := uuid.NewString()
pluginID := uuid.NewString()

kongService := deploy.KongServiceAttachedToCP(t, ctx, clientNamespaced, cp)
kongService := deploy.KongService(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)

updateKongServiceStatusWithProgrammed(t, ctx, clientNamespaced, kongService, serviceID, cp.GetKonnectStatus().GetKonnectID())
kongConsumerGroup := deploy.KongConsumerGroupAttachedToCP(t, ctx, clientNamespaced, cp)
kongConsumerGroup := deploy.KongConsumerGroupAttachedToCP(t, ctx, clientNamespaced,
deploy.WithKonnectNamespacedRefControlPlaneRef(cp),
)
updateKongConsumerGroupStatusWithKonnectID(t, ctx, clientNamespaced, kongConsumerGroup, consumerGroupID, cp.GetKonnectStatus().GetKonnectID())

sdk.PluginSDK.EXPECT().
Expand Down
Loading

0 comments on commit 848ac5a

Please sign in to comment.