Skip to content
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

Don't list all RoleBindings and use PMR only #19

Open
kimwnasptd opened this issue Jan 9, 2025 · 2 comments
Open

Don't list all RoleBindings and use PMR only #19

kimwnasptd opened this issue Jan 9, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@kimwnasptd
Copy link
Contributor

Context

This came up from the discussion of the early implementation of the PMR library #17 (comment)

Right now the PMR functions, during create_or_update_profiles(), will remove all RoleBindings and AuthorizationPolicies in a stale Profile. This has the following performance implications

  1. If there's no cache, so that the code efficiently gets the list of RoleBindings, then a request to fetch all RoleBindings will happen every time create_or_update_profiles() is called
  2. The code will then iterate over all RoleBindings to filter the ones that are KFAM RoleBindings

The current architecture was done this way to ensure that if someone onboards the Profile Automator charm(s) to an existing cluster, in which an admin might be manually updating the RoleBindings with custom automation, the state can align with our design.

If a user though starts from a clean cluster, then there's no need to look at all RoleBindings and we could rely on the fact that only RoleBindings were created via the Profile Automator charm(s), thus we could only use the PMR to remove RoleBindings in stale Profiles.

What needs to get done

  1. Re-evaluate once we start seeing performance issues
  2. Iron out the edge cases of the execution path, when handling stale Profiles, if we would only use the PMR
  3. Evaluate if we can
    1. Remove the function list_contributor_rolebindings altogether
    2. Parse the PMR and figure out which RoleBindings need to be deleted

Definition of Done

  1. The code doesn't run list_contributor_rolebindings at all
@kimwnasptd kimwnasptd added the enhancement New feature or request label Jan 9, 2025
Copy link

Thank you for reporting your feedback to us!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-6720.

This message was autogenerated

@kimwnasptd
Copy link
Contributor Author

My main concern with the above is that when figuring out a Profile is stale, this means that it doesn't exist in the PMR at all.

So we couldn't use the PMR to know which RoleBindings / AuthorizationPolicies should be deleted, since the PMR doesn't include any information about this Profile.

An alternative though could be to figure out a more efficient way to ask for KFAM RoleBindings, by filtering with annotations via lightkube. The K8s API Server would still need to do a filtering, but at least the whole list of RoleBindings is not passed back and forth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant