-
Notifications
You must be signed in to change notification settings - Fork 785
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
chroot.setupChrootBindMounts: pay more attention to flags #5083
Conversation
20bce70
to
f9cf345
Compare
Ephemeral COPR build failed. @containers/packit-build please check. |
0f7d741
to
4f42384
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.
Smoke test is failing.
4f42384
to
bbacc87
Compare
LGTM |
Guys, you really need to be reading the Special notes for your reviewer: part of the description. |
I like side-eye |
44f4f97
to
613df06
Compare
613df06
to
c697d9a
Compare
/retitle chroot.setupChrootBindMounts: pay more attention to flags |
Pay better attention to dev/nodev/exec/noexec/suid/nosuid/ro/rw flags on bind, overlay, and tmpfs mounts when any of them are specified. Stop quietly adding "nodev" when it isn't asked for. Signed-off-by: Nalin Dahyabhai <[email protected]>
c697d9a
to
2a3a956
Compare
LGTM |
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.
LGTM
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, nalind, vrothberg 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 |
/cherry-pick release-1.32 |
@nalind: only containers org members may request cherry picks. You can still do the cherry-pick manually. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cherry-pick release-1.32 |
Is the bot misbehaving? |
@vrothberg: new pull request created: #5099 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
I suspect it got tripped up because my membership in the organization was set to Private, unlike yours, which is Public. I've just toggled mine to match. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Pay better attention to nodev/noexec/nosuid/readonly flags on bind, overlay, and tmpfs mounts. Stop quietly adding "nodev" when it isn't asked for.
How to verify it
New integration test!
Which issue(s) this PR fixes:
This fixes certain failures I've been seeing while attempting to get OpenShift's builds working in unprivileged user namespaces, where the builder uses buildah as a library and specifies that secrets by bind-mounted for RUN instructions using nodev/noexec/nosuid/ro, where the builder has to be just a bit more careful because, once you untangle the user namespace ID mappings, it doesn't actually own the underlying content.
Special notes for your reviewer:
Once merged, I'm going to want to cherry pick this to the 1.32 branch for use in OpenShift.
Does this PR introduce a user-facing change?