fix(gatewayapi): prefer sectioned BackendTLSPolicy over wildcard for same target#9068
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e23c272bf6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…Policy Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com>
…same target When two BackendTLSPolicies target the same backend — one with sectionName set, one without — the wildcard wins whenever it sorts earlier, since policies are iterated in creationTimestamp order with no specificity preference. The sectioned policy is silently dropped: never selected by the translator, no status written. Make getBackendTLSPolicy prefer policies with an explicit sectionName match before falling back to wildcard matches. Matches the specificity ordering SecurityPolicy already implements. Refs envoyproxy#9035 Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com>
1ff61c0 to
e5cfd08
Compare
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9068 +/- ##
=======================================
Coverage 74.76% 74.76%
=======================================
Files 252 252
Lines 40571 40584 +13
=======================================
+ Hits 30333 30344 +11
Misses 8167 8167
- Partials 2071 2073 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zirain
left a comment
There was a problem hiding this comment.
Can you add a release notes for this?
Signed-off-by: AlecDiraimondo <50001153+AlecDiraimondo@users.noreply.github.com>
Signed-off-by: Alec Diraimondo <drayalec@gmail.com>
|
@zirain sorry had to rebase and dismiss review. Also how do I rerun the ci-checks? Was unable to find the button |
| } | ||
|
|
||
| // stricter variant: only matches when sectionName is set and equal | ||
| func backendTLSTargetSectionMatched(policy *gwapiv1.BackendTLSPolicy, target gwapiv1.LocalPolicyTargetReferenceWithSectionName, backendNamespace string) bool { |
There was a problem hiding this comment.
See backendTLSTargetMatched if we can dedup this
| ) *gwapiv1.BackendTLSPolicy { | ||
| // SectionName is port number for EG Backend object | ||
| target := t.getTargetBackendReference(backendRef, backendNamespace) | ||
| for _, policy := range policies { |
There was a problem hiding this comment.
can we return early if target.SectionName == nil?
What this PR does / why we need it
This PR makes
getBackendTLSPolicyprefer policies with an explicitsectionNamematch before falling back to wildcard matches. The pattern mirrors the specificity orderingSecurityPolicyimplements, for example.Currently, when two
BackendTLSPoliciestarget the same backend — one withsectionNameset, one without — the wildcard policy wins whenever it sorts earlier, since policies are iterated increationTimestamporder with no specificity preference. The sectioned policy is silently dropped: never selected by the translator, no status written.The Gateway API BackendTLSPolicy spec states precedence checks should happen at matching sectionName and target. This is slightly up for interpretation, but honoring the more specific match seems to be the more idiomatic approach.
The existing conformity test for gatewayAPI passes https://github.com/kubernetes-sigs/gateway-api/blob/3e4ef89ce6d9a21202275739566d7c649211620c/conformance/tests/backendtlspolicy-conflict-resolution.go#L117-L152 due to the sectioned backendTLSPolicy being created first
The included testdata case (
backendtlspolicy-wildcard-older-than-sectioned) demonstrates this case, and the golden config shows the corrected behavior:policy-sectioned.status.ancestorsnow populates withAccepted: True(wasnull).sniflips fromwildcard.example.com→sectioned.example.com.Which issue(s) this PR fixes
Fixes #9035