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

Add dummy service accounts to hook-image-puller and continuous-image-puller pods #3594

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

samyuh
Copy link

@samyuh samyuh commented Jan 10, 2025

This MR adds two separate dummy ServiceAccounts for hook-image-puller and continuous-hook-image-puller.

  • The continuous image puller runs constantly to ensure images are always available.
  • The hook image puller is temporary, used during tasks like upgrades, and is deleted afterward.

Closes #3545

@consideRatio
Copy link
Member

I'm super low on time, but lets try this - do your best to address the review comments, and i'll try get it through to a merge by finishing details this weekend.

Details if you want to try work those as well, from the top of my head would include:

  • ensure lint and validate values can trial configure this if new config is introduced
  • render and check labels etc align with how other continious/hook resources have labels
    I'm not able to really think properly about this now as i'm taking care of my toddler, sneaking in a review effort quickly from my mobile

@samyuh
Copy link
Author

samyuh commented Jan 10, 2025

Thanks for the feedback, I will address everything today.

@samyuh
Copy link
Author

samyuh commented Jan 10, 2025

Tests are failing since https://mybinder.org/ is down.

@samyuh samyuh requested a review from consideRatio January 10, 2025 16:05
@consideRatio consideRatio changed the title feat: Add service account to hook-image-puller and continuous-image-puller Add dummy service accounts to hook-image-puller and continuous-image-puller pods Jan 10, 2025
@consideRatio
Copy link
Member

consideRatio commented Jan 10, 2025

@samyuh great work! I pushed two detail commits.

@manics do you have time for a quick gut-check evaluation? This PR adds two dummy service accounts to be used by the the image-puller pods to please various security checks as discussed in #3545 (comment).

Do we make them enabled by default, or not?

I'm leaning towards enabled by default to reduce the hassle for anyone needing to comply with these benchmarks etc, but I'm not fully confident it has no drawbacks for users in other situations.

@samyuh
Copy link
Author

samyuh commented Jan 10, 2025

@samyuh great work! I pushed two detail commits.

@manics do you have time for a quick gut-check evaluation? This PR adds two dummy service accounts to be used by the the image-puller pods to please various security checks as discussed in #3545 (comment).

Do we make them enabled by default, or not?

I'm leaning towards enabled by default to reduce the hassle for anyone needing to comply with these benchmarks etc, but I'm not fully confident it has no drawbacks for users in other situations.

Thanks for your effort and your review!

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

Successfully merging this pull request may close these issues.

hook and continuous image pullers' DaemonSets: configuring k8s ServiceAccount - yes or no?
2 participants