Conversation
… 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>
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>
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>
…issuer routes 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>
…c traffic control feature. some tests disabled due to know issue with naming convention 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>
…efix 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>
There was a problem hiding this comment.
Pull request overview
This PR implements Dynamic Traffic Control (DTC), an automatic cross-zone failover system that allows subscribers to access APIs from multiple zones without manual failover zone configuration. The system aggregates DTC-enabled zones into a special "dtc" realm, enabling Kong to route traffic across all zones when any subscription has failover enabled.
Changes:
- Added optional
GatewayConfig.DtcUrlfield for zones to enable DTC capability - New
FailoverEnabledfield on Rover subscriptions to auto-discover failover zones from all DTC-eligible zones - Implemented exposure-driven pattern: ApiExposure now creates proxy routes for all cross-zone subscribers instead of ApiSubscription creating them individually
- Routes now support multiple downstreams (one per realm URL) through new
AsDownstreams()method andGetHosts()/GetPaths()helper methods - DTC realms act as "superset" realms containing the default gateway URL plus all DTC URLs from all zones
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
admin/api/v1/zone_types.go |
Added optional DtcUrl field to GatewayConfig |
admin/internal/handler/zone/handler.go |
Created createGatewayDtcRealm() to build superset realms with all DTC URLs and issuers |
rover/api/v1/rover_types.go |
Added FailoverEnabled bool field for automatic DTC zone discovery |
rover/api/v1/traffic_types.go |
Removed Failover field from SubscriberTraffic (now auto-discovered only) |
rover/internal/handler/rover/api/util.go |
New GetDtcEligibleZones() function to find Enterprise-visible zones with DTC URLs |
gateway/api/v1/realm_types.go |
New AsDownstreams() method to create multiple downstreams from realm URLs |
gateway/api/v1/route_types.go |
Added GetHosts() and GetPaths() methods with deduplication logic |
api/internal/handler/util/route_util.go |
Refactored proxy route creation for exposure-driven pattern; simplified route naming |
api/internal/handler/apisubscription/handler.go |
Changed to reference routes from ApiExposure instead of creating them |
api/internal/handler/apiexposure/handler.go |
New exposure-driven pattern: creates proxy routes for all cross-zone subscribers |
api/api/v1/apiexposure_types.go |
Added ProxyRoutes array to track proxy routes created by ApiExposure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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>
stefan-ctrl
left a comment
There was a problem hiding this comment.
Great Job 👍
Have a simple question regarding Empty-Issuers in Downstream configuration and a small remark regarding harded-strings.
| func ForDtcGatewayRealm() string { | ||
| return "dtc" | ||
| } | ||
|
|
There was a problem hiding this comment.
A comment above ForDtcGatewayRealm be nice, explaining why this value is being hardcoded wand what it is used for (i.e. explain the convetion).
Also, "dtc" could also be a constant.
There was a problem hiding this comment.
added a constant + comment
| ) | ||
| realmName := contextutil.EnvFromContextOrDie(ctx) // default | ||
| if hasFailoverSubscription { | ||
| realmName = "dtc" |
There was a problem hiding this comment.
Perhabs in common or admin/api why should have a constant for that, that would be imported here.
There was a problem hiding this comment.
here i just added a comment. importing was problematic due to circles and i think putting it into common would work, but feels like an overkill scope - feel free to discuss it
| It("should handle mismatched URLs and issuers count (more URLs than issuers)", func() { | ||
| realm := &gatewayv1.Realm{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "dtc", | ||
| Namespace: "test-zone", | ||
| }, | ||
| Spec: gatewayv1.RealmSpec{ | ||
| Urls: []string{ | ||
| "https://gateway1.example.com/", | ||
| "https://gateway2.example.com/", | ||
| "https://gateway3.example.com/", | ||
| "https://gateway4.example.com/", | ||
| }, | ||
| IssuerUrls: []string{ | ||
| "https://idp1.example.com/auth/realms/dtc", | ||
| "https://idp2.example.com/auth/realms/dtc", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| downstreams, err := realm.AsDownstreams("/api/v1") | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(downstreams).To(HaveLen(4)) | ||
|
|
||
| // First two downstreams should have matching issuers | ||
| Expect(downstreams[0].IssuerUrl).To(Equal("https://idp1.example.com/auth/realms/dtc")) | ||
| Expect(downstreams[1].IssuerUrl).To(Equal("https://idp2.example.com/auth/realms/dtc")) | ||
|
|
||
| // Last two downstreams should have empty issuer URLs | ||
| Expect(downstreams[2].IssuerUrl).To(BeEmpty()) | ||
| Expect(downstreams[3].IssuerUrl).To(BeEmpty()) | ||
| }) |
There was a problem hiding this comment.
What is the scenario for such a mismatch in Downstreams and IssuerUrls? When do we expect this to happen?
There was a problem hiding this comment.
this can happen with DTC configurations. if you have 3 zones with DTC, then you have 3 + 1 urls (dtc of each zone + the current zones own non-dtc url), but there are no dtc issuers. so you will only have the 3 zones issuers. thus you have 4 vs 3.
also, please be aware that we want to refactor this and move the issuer(s) outside of the downstreams. most likely as a field in security. this will make things clearer, remove the n:m confusion and remove the need to deduplicate values
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>
DTC (Dynamic Traffic Control)
Purpose: Automatic cross-zone failover. Zones with DTC URLs are aggregated into a special "dtc" realm. When any subscription has failover enabled, routes use this realm so Kong can route traffic across all DTC-eligible zones.
Key changes:
admin/zone_types.go— New optionalGatewayConfig.DtcUrlfield on zones.admin/zone/handler.go—createGatewayDtcRealm()builds a superset realm with the default gateway URL + all DTC URLs from all zones, paired with their issuers.route_util.go—CreateProxyRoute()now takes arealmNameparam. Downstream uses the provided realm (default or dtc); upstream always uses the default realm.apiexposure/handler.go— Selects DTC realm whenHasAnySubscriptionWithFailover()returns true.rover/rover_types.go— NewFailoverEnabledbool field. When true, failover zones are auto-discovered.rover/api/util.go— NewGetDtcEligibleZones()returns zones with a DTC URL and Enterprise visibility.rover/api/subscription.go— WhenFailoverEnabled, auto-populates failover zones on ApiSubscription from DTC-eligible zones.