Allow selecting nodes by CIDR policy#27464
Merged
nathanjsweet merged 5 commits intomainfrom Sep 15, 2023
Merged
Conversation
ca47340 to
49b3622
Compare
Contributor
Author
|
CI is not interesting because this is all behind a feature flag. I'll add a do-not-merge commit that changes the default. |
49b3622 to
0133006
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
Author
|
Test failures are all uninteresting; mostly checkpatch related issues (which is intentional) and one test that needs to be updated to handle node identities being enabled by default. Nice! |
0133006 to
d5bddb3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
d5bddb3 to
6084a1d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
2 tasks
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
Author
|
All remaining functional tasks are done. This PR is ready for full review. Un-marking as draft. Note: there is still a do-not-merge commit which forces the feature-flag on. This will be removed before merging. We're working on adding integration tests; these may be added in a follow-up commit, as they will require cilium-cli support. |
- identity: Properly Assign Host Identity to IPs When the option "PolicyCIDRSelectsNodes" is set to true then we need to make sure to label all host IPs with their CIDR labels as well so that they are selectable. When nodemanager adds or updates the local host it is on, the IP addresses associated with it are added as labels to the host identity before it is initialized for the IPCache entry. - policy: Unit Tests For Cluster Entity CIDR Selection When cluster entities are CIDR selectable via the PolicyCIDRMatchMode option, then test for these entities being selectable in a policy via a CIDR rule. Signed-off-by: Nate Sweet <nathanjsweet@pm.me>
This adds the ability to enable this NetworkPolicy feature wherein nodes are selectable by CIDR selectors. Ordinarily this is not allowed. This commit just wires up the option. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
6ff0870 to
cd8d773
Compare
Member
|
/test |
christarazi
approved these changes
Sep 14, 2023
Member
christarazi
left a comment
There was a problem hiding this comment.
LGTM now as there no outstanding concerns. Please go through the remaining unresolved conversations, as I still see 7 as reported by the GH UI.
Member
|
@gandro I added you to the reviewers to review the "helm" parts. Thanks 🙏 . |
This was referenced Oct 3, 2023
This was referenced Feb 27, 2026
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
11 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.
This PR adds a new option,
policy-cidr-selects-nodes, which means that cluster nodes can be selected by CIDR network policies. Normally nodes are only accessible viaremote-nodeentity selectors.How it works:
We already have the notion of an "identity scope" -- if the top 8 bits of the 32-bit identity are 0, we assume the ID is global. If the top bits are 0x01, then the ID is a local CIDR identity. This PR declares the scope 0x02 to be for node identities.
A separate identity scope is needed because the datapath needs to be able to quickly determine if a given identity corresponds to a remote node. Previously, all nodes had reserved identities 4 or 7. By using a separate identity scope, a fast bit comparison can be done without any additional map lookups.
So, most of this PR consists of refactoring commits to prepare for scoped local identity allocators. Then, a few commits at the end enable the behavior, switching nodes to the new identity scope and attaching
cidr:labels to the local-node identity.Fixes: #20550