Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(konnect): do not set owner relationship between ControlPlane and its config entities #1099

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
run:
timeout: 5m
timeout: 8m
linters:
enable:
- asciicheck
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,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
35 changes: 30 additions & 5 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,8 +173,8 @@ func (r *KonnectEntityReconciler[T, TEnt]) Reconcile(
}
}
}
// Status update will requeue the entity.
return ctrl.Result{}, nil

return setProgrammedStatusConditionBasedOnOtherConditions(ctx, r.Client, ent)
}
// If a type has a KongService ref, handle it.
res, err = handleKongServiceRef(ctx, r.Client, ent)
Expand Down Expand Up @@ -1009,3 +1008,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.KonnectEntityProgrammedReasonConditionWithStatusFalseExists,
"Some conditions have status set to False",
); errStatus != nil || !res.IsZero() {
return res, errStatus
}
return ctrl.Result{}, nil
}
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/kong/gateway-operator

go 1.23.2
go 1.24.0

// 1.2.2 was released on main branch with a breaking change that was not
// intended to be released in 1.2.x:
Expand All @@ -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.1-0.20250131124626-916233dcfdcd
github.com/kong/kubernetes-configuration v1.1.1-0.20250206104619-59e9230a3a4c
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 @@ -188,7 +188,7 @@ require (
github.com/sergi/go-diff v1.3.1 // indirect
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/spf13/cobra v1.8.1 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/spf13/pflag v1.0.6 // indirect
github.com/stoewer/go-strcase v1.3.0 // indirect
github.com/stretchr/objx v0.5.2 // indirect
github.com/texttheater/golang-levenshtein v1.0.1 // indirect
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
7 changes: 4 additions & 3 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.1-0.20250131124626-916233dcfdcd h1:6tT9I12vgR6/8N6EDWYeteOgpsRB8L2ipQgDn5ChjZo=
github.com/kong/kubernetes-configuration v1.1.1-0.20250131124626-916233dcfdcd/go.mod h1:WAf/rE3AH+y5SUrwG+DVYNCoeI82LUAjT3n4Hu+d20s=
github.com/kong/kubernetes-configuration v1.1.1-0.20250206104619-59e9230a3a4c h1:n+/Ci/ifwlfuy/JVb95jjFSSt5c/P0AozuJreIwvIbo=
github.com/kong/kubernetes-configuration v1.1.1-0.20250206104619-59e9230a3a4c/go.mod h1:wFSxyDrPi3o6m6n/Suj+obbb+WHp4leViuj8POcM9tA=
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 Expand Up @@ -430,8 +430,9 @@ github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9yS
github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0=
github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o=
github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs=
github.com/stoewer/go-strcase v1.3.0/go.mod h1:fAH5hQ5pehh+j3nZfvwdk2RgEgQjAoM8wodgtPmh1xo=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
Expand Down
2 changes: 1 addition & 1 deletion hack/generators/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/kong/gateway-operator/hack/generators

go 1.23.2
go 1.24.0

replace github.com/kong/gateway-operator => ../../

Expand Down
4 changes: 2 additions & 2 deletions hack/generators/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp
github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME=
github.com/spf13/cast v1.7.0 h1:ntdiHjuueXFgm5nzDRdOS4yfT43P5Fnud6DH50rz/7w=
github.com/spf13/cast v1.7.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o=
github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
Expand Down
19 changes: 12 additions & 7 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,13 +195,18 @@ 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) {
switch condition.Type {
case string(consts.ReadyType), string(gatewayv1.GatewayConditionProgrammed):
continue
}
if condition.Type != string(consts.ReadyType) && condition.Status != metav1.ConditionTrue {
return false
default:
if condition.Status != metav1.ConditionTrue {
return false
}
}
}
return true
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.KonnectEntityProgrammedReasonConditionWithStatusFalseExists
}
}

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.KongConsumer(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
Loading
Loading