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

Adding security context to initContainer #756

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

Conversation

kndoni
Copy link

@kndoni kndoni commented Jun 3, 2024

Changes

  1. Adding security context to init container as keyverno policies implies that initContainer should have securityContext defined as well
  2. Adding resources for every container and initContainer on each daemonset.
  3. Adding property devicePluginMps in values that controlles enabling or disabling mps daemonset.

Copy link
Member

@elezar elezar left a comment

Choose a reason for hiding this comment

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

@kndoni please also add this to the other daemonsest in the helm package.

@kndoni
Copy link
Author

kndoni commented Jun 4, 2024

@kndoni please also add this to the other daemonsest in the helm package.

I have added security context for other containers in daemonsets as well

@kndoni kndoni requested a review from elezar June 5, 2024 08:59
elezar
elezar previously approved these changes Jun 5, 2024
@elezar elezar dismissed their stale review June 5, 2024 10:19

needs re-review.

Copy link
Member

@elezar elezar left a comment

Choose a reason for hiding this comment

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

I don't think that adding a security context if we're already running a container as priveleved makes sense. We also do not have the template used for the mps-control-daemon defined.

@kndoni kndoni requested a review from elezar June 5, 2024 10:31
@kndoni
Copy link
Author

kndoni commented Jun 6, 2024

@elezar Let me know if something else should be changed as well.
Thanks in advance!

Adding security context to init container as keyverno policies implies that initContainer should have securityContext defined as well

Signed-off-by: Kristi <74095043+kndoni@users.noreply.github.com>
Signed-off-by: Kristi <74095043+kndoni@users.noreply.github.com>
Signed-off-by: Kristi <74095043+kndoni@users.noreply.github.com>
Signed-off-by: Kristi <74095043+kndoni@users.noreply.github.com>
Signed-off-by: Kristi <74095043+kndoni@users.noreply.github.com>
Signed-off-by: Kristi <74095043+kndoni@users.noreply.github.com>
…s daemonset

Signed-off-by: Kristi <74095043+kndoni@users.noreply.github.com>
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.

None yet

2 participants