-
Notifications
You must be signed in to change notification settings - Fork 168
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
New securityContext options and annotations #131
Conversation
Control pod Spec Co-Authored-By: Theron Voran <tvoran@users.noreply.github.com>
Adjust description Co-Authored-By: Theron Voran <tvoran@users.noreply.github.com>
Remove GroupId to 0 in case of SameID Co-Authored-By: Theron Voran <tvoran@users.noreply.github.com>
Adjust description Co-Authored-By: Theron Voran <tvoran@users.noreply.github.com>
Adjust description Co-Authored-By: Theron Voran <tvoran@users.noreply.github.com>
I'm also wondering if this should be an annotation in addition to (or instead of) the command-line option. |
To have the same behavior like the others command line I think is a good idea. |
agent-inject/agent/annotations.go
Outdated
if pod.Spec.Containers[0].SecurityContext.RunAsUser == nil { | ||
return errors.New("RunAsUser is nil for Container 0's SecurityContext") | ||
} | ||
cfg.UserID = strconv.FormatInt(*pod.Spec.Containers[0].SecurityContext.RunAsUser, 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should be looking at pod.Spec.SecurityContext
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Spec.SecurityContext
is common for all containers running inside the pod.
The Spec.Container[0].SecurityContext
overrides the settings at the pod level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, and after running this on OpenShift via CRC, I see that RunAsUser
is being set at the container level.
I'm also now thinking that it might be simpler to just not set a SecurityContext at all on the init and sidecar containers, since OpenShift will do that for you based on the project/namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm I have to double check it, but the reason that we explicitly put the security context is because we are injecting the side/init-container with the webhook and it is not OpenShift doing it.
Setup the RunAsSameUser option as both a startup option and annotation, to match the other user and group options. Added a SetSecurityContext option that controls whether or not the SecurityContext is set on the injected containers.
Indeed, so now RunAsSameUser is available as both a command-line option and an annotation, to match the user and group options. I also added a SetSecurityContext command-line option and annotation, to control whether the securityContext is set or not. Setting this to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor stuff but overall looks great!
Adds an option for running the injected vault containers as the User of the first application container in the Pod. Requires `Spec.Containers[0].SecurityContext.RunAsUser` to be set in the pod spec. Defaults to false. Available as: - `-run-as-same-user` command-line option - `AGENT_INJECT_RUN_AS_SAME_USER` environment variable - `vault.hashicorp.com/agent-run-as-same-user` annotation Also adds an option for controlling whether or not the SecurityContext is set on injected containers. Defaults to true. Available as: - `-set-security-context` command-line option - `AGENT_INJECT_SET_SECURITY_CONTEXT` environment variable - `vault.hashicorp.com/agent-set-security-context` annotation Co-authored-by: Mattia <mmascia@redhat.com>
(Continuation of #126)
Adds an option for running the injected vault containers as the User of the first application container in the Pod. Requires
Spec.Containers[0].SecurityContext.RunAsUser
to be set in the pod spec. Defaults to false. Available as:-run-as-same-user
command-line optionAGENT_INJECT_RUN_AS_SAME_USER
environment variablevault.hashicorp.com/agent-run-as-same-user
annotationAlso adds an option for controlling whether or not the SecurityContext is set on injected containers. Defaults to true. Available as:
-set-security-context
command-line optionAGENT_INJECT_SET_SECURITY_CONTEXT
environment variablevault.hashicorp.com/agent-set-security-context
annotation