SHIP-0036: Proposal to set runAs user/group for supporting steps to the runAs configuration of the build strategy steps #131
Conversation
SaschaSchwarze0
requested review from
adambkaplan,
HeavyWombat,
otaviof and
qu1queee
and removed request for
sbose78 and
gabemontero
SaschaSchwarze0
force-pushed
the
sascha-0036-runas-user
branch
from
04c33b6 to
c50c06b
Compare
SaschaSchwarze0
force-pushed
the
sascha-0036-runas-user
branch
from
c50c06b to
56193ed
Compare
|
/lgtm |
SaschaSchwarze0
force-pushed
the
sascha-0036-runas-user
branch
from
56193ed to
e3f87bd
Compare
|
Proposal was updated to reflect the result of investigations on how to run the |
SaschaSchwarze0
changed the title
SaschaSchwarze0
force-pushed
the
sascha-0036-runas-user
branch
3 times, most recently
from
192c21b to
0a98971
Compare
…s configuration of the build strategy steps
SaschaSchwarze0
force-pushed
the
sascha-0036-runas-user
branch
from
0a98971 to
32ccc8d
Compare
|
Added strategy-level |
SaschaSchwarze0
force-pushed
the
sascha-0036-runas-user
branch
from
420e1b3 to
8be7b31
Compare
There was a problem hiding this comment.
/lgtm
I think that some of the details in this proposal only apply to Shipwright-provided steps (like 95% confident). I'm not going to hold back on merging this SHIP given its age and prior iterations.
@SaschaSchwarze0 if I have time, I may write a follow-up PR that adds more to the "non-goals" section.
|
|
||
| ### Base image changes | ||
|
|
||
| Container images define which users and groups exist in the container by specifying the content of `/etc/passwd` and `/etc/group`. Though, it is generally possible to run a container with as a user or group that is not included in the container image. |
There was a problem hiding this comment.
it is generally possible to run a container with as a user or group that is not included in the container image.
OpenShift does this by default - containers that don't specify runAsUser and runAsGroup get assigned a random, high-value UID/GID.
|
|
||
| We therefore must ensure that those files contain proper information. | ||
|
|
||
| We will change the base image to contain `/etc/passwd` and `/etc/group` files with the content that applies to the default Shipwright configuration. That will be for `/etc/passwd`: |
There was a problem hiding this comment.
We will change the base image...
Is this the base image for user workloads? Or is this only referencing the base image we use for our "provided" steps (git-clone, etc.)
There was a problem hiding this comment.
Our steps, we have no control at all over the (base) image of strategy steps.
|
|
||
| The BuildRun reconciler is changed to check the strategy-level `securityContext`. If it is defined, then their `runAsUser` and `runAsGroup` override those settings from the container templates of Shipwright-managed steps in the configuration. They are also defaults for the strategy steps' securityContext. | ||
|
|
||
| If the strategy-level `securityContext` is defined, then the reconciler will setup the `/etc/passwd` and `/etc/group` files to match this configuration. The following annotations are added to the TaskRun (assuming a build strategy that runs as user 1010 and group 1020): |
There was a problem hiding this comment.
Again, I think we need to be clear here on which containers we are modifying /etc/passwd and /etc/group. If these are for the Shipwright pre/post steps, this is fine. For arbitrary build strategy steps provided by users, this almost certainly will break things.
| ### Non-Goals | ||
|
|
||
| - Read the runAsUser and runAsGroup from the step image when they are not defined directly on the step. This can be a future extension, but for now we expect the build strategy author to specify runAsUser and runAsGroup even if they match the image configuration. |
There was a problem hiding this comment.
If we are not modifying /etc/group and /etc/passwd on build strategy steps themselves, this should be called out as a non-goal.
|
|
||
| > **Note**: A container using the downward API as a subPath volume mount does not receive updates when field values change. | ||
|
|
||
| This can be considered a current limitation in Kubernetes and could change in the future. Even when this is lifted, we will still be safe because a Kubernetes user who is allowed to update the pod, can only change the content of `/etc/passwd` but not the `runAsUser` and `runAsGroup` of the container as we will also reduce the default configuration of our supporting steps to not allow privilege escalation and to drop all capabilities (`SETGID` and `SETUID` would be required to switch the user). We actually should have configured them like this all the time already to give them no unnecessary privileges. |
There was a problem hiding this comment.
Same as above - proposal just needs to clarify that the /etc/group and /etc/passwd machinations only applies to Shipwright-provided steps.
|
I spoke with @SaschaSchwarze0 , we will be adding a new update after merging this PR, thanks @adambkaplan . /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qu1queee The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
The proposal explains the background on why we currently have a prepare step that runs as root in some sample build strategies and aims to get rid of them by running all steps (shipwright-injected and strategy steps) as the same user.