Repository navigation
fix(gateway): copy allowed routes for the edge listener - #4167
BingEdward (7487) wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to This change preserves the Gateway configuration while giving the edge listener its own AllowedRoutes copy. No actionable merge risk remains. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: copying allowed routes for the edge listener. |
| Linked Issues check | |
Issue #4118 requires each CertModeEdge listener to receive its own AllowedRoutes copy. The PR changes the edge listener assignment to edgeAllowedRoutes.DeepCopy(). The existing edge-apex and… |
| Out of Scope Changes check | |
The whole-PR summary reports one assignment change in internal/controller/tenantgateway/reconciler.go. The change only separates ownership of the AllowedRoutes value. It does not change rendered c… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. |
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
github-actions
Bot
added
size/L
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
BingEdward (@7487) NOT LGTM: main moved under this branch, and most of what it fixes is already fixed there. The branch now conflicts in reconciler.go and reconciler_test.go.
#3342 (merged 2026-09-11) reworked renderGateway. On current main (f6a615f) port443Kinds is a constructor function, every https, https-apex and child listener gets httpsAllowedRoutes.DeepCopy(), and each tls-<svc> listener gets its own allowedRoutes.DeepCopy() with fresh kinds. The sharing you describe in the dns01/existingSecret and passthrough paths is gone.
What is still left is exactly the case #4118 reports: the edge listener at reconciler.go:1546 still takes edgeAllowedRoutes itself, while edge-apex and the children get a copy. So after a rebase the fix is AllowedRoutes: edgeAllowedRoutes.DeepCopy() on that one listener. A new listenerAllowedRoutes helper and a rewrite of every call site are no longer needed for that. I would take the one-line change without a new test, since nothing observable changes. If you want to keep the aliasing test, trim it to the edge mode.
I also checked the part that matters most in this file. listenerAllowedRoutes keeps base.Namespaces as is and only swaps the kinds, so this branch does not widen which namespaces can attach routes to any listener.
BingEdward (7487)
changed the title
9902778 to
7cf56fb
Compare
github-actions
Bot
added
kind/bug
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
BingEdward (@7487) LGTM. The branch is now the one-line change: the edge listener gets its own edgeAllowedRoutes.DeepCopy(), same as edge-apex and the children. The trial merge with current main is clean, and go test ./internal/controller/tenantgateway passes on it. The PR body matches the diff.
The edge listener retained the renderer's local allowed-routes value, while the apex and child listeners already owned independent copies. Give edge the same ownership so later changes to the local value cannot reach a rendered listener. The rendered Gateway remains unchanged. Fixes cozystack#4118 Assisted-by: LLM Signed-off-by: 7487 <[email protected]>
7cf56fb to
f2ec6af
Compare
2 participants
What this PR does
Fixes #4118. The
edgelistener retained the renderer's localedgeAllowedRoutesvalue, whileedge-apexand child listeners already received independent copies. Giveedgeits ownDeepCopy()as well, so later changes to the local value cannot reach the rendered listener. The rendered Gateway configuration is unchanged.The change is confined to that one assignment. Current main already creates independent allowed-routes values for HTTPS and TLS-passthrough listeners.
Validation:
go test ./internal/controller/tenantgateway -count=1passes. No new test is added, as accepted in the maintainer's review.Downstream repositories
The diff changes only ownership of a local allowed-routes value; API fields, namespace selectors, route kinds, package layout and rendered resources stay the same, so it reaches none of the downstream trigger map's contracts.
Release note
Summary by CodeRabbit