feat(exposure-driven-proxy-routes): proxy routes are now created from api exposures perspective by julius-malcovsky · Pull Request #316 · telekom/controlplane

julius-malcovsky · 2026-04-02T11:18:10Z

… api exposures perspective

… api exposures perspective Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>

Copilot

Pull request overview

This PR implements an exposure-driven pattern for proxy route creation, where API exposures (providers) now create proxy routes for all cross-zone subscriptions, rather than each subscription creating its own routes. This architectural change simplifies subscription logic and centralizes route management under the ApiExposure controller.

Changes:

Centralized proxy route creation: ApiExposure now queries approved subscriptions via FindCrossZoneApiSubscriptionZones() and creates proxy routes for each subscriber zone
Simplified subscriptions: ApiSubscription handlers now only reference routes created by ApiExposure, reducing complexity
Improved watch predicate: ApiExposure controller watches ApiSubscription changes with ResourceVersionChangedPredicate to react to approval status changes (not just spec changes)
Stale route cleanup: New CleanupStaleProxyRoutes() function removes routes that weren't touched in the current reconciliation cycle
Comprehensive tests: Large new test suite validates failover scenarios, deduplication, and approval workflows

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
api/internal/handler/util/route_util.go	New `CleanupStaleProxyRoutes()` function for stale route cleanup
api/internal/handler/util/getters.go	New `FindCrossZoneApiSubscriptionZones()` function to identify zones needing proxy routes
api/internal/handler/apisubscription/handler.go	Refactored to reference proxy routes created by ApiExposure instead of creating them
api/internal/handler/apiexposure/handler.go	Now creates proxy routes for all cross-zone subscriptions; enhanced deletion logic
api/internal/controller/apiexposure_controller.go	Changed ApiSubscription watch to ResourceVersionChangedPredicate
api/config/crd/bases/api.cp.ei.telekom.de_apiexposures.yaml	Added `proxyRoutes` array field to ApiExposureStatus
api/api/v1/apiexposure_types.go	Added `ProxyRoutes` field to ApiExposureStatus struct
api/api/v1/zz_generated.deepcopy.go	Updated deepcopy implementation for ProxyRoutes
api/internal/controller/apisubscription_controller_test.go	Updated tests; removed obsolete failover scenarios
api/internal/controller/apisubscription_controller_failover_test.go	New comprehensive failover test suite

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

api/internal/handler/util/getters.go

Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>

ron96g · 2026-04-02T11:59:37Z

api/internal/handler/apisubscription/handler.go

-		options = append(options, util.ReturnReferenceOnly())
+	if sameZoneAsExposure {
+		// Same zone: reference the real route in exposure zone
+		zone, err := util.GetZone(ctx, scopedClient, apiExposure.Spec.Zone.K8s())


Is this needed? Cant we get them from the ApiExposure status directly? Then we could laos add a Blocked condition to indicate that ApiExposure is not done or whatever.

ron96g · 2026-04-02T12:40:39Z

api/internal/handler/util/getters.go

+
+		// Check approval status
+		approvalGranted := false
+		for _, cond := range sub.GetConditions() {


Have a look at the event domain how its done there. There are already methods to do this

ron96g · 2026-04-02T12:40:54Z

api/internal/handler/util/getters.go

+
+		// Skip subscriptions that are being deleted; their finalizer may still
+		// be running, but they should no longer influence proxy route creation.
+		if sub.GetDeletionTimestamp() != nil {


check event domain how its done there

ron96g · 2026-04-02T12:41:25Z

api/internal/handler/util/getters.go

+	var zones []types.ObjectRef
+
+	// Helper function to add a zone if it's not already seen and passes filters
+	addZoneIfValid := func(zone types.ObjectRef, reason string) {


Personally I think this is really hard to read with the inline function and then the 2 loops

tried a different approach - let me know if its easier on the eyes :)

ron96g · 2026-04-02T12:41:46Z

api/internal/handler/util/route_util.go

+// This handles zone changes: when subscriptions move or are deleted, old proxy routes are cleaned up.
+// NOTE: This does NOT delete provider failover routes (labeled with failover.secondary=true),
+// as those are managed separately by ApiExposure and should not be cleaned up here.
+func CleanupStaleProxyRoutes(ctx context.Context, apiBasePath string) (int, error) {


Do we need this or can we just use the JanitorClient?

Context: Now the ApiExposure always provisions the entire state (all Routes), therefore, the JanitorClient can just cleanup all Rout CRs that were not "touched" in the reconcilation

Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>

…lity Co-authored-by: Björn Kottner <BjoernKarma@users.noreply.github.com> Co-authored-by: Ismael Garba <iagarba@users.noreply.github.com> Co-authored-by: Stefan Siber <stefan-ctrl@users.noreply.github.com> Co-authored-by: Ron Gummich <ron96g@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 2, 2026 11:18

Copilot started reviewing on behalf of julius-malcovsky April 2, 2026 11:18 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

api/internal/handler/util/getters.go Outdated Show resolved Hide resolved

julius-malcovsky self-assigned this Apr 2, 2026

julius-malcovsky requested a review from ron96g April 2, 2026 11:56

ron96g requested changes Apr 2, 2026

View reviewed changes

julius-malcovsky and others added 2 commits April 3, 2026 15:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(exposure-driven-proxy-routes): proxy routes are now created from api exposures perspective#316

feat(exposure-driven-proxy-routes): proxy routes are now created from api exposures perspective#316
julius-malcovsky wants to merge 4 commits intomainfrom
feat/exposure-driver-proxy-routes

julius-malcovsky commented Apr 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

ron96g Apr 2, 2026

Uh oh!

julius-malcovsky Apr 3, 2026

Uh oh!

ron96g Apr 2, 2026

Uh oh!

ron96g Apr 2, 2026

Uh oh!

julius-malcovsky Apr 3, 2026

Uh oh!

ron96g Apr 2, 2026

Uh oh!

julius-malcovsky Apr 3, 2026

Uh oh!

ron96g Apr 2, 2026

Uh oh!

ron96g Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

julius-malcovsky commented Apr 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants