-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix #46 Support for volumes with 3 parts #47
Conversation
Signed-off-by: Thibault Guittet <[email protected]>
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.
Assuming that the three-token case works like the two-token case, this seems fine.
Bump, would love to be able to run this locally for testing :) |
Hi Thibault, I think this PR is waiting for someone to write some tests for it. I don't think we were going to ask you to do it, but none of us have gotten around to it, yet, either...so, this PR is languishing. Sorry! |
Adding `:Z` to the volume definition seems to work for both SELinux and non-SELinux systems. It turns out this allows to simplify the test. On Fedora (SELinux: Enforcing): $ go test -run TestSimpleVolume PASS ok go.flow.arcalot.io/podmandeployer 0.404s On Ubuntu (Apparmor disabled): $ go test -run TestSimpleVolume PASS ok go.flow.arcalot.io/podmandeployer 1.346s
@webbnh I have no issues on testing when adding |
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.
@rh-tguittet, thanks for the update!
However, we should retain the old test (without the :Z
) and add a new test for that case.
Also, are there other cases we should test (like :z
, :ro
, and/or multiple options, like :z,ro,noexec
)?
I'll take care of these if you want. (I'm currently doing other work in that file.)
#52 now supersedes this PR -- I've rebased this branch on the end of |
Changes introduced with this PR
Fix for issue #46, this is needed for selinux enabled distros.
This enables support for 2 and 3 parts volumes:
./local:/in-container
./local:/in-container:z
By contributing to this repository, I agree to the contribution guidelines.