feat: support SecurityPolicy client-IP authorization for UDPRoute - #9833
Open
zhaohuabing wants to merge 12 commits into
Open
feat: support SecurityPolicy client-IP authorization for UDPRoute#9833zhaohuabing wants to merge 12 commits into
zhaohuabing wants to merge 12 commits into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9833 +/- ##
==========================================
+ Coverage 76.34% 81.28% +4.93%
==========================================
Files 261 264 +3
Lines 44607 40838 -3769
==========================================
- Hits 34056 33194 -862
+ Misses 8297 7644 -653
+ Partials 2254 0 -2254 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
zhaohuabing
changed the title
zhaohuabing
marked this pull request as ready for review
August 27, 2026 07:01
Adds UDPRoute to the SecurityPolicy targetRef/targetRefs kind allow-lists and to the mergeType xRoute list, so client-IP CIDR authorization can be expressed for UDP the same way it already is for TCPRoute. This is the API layer only; the gatewayapi translator, IR and xDS support follow separately, mirroring how TCPRoute support landed. Fixes envoyproxy#9832 Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
A UDP listener has no network filter chain, so the network RBAC filter used for TCPRoute authorization cannot be reused. Instead the authorization decision rides along with route selection in the matcher that udp_proxy already uses: a datagram whose source IP matches is routed to the cluster, and one that matches nothing is dropped by udp_proxy and counted as downstream_sess_no_route. Because a denial can only be expressed as the absence of a match, the ordered Allow/Deny rules are compiled into allow-only predicates. A Deny rule never becomes an entry of its own; it subtracts from the Allow rules that follow it and from a permissive default action. An allowlist therefore emits one entry per Allow rule with no on_no_match, and a denylist collapses to a single negated entry. Adds Authorization to ir.UDPRoute. Nothing populates it yet. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
Wires client-IP CIDR authorization through to ir.UDPRoute, for SecurityPolicies targeting a UDPRoute as well as those targeting a Gateway, a Gateway listener, a ListenerSet or a ListenerSet listener — matching what TCP already supports. TCP and UDP accept and reject exactly the same fields, so validateSecurityPolicyForTCP becomes validateSecurityPolicyForL4, taking the protocol name for its error messages. A UDP listener holds at most one route: when several UDPRoutes name the same listener only the oldest is attached, so a policy targeting one of the others has nothing to apply to. The TCP case also gains the nil listener guard it was missing. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
zhaohuabing
force-pushed
the
api-sp-udproute
branch
from
August 27, 2026 07:01
f0e66a4 to
e5d3f92
Compare
Member
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5d3f9225a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three UDP listeners on one Gateway: one allowing every source, one allowing only a private range the client can never be in, and one denying by default with no rules. All three share a Gateway on purpose — a config Envoy rejects would take down the whole proxy, so the allowed listener answering is what proves the other two are dropping rather than simply missing. The deny-all policy targets the Gateway listener rather than the route, so both attachment paths are covered. The deny assertions are not polls. A denied datagram draws no reply, so a poll that retried until it succeeded would report success after a full minute of correct denials; each deny case instead requires three consecutive queries to go unanswered, after the allowed listener has already answered. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
…tions The old fixture put the Deny rule first with an Allow range wholly inside it, so the emitted Allow entry could never match and read as redundant. Swapping to allow-then-deny alone would have lost coverage of the narrowed-Allow predicate instead. The rule list now runs Allow, Deny, Allow, with the trailing Allow only partly overlapping the Deny. Every emitted entry is reachable and each covers a distinct mechanism: an Allow that precedes the Deny needs no exclusion, one that follows it is narrowed by it, and the permissive default becomes a catch-all. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
Apply a route-targeted policy only to the IR route it targets. Several xRoutes can name the same L4 listener while only one of them is attached, yet the unattached ones keep that listener in their parent context. A policy targeting an unattached route therefore reached the attached route and changed the authorization of the route actually serving traffic — a Deny-by-default policy meant for a dormant route could black-hole the live one. Both the TCP and the new UDP paths are affected; compare the IR route name against the target. Reject Authorization rules that carry an operation on an L4 target. Methods and paths are HTTP concepts and the L4 matcher only reads the client CIDRs, so "allow GET from 10.0.0.0/8" was silently translated to "allow everything from 10.0.0.0/8". This affected TCPRoute before UDPRoute existed. Both were found in review of the UDPRoute work; the TCP halves are pre-existing. No existing testdata changed, so neither path was covered before. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
The URL was guessed and 404s, breaking docs-check-links. Use the same reference/api-types path as the HTTPRoute and GRPCRoute entries beside it. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
Member
Author
|
@codex review |
The task's example targets an HTTPRoute, which reads as though IP allowlists/denylists were HTTP-only. Say that the same policy works on a TCPRoute or UDPRoute, and that only clientCIDRs applies there. Also scope the Original Source IP section, which is entirely about X-Forwarded-For and so does not describe how the client IP is obtained on an L4 route. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9aa5371b1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ul.Route.Authorization != nil { | ||
| continue | ||
| } | ||
| ul.Route.Authorization = l4Authorization |
There was a problem hiding this comment.
When a SecurityPolicy targets a Gateway, ListenerSet, or one of their UDP listeners, this path bypasses validateSecurityPolicyForL4 and attaches the complete authorization object. A valid policy intended for a mixed-protocol Gateway can therefore combine clientCIDRs with a JWT, header, or operation constraint, but buildUDPProxyMatcher evaluates only the CIDRs and silently admits UDP traffic without satisfying the other required constraint; rules without CIDRs can instead collapse to the default action. The new route-level validation does not cover these listener-scoped targets, so validate the policy against each targeted UDP listener and surface a status error rather than applying a partial rule.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment.
This is also the current behavior for TCPRoutes, let's address them both in a follow-up PR to keep this one focused. #9868
The test waited for pods labelled app=udp and reused the coredns Service that testdata/udproute.yaml creates. That manifest is deleted when the UDPRoute test cleans up, and the UDPRoute test runs first, so the backend was already gone and WaitForPods timed out after five minutes. Ship a coredns Service, ConfigMap and Deployment named udp-authz-coredns and labelled app=udp-authz, and point all three routes at it. This is what UDPRouteSourceIPConsistentHash does with udp-lb-coredns, and it makes the test independent of what any other test leaves behind. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
The coredns block was copied from udproute-consistent-hash-source-ip.yaml, which indents sequences under their key, while the rest of the file keeps them flush. yamllint is configured with indent-sequences: consistent, so mixing the two in one file fails. Re-indent the copied block to match the rest. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
Member
Author
|
/retest |
1 similar comment
Member
Author
|
/retest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allows
SecurityPolicyto target aUDPRoute, giving UDP services — DNS, syslog, NTP, telemetry collectors, game servers, VoIP/RTP — the same client-IP allowlist/denylist already available forTCPRoute. As with TCP, onlyauthorizationrules usingprincipal.clientCIDRsapply.Fixes #9832
How it works
A UDP listener has no network filter chain, so the network RBAC filter used for TCPRoute authorization cannot be reused. The decision instead rides along with route selection in the
matcherthatudp_proxyalready uses: a datagram whose source IP matches routes to the cluster, and one that matches nothing is dropped and counted asdownstream_sess_no_route.Because a denial can only be expressed as the absence of a match, the ordered Allow/Deny rules are compiled into allow-only predicates — a Deny rule never becomes an entry of its own, it subtracts from the Allow rules that follow it and from a permissive default. An allowlist emits one entry per Allow rule with no
on_no_match; a denylist collapses to a single negated entry.Enforcement happens at session creation rather than per packet. That is harmless here because the session key includes the source IP, so a denied source never gets a session. Note there is no dedicated denied counter — drops land in
downstream_sess_no_route, unlike TCP'srbac.denied.mergeTypeacceptsUDPRoutetoo, matchingBackendTrafficPolicy.