cilium: support service source ranges also for other types #35512
Conversation
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
|
/test |
This was referenced Oct 23, 2024
mhofstetter
reviewed
Oct 23, 2024
There was a problem hiding this comment.
Thanks for the fix and improvements! I left some suggestions inline. 🎉
BTW: Commit checks are failing: Error: ERROR:BAD_SIGN_OFF: Unrecognized email address: '@judge-red' & Error: ERROR:CUSTOM: Please avoid long commit subjects (max: 75, found: 78)
It's a GH handle, in some of the cases we use these in the Reported-by where we don't have an email. The checkpatch commit subject nits imho can be ignored if they're <= ~80 chars, it's mostly a kernel thing and not even there strictly enforced (now obviously, one shouldn't have 90+ chars as subject). |
a720b0c to
8b31e8f
Compare
checkLBSrcRange boolean determines whether the LB map service entry has the flag to perform source CIDR checks or not. This should only be set if the current service (not the old one) contains source CIDRs. When the flag is not cleared after removing loadbalancerSourceRanges from a LoadBalancer service, then all traffic to that service will be blackholed as there is no CIDR entry to allow traffic anymore! Fixes: #32617 Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
8b31e8f to
4827d5d
Compare
|
/test |
mhofstetter
approved these changes
Oct 23, 2024
…her types
Sandro Mathys reported that loadbalancerSourceRanges are only enforced for
LoadBalancer services, but not for NodePort services which poses a problem
in some environments such as OpenStack. He says:
In OpenStack, the NodePort service is the backend of the load balancer,
thus that's where the filtering needs to happen. And thus the NodePort
allocation can't be disabled.
In order to avoid breaking existing behavior, add an option which is called
bpf-lb-source-range-all-types that can be set to {true,false} (default false)
and when enabled it installs the CIDR filters to all services which receive
cluster external (N/S) traffic.
Fixes: #34903
Reported-by: Sandro Mathys <sandro@mathys.io>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Add a config to make this option also accessible to Helm users. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Extend the section to document possibilities for filtering traffic. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Add proper docs as we use this now already in two examples, but a dedicated section with explanation was still missing. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
4827d5d to
413a09a
Compare
|
/test |
learnitall
approved these changes
Oct 23, 2024
dylandreimerink
approved these changes
Oct 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
(see commit msg)