-
Notifications
You must be signed in to change notification settings - Fork 788
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
Use mask definitions from containers/common @rhatdan #5111
Conversation
Signed-off-by: Daniel J Walsh <[email protected]>
Signed-off-by: Daniel J Walsh <[email protected]>
@mheon PTAL |
LGTM |
"/proc/scsi", | ||
"/proc/timer_list", | ||
"/proc/timer_stats", | ||
"/sys/dev/block", |
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.
This appears to allow the rest of /sys/dev
, which we didn't before.
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.
Podman switched from /sys/dev to /sys/dev/block a while ago. So I went with the Podman default.
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.
Is it correct for build-time as well as run-time?
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 would figure. Are concerned about access to content in /sys/dev? Note this is looser then buildah had before.
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.
It being looser is what prompted my question. containers/podman#6957 masked all of /sys/dev without being more specific about what under it mattered, containers/podman#8408 narrowed the default to /sys/dev/block, and https://github.com/containers/podman/issues/12746 is asking to unmask the only part under it that we're still going to be masking.
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.
Yup, just going for consistency now.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, rhatdan 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: containers/podman#5111 failed to apply on top of branch "release-1.32":
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. |
This mimics containers#5111 but without the vendor update, which is very large on this branch. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
This mimics containers#5111 but without the vendor update, which is very large on this branch. [NO NEW TESTS NEEDED] This cannot be tested in CI as cloud providers already don't provide these interfaces in their VMs. Signed-off-by: Matt Heon <[email protected]>
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?