-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
docs: Pod Security Context: Volumes aren't required in >= v3.3 and runAsNonRoot #14072
base: main
Are you sure you want to change the base?
Conversation
f9582e3
to
08d7df9
Compare
I'm hoping this will save other developers a couple of hours of going down an unnecessary path. Signed-off-by: Paul Watts <[email protected]>
08d7df9
to
e8e5d1b
Compare
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.
Thanks for your contribution! One small comment, but otherwise LGTM
Code review feedback Co-authored-by: Mason Malone <[email protected]> Signed-off-by: Paul Watts <[email protected]>
!!! Note "You must use volumes for output artifacts (v3.3 or earlier)" | ||
If you use `runAsNonRoot` in versions v3.3 or earlier, you cannot have output artifacts on base layer (e.g. `/tmp`). You must use a volume (e.g. [empty dir](empty-dir.md)). | ||
In versions later than v3.3, the [Emissary executor](workflow-executors.md#emissary-emissary) | ||
allows artifacts on the base layer with `runAsNonRoot`. |
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.
!!! Note "You must use volumes for output artifacts (v3.3 or earlier)" | |
If you use `runAsNonRoot` in versions v3.3 or earlier, you cannot have output artifacts on base layer (e.g. `/tmp`). You must use a volume (e.g. [empty dir](empty-dir.md)). | |
In versions later than v3.3, the [Emissary executor](workflow-executors.md#emissary-emissary) | |
allows artifacts on the base layer with `runAsNonRoot`. | |
!!! Note "You must use volumes for output artifacts (v3.3 or earlier with non-emissary executor)" | |
If you use `runAsNonRoot` in versions v3.3 or earlier with non-emissary executor, you cannot have output artifacts on base layer (e.g. `/tmp`). You must use a volume (e.g. [empty dir](empty-dir.md)). | |
The [Emissary executor](workflow-executors.md#emissary-emissary) (available from v3.0, default since v3.3) | |
allows artifacts on the base layer with `runAsNonRoot`. |
Let's be more precise with the wording.
Otherwise, we could remove this line since <= v3.4 is no longer maintained
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.
For all supported versions you don't need volumes it seems, so the whole note could just be removed?
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.
When I originally submitted the PR, I didn't know the docs were versioned or that 3.3 was no longer supported.
With that knowledge, if it were my project, I would also lean toward removing the note completely.
The purpose for submitting the PR in the first place was hopefully to save people time from going down an wrong path.
Motivation
A note regarding volumes being required for
runAsNonRoot
took me down an incorrect path before I saw that was unnecessary in newer versions. I'm hoping this will save other developers a couple of hours of going down the same path.Modifications
Updated the note to qualify that it is only applicable for versions v3.3 or earlier, before the Emissary executor.