Policy repository: use SelectorCache to determine subject pods #32849
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 |
christarazi
reviewed
Jun 3, 2024
There was a problem hiding this comment.
Thanks for the PR, a nice simplification. Just one question below.
I noticed a few legitimate CI failures:
- https://github.com/cilium/cilium/actions/runs/9350190323/job/25733138782
- https://github.com/cilium/cilium/actions/runs/9350190264/job/25733251501
- https://github.com/cilium/cilium/actions/runs/9350189823/job/25733246928
- Might require a closer look; a lot of the host firewall (uses host endpoint) cases failed
squeed
commented
Jun 3, 2024
2c07bc0 to
d582284
Compare
|
/test |
|
https://github.com/cilium/cilium/actions/runs/9370599204/job/25797962137#step:13:179 seems like a relevant failure. Host firewall tests failed again, so seems unlikely to be a flake 🤔 |
d582284 to
e02e801
Compare
|
Fixed up the integration tests; some of them had incomplete scaffolding now that the SelectorCache is used in a few more places. No logic issues. |
e02e801 to
5b17c49
Compare
|
/test |
|
Host firewall tests do seem to be broken; marking as DNM while I debug |
While computing the delta on an ID allocation, the SelectorCache incorretly handled the case where a label change caused an identity to no longer be selected by a selectior. The only identity that should have mutable labels is the local host, so this is not actually a visible bug. In preparation for using the SelectorCache to determine policy targets, however, it is now necessary. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
In order to determine applicable identities to which a policy applies, we need to evaluate label selectors. Given that we already have an efficient mechanism for caching label selectors (the SelectorCache), we should use that for subject endpoints as well. This refactors the PolicyRepository to use the SelectorCache when determining subject identities. It removes yet another static cache of matched identities and a corresponding event bus. It also saves memory in the case of reused selectors, which is common. An important consideration is that any new identities must be in the selectorcache *before* that endpoint is regenerated, or else it will not get the correct set of policies. Indeed this is safe, because identity allocation updates the SelectorCache synchronously, and endpoints must have their security identity allocated before they can use it. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
5b17c49 to
4068646
Compare
|
/ci-ginkgo |
|
There, fixed a HostFirewall case I hadn't handled correctly. Thanks, CI! |
|
/test |
derailed
approved these changes
Jun 7, 2024
|
CI is green. @aditighag / @ldelossa could you give this a review, please? |
Hi @squeed I don't have full context, and it's perhaps best reviewed by someone else. I'll remove my review assignment. Sorry for the late response. |
ldelossa
approved these changes
Jun 12, 2024
|
Our bot is a bit confused, we still require an approval from @cilium/endpoint, it will auto merge once that is in |
aanm
approved these changes
Jun 12, 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.
In order to determine applicable identities to which a policy applies, we need to evaluate label selectors. Given that we already have an efficient mechanism for caching label selectors (the SelectorCache), we should use that for subject endpoints as well.
This refactors the PolicyRepository to use the SelectorCache when determining subject identities. It removes yet another static cache of matched identities and a corresponding event bus. It also saves memory in the case of reused selectors, which is common.
An important consideration is that any new identities must be in the SelectorCache before that endpoint is regenerated, or else it will not get the correct set of policies. Indeed this is safe, because identity allocation updates the SelectorCache synchronously, and endpoints must have their security identity allocated before they can use it.