Skip to content

Conversation

squeed
Copy link
Contributor

@squeed squeed commented May 31, 2024

The policy.NewPolicyRepository() took a reference to an ID allocator, but it only used this to determine the initial set of identities. This funny little parameter created a circular reference that is tricky to untangle during initialization.

We can remove this; the initial set of identities is well known and we don't need to create the allocator just to provide that parameter.

So, remove unnecessary parameters and un-circular-ize the policy cell initialization.

@squeed squeed requested review from a team as code owners May 31, 2024 09:40
@squeed squeed requested a review from nebril May 31, 2024 09:40
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 31, 2024
@squeed squeed added kind/cleanup This includes no functional changes. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. release-note/misc This PR makes changes that have no direct user impact. labels May 31, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 31, 2024
@squeed squeed force-pushed the trifecta-cleanup branch from 2f92bb4 to 60a9567 Compare May 31, 2024 10:52
Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice - cilium/envoy ✔️

Copy link
Member

@nebril nebril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ci-structure LGTM

Copy link
Contributor

@thorn3r thorn3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for clustermesh changes 👍

@squeed squeed force-pushed the trifecta-cleanup branch from 60a9567 to 940ae20 Compare June 3, 2024 09:49
@viktor-kurchenko
Copy link
Contributor

/test

@aditighag
Copy link
Member

(Sorry for the delay.)

@squeed squeed force-pushed the trifecta-cleanup branch from 940ae20 to ba45026 Compare June 4, 2024 15:47
@squeed
Copy link
Contributor Author

squeed commented Jun 4, 2024

/test

squeed added 4 commits June 5, 2024 11:04
We no longer allocate identities in the SelectorCache; stop plumbing it
through.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
This utility type doesn't belong in an implementation package.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
The dependencies aren't actually as complicated as the code makes them
out to be. We can construct them iteratively without mutation, now that
we no longer need the cache to list reserved identities.

This is a no-op change.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
I commented it out but forgot to remove it in a prior PR.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
@sayboras sayboras force-pushed the trifecta-cleanup branch from ba45026 to acd565e Compare June 5, 2024 01:04
@sayboras
Copy link
Member

sayboras commented Jun 5, 2024

/test

1 similar comment
@sayboras
Copy link
Member

sayboras commented Jun 5, 2024

/test

@sayboras
Copy link
Member

sayboras commented Jun 5, 2024

I have rebased with main branch to pick up the below CI fix.

#32881

@sayboras sayboras enabled auto-merge June 5, 2024 01:19
@sayboras sayboras added this pull request to the merge queue Jun 5, 2024
Merged via the queue into cilium:main with commit bcbc4c4 Jun 5, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 5, 2024
michi-covalent added a commit to cilium/cilium-cli that referenced this pull request Jun 17, 2024
Update cilium to pick up cilium/cilium#32813.

Ref: #2599

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
michi-covalent added a commit to cilium/cilium-cli that referenced this pull request Jun 17, 2024
Update cilium to pick up cilium/cilium#32813.

Ref: #2599

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
sayboras pushed a commit to cilium/cilium-cli that referenced this pull request Jun 17, 2024
Update cilium to pick up cilium/cilium#32813.

Ref: #2599

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
michi-covalent added a commit that referenced this pull request Jul 16, 2024
Update cilium to pick up #32813.

Ref: #2599

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
michi-covalent added a commit that referenced this pull request Aug 5, 2024
[ cherry-picked from cilium/cilium-cli repository ]

Update cilium to pick up #32813.

Ref: #2599

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 16, 2024
[ cherry-picked from cilium/cilium-cli repository ]

Update cilium to pick up #32813.

Ref: #2599

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants