gateway-api: exclude conflicted listeners from translation - #47306
Conversation
Gateway listener conflicts are reflected in status, but all configured listeners are currently passed to ingestion. This can cause rejected listeners to be programmed whenever another listener keeps the Gateway valid. Exclude conflicted direct Gateway listeners when building the merged listener model. Update the golden CEC to assert that port 443, its filter chains, and its backend are not emitted. Fixes: cilium#46917 Signed-off-by: Akash Kumar <[email protected]>
maintainer-s-little-helper
Bot
added
the
dont-merge/needs-release-note-label
github-actions
Bot
added
feature/k8s-gateway-api
kind/community-contribution
|
This looks like a good first start towards resolving the issue. A few questions:
If the answer to these questions is "no, no, no", then this looks like a good patch to me. I'm currently reviewing this area of the code to explore those questions. Let me know what you find. |
asauber
added
the
release-note/bug
maintainer-s-little-helper
Bot
removed
the
dont-merge/needs-release-note-label
I've confirmed that the bug does still exist for ListenerSet listeners, but this can be treated as a separate issue.
No, given that the conflicted checks are re-calculated for status writing, this change does not affect status writing
There are at least three missing test cases for this type of "a rejected listener should not reach translation" issue, but they can be treated as separate issues. I will open a PR with each test case on a separate commit so that we can discuss there. Given that this is the correct fix for this specific test case variant. I'm inclined to approve. |
|
/test |
|
I included this commit as part of a wider fix that fixes the bug for ListenerSets as well. #47457 I would say that this is fine to merge, and I can rebase the new PR, or we can close this one and review that one. Either way, thank you for the contribution and please continue to contribute concise fixes for specific issues. |
mhofstetter
added
area/servicemesh
nbusseneau
added
backport-pending/1.20
github-actions
Bot
added
backport-done/1.20
5 participants
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXXline if the commit addresses a particularGitHub issue.
Fixes: <commit-id>tag, thenplease add the commit author[s] as reviewer[s] to this issue.
in accordance with the Cilium AI Policy, and indicate the rating using
AI Influence Level.
Example: "This PR was prepared with AIL:3. I personally checked X."
Gateway listener conflicts (e.g. two listeners sharing a port with
ProtocolConflict) are reflected correctly in Gateway status, but everyconfigured listener was still passed into the translation pipeline. As a
result, rejected/conflicted listeners could still be programmed into Envoy
and serve traffic, as long as the Gateway had at least one other accepted
listener keeping it
Programmed.This excludes conflicted direct Gateway listeners when building the merged
listener model in
resolveAllowedListeners, so only listeners that areactually accepted are handed to translation. The
tlsroute-mixed-protocol-listenersgolden CEC fixture is updated to assert that port 443, its TLS filter
chains, route config, and backend are absent from the resulting Envoy
config, instead of asserting their presence.
Fixes: #46917
This PR was prepared with AIL:3. I personally checked that the conflicted
listener exclusion only applies to already-rejected listeners (Gateway
status
Accepted=False/Conflicted=ProtocolConflict) and verified theregenerated golden fixture no longer emits port 443 or its associated TLS
chains, route config, and backend.