fix(controllers): prevent deleting of referenced realms and gateways

[ron96g]

No description provided.

[Copilot]

Pull request overview

This PR adds “deletion guards” to prevent removal of Identity Realms and Gateways while they are still referenced by dependent resources (Identity Clients; Gateway Routes/Consumers). It also adds controller watches and integration tests to ensure terminating objects get re-reconciled once references are deleted.

Changes:

• Block Realm deletion when any Client still references it; trigger Realm reconcile on Client deletion.
• Block Gateway deletion when any Route or Consumer still references it; trigger Gateway reconcile on Route/Consumer deletion.
• Add integration tests covering “terminating until references are gone” for both Realm and Gateway.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
identity/test/utils/mocks.go	Extends Keycloak mock realm matcher to include the new deletion-guard test realm.
identity/internal/handler/realm/handler.go	Prevents deleting a Realm while it is still referenced by Clients (blocked error).
identity/internal/handler/realm/handler_test.go	Adjusts test mock setup to tolerate the new List() call during Realm operations.
identity/internal/controller/realm_controller.go	Adds watch from Client deletions to the referenced Realm to re-trigger reconciliation during deletion.
identity/internal/controller/realm_controller_test.go	Adds integration test verifying a referenced Realm stays terminating until its Client is deleted.
gateway/internal/handler/route/handler.go	Adjusts Route deletion behavior to tolerate missing Gateway during cleanup.
gateway/internal/handler/gateway/handler.go	Prevents deleting a Gateway while it is still referenced by Routes/Consumers (blocked error).
gateway/internal/handler/gateway/handler_test.go	Updates GatewayHandler Delete test to provide a client via context for List() calls.
gateway/internal/handler/consumer/handler.go	Adjusts Consumer deletion behavior to tolerate missing Gateway during cleanup.
gateway/internal/controller/index.go	Adds field indices to look up Routes/Consumers by Gateway reference efficiently.
gateway/internal/controller/gateway_controller.go	Adds watches from Route/Consumer deletions to the referenced Gateway to re-trigger reconciliation during deletion.
gateway/internal/controller/controller_test.go	Adds integration test verifying a referenced Gateway stays terminating until Route and Consumer are deleted.

Suppressed comments (1)

gateway/internal/controller/gateway_controller.go:75

• mapRouteToGateway can enqueue a reconcile request with an empty name/namespace if a Route is deleted with an empty spec.gatewayRef (the field is required but subfields are not validated for non-empty). Guard against empty refs to avoid reconciling "" and spurious errors.

func (r *GatewayReconciler) mapRouteToGateway(_ context.Context, obj client.Object) []reconcile.Request {
	route, ok := obj.(*v1.Route)
	if !ok {
		return nil
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (3)

gateway/internal/controller/index.go:20

• The new shared index name spec.gatewayRef is also used to index Consumers, but the Consumer API field is spec.gateway (not spec.gatewayRef). This diverges from the existing pattern where index keys match the actual spec paths (e.g., spec.route, spec.route.name).

Consider introducing a separate index key for Consumers (e.g. spec.gateway) and using it in the GatewayHandler list selector to keep the index naming self-descriptive.

var IndexFieldSpecRoute = "spec.route"
var IndexFieldSpecRouteName = "spec.route.name"
var IndexFieldSpecGatewayRef = "spec.gatewayRef"

gateway/internal/handler/gateway/util.go:29

• GetGatewayByRef now returns the Gateway object even when it isn't Ready, but it returns early before running the resolveSecrets logic. Callers like Route/Consumer Delete pass resolveSecrets=true and then use the returned Gateway to build a Kong client; skipping secret resolution here can cause deletion to fail for Gateways that rely on secret-manager refs.

Consider honoring resolveSecrets regardless of readiness and return (ready, gateway, nil) at the end.

	if !meta.IsStatusConditionTrue(gateway.GetConditions(), condition.ConditionTypeReady) {
		return false, gateway, nil
	}

gateway/internal/handler/consumer/handler.go:51

• In ConsumerHandler.Delete, the Kong delete failure is wrapped with the message "failed to create or update consumer". This is a delete path, so the message is misleading and makes troubleshooting harder.

		// There is no gateway configuration available for deleting the Kong consumer.
		if apierrors.IsNotFound(err) {
			return nil
		}
		return err

[julius-malcovsky]

wondering if it would make sense to add the client.Limit(1) for the listing .. were curious if there are any routes at all, we dont need all of them ... yes this is not a hotpath so its nitpicking :) ... plus we would lose the nice error message ... just an idea :)

[ron96g]

Yes or/and only get metadata if possible

[julius-malcovsky]

can you remind me whats the point of reprocessing the gateway everytime a route is deleted ? i know we talked about it, just cant recall

[ron96g]

For correctness to cleanup but I think we should use a RetryWithDelay error instead of a Watcher... Also the Route should watch the Gateway (Route -> gen_change -> Gateway)

[stefan-ctrl]

Talked in the team; will leave this PR open for now and discuss it in more detail after the German vacation time.