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

docs: Pod Security Context: Volumes aren't required in >= v3.3 and runAsNonRoot #14072

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

Conversation

paulcwatts
Copy link

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.

@paulcwatts paulcwatts force-pushed the pod-security-doc-fix branch from f9582e3 to 08d7df9 Compare January 10, 2025 20:40
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]>
@paulcwatts paulcwatts force-pushed the pod-security-doc-fix branch from 08d7df9 to e8e5d1b Compare January 10, 2025 20:43
@blkperl blkperl added the area/docs Incorrect, missing, or mistakes in docs label Jan 11, 2025
Copy link
Contributor

@MasonM MasonM left a 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

docs/workflow-pod-security-context.md Outdated Show resolved Hide resolved
Code review feedback

Co-authored-by: Mason Malone <[email protected]>
Signed-off-by: Paul Watts <[email protected]>
Comment on lines +27 to +30
!!! 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`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
!!! 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

Copy link
Member

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?

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Incorrect, missing, or mistakes in docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants