New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix kube-proxy bug with multiple LB IPs and source ranges #109826
fix kube-proxy bug with multiple LB IPs and source ranges #109826
Conversation
@danwinship: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The LoadBalancer rules change if the node IP is in one of the LoadBalancerSourceRange subnets, so make sure to set nodeIP on the fake proxier so we can test this, and add a second source range to TestLoadBalancer containing the node IP. (This changes the result of one flow test that previously expected that node-to-LB would be dropped.)
The various loops in the LoadBalancer rule section were mis-nested such that if a service had multiple LoadBalancer IPs, we would write out the firewall rules multiple times (and the allowFromNode rule for the second and later IPs would end up being written after the "else DROP" rule from the first IP).
a150292
to
813aca4
Compare
/lgtm |
/retest |
The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass. This bot retests PRs for certain kubernetes repos according to the following rules:
You can:
/retest |
1 similar comment
The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass. This bot retests PRs for certain kubernetes repos according to the following rules:
You can:
/retest |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Due to incorrect nesting of the LB-related loops in syncProxyRules, if you had a LoadBalancer service with multiple LoadBalancer IPs, and a LoadBalancerSourceRange that overlapped the node IP, we would generate the rules incorrectly (with the result that traffic from the node to the second LB IP would probably be dropped rather than accepted).
eg, given LoadBalancer IPs
1.2.3.4
and5.6.7.8
, node IP192.168.0.2
, and source ranges[192.168.0.0/24, 203.0.113.0/25]
, we would generate:rather than the desired
(The allow-from-LB-IP rule is added only when the source ranges overlap the node IP, and is explained in the code as:
)
This is not a regression; our refactorings in 1.24 correctly preserved the existing behavior, which has apparently always been wrong. (It's probably uncommon to have multiple LB IPs for the same service, so no one ever noticed?)
Which issue(s) this PR fixes:
none; discovered while refactoring for another PR
Does this PR introduce a user-facing change?
/sig network
/priority important-soon