pkg/loadbalancer: further restrict use of wildcard svc entries#43721
Merged
julianwiedmann merged 2 commits intocilium:mainfrom Jan 14, 2026
Merged
pkg/loadbalancer: further restrict use of wildcard svc entries#43721julianwiedmann merged 2 commits intocilium:mainfrom
julianwiedmann merged 2 commits intocilium:mainfrom
Conversation
Following the introduction of wildcard service entries in the loadbalancer in commit b3b0fc8, it became possible to disrupt internal cluster traffic, where a VIP was allocated via NodeIPAM. An initial fix was applied for this scenario in commit c4d6072, which updated the loadbalancer's BPF reconciler to restrict the use of wildcard service entries if using NodeIPAM, either by default or via loadBalancerClass. Unfortunately the revised logic still permits wildcard service entries if non-Cilium IPAM is used, which can still cause disruption to cluster traffic [0]. This commit reverses the loadBalancerClass check to instead only use wildcard service entries if the loadBalancerClass is defined and is either Cilium's BGPLoadBalancerClass or L2AnnounceLoadBalancerClass. This change has required modification of unit tests that set a dummy loadBalancerClass. However, additional tests will be added in the next commit to verify this change in behaviour. [0] cilium#43206 (comment) Fixes: c4d6072 ("loadbalancer: do not create wildcard entries for Node-IPAM VIPs") Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> Signed-off-by: Alasdair McWilliam <alasdair.mcwilliam@isovalent.com>
This commit adds additional test coverage with respect to the programming of wildcard service entries based on how the VIP address has been allocated. Coverage expects wildcard service entries to be programmed for: - undefined loadBalancerClass - loadBalancerClass: io.cilium/bgp-control-plane - loadBalancerClass: io.cilium/l2-announcer Coverage expects no wildcard service entries to be programmed for: - defined loadBalancerClass but not matching any of the above - loadBalancerClass: io.cilium/node Signed-off-by: Alasdair McWilliam <alasdair.mcwilliam@isovalent.com>
Member
Author
|
/test |
Member
Author
|
/ci-l3-l4 |
Member
|
Closed
3 tasks
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
6 participants
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.
Following PR #43565, the loadbalancer's BPF reconciler was updated to avoid programming wildcard service entries with NodeIPAM allocated VIPs.
Unfortunately the fix was a little too focused and does not address use cases of different classes, e.g. those not provided by Cilium. This means we can still program a wildcard entry for an IP not allocated by Cilium, resulting in connectivity faults in a node.
This PR reverses the wildcard candidate check, such that if a loadBalancerClass is not provided by Cilium, we do not program a wildcard service entry.
Also included is additional test coverage for this.
Fixes: #43206 (again!)
This fix should be applied to the v1.19 branch as well - let me know if I should raise a separate PR for this, or if it will just be backported.