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

feat(containers): added z-flag support for bind mounts in container creation #1910

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

Trudels
Copy link

@Trudels Trudels commented May 17, 2018

Hey Anthony,

Please let me know about the status of the PR.

Greets & Have a nice day
Sebastian

@G07cha
Copy link
Contributor

G07cha commented May 20, 2018

Is there any issue already to which this PR is related to?

@deviantony
Copy link
Member

Yes there is: #853

@G07cha
Copy link
Contributor

G07cha commented May 20, 2018

Alright, so if this feature works only for SELinux maybe we can check if OS is SELinux using SystemInfo call before displaying additional UI elements(just to avoid inputs that is not going to affect anything).

@deviantony
Copy link
Member

deviantony commented May 21, 2018

@G07cha these options help working around issues when using Docker with SELinux enabled but I don't think that they are reserved to these kind of environments.

@G07cha
Copy link
Contributor

G07cha commented May 21, 2018

But is it affects anything in other environments? If it's not I don't see a point of having it for places where it doesn't affect anything.

@Trudels
Copy link
Author

Trudels commented May 21, 2018

@G07cha the :z flag enables to inherit permissions including selinux labels (most common problem occurance in combination with the z flag)
i think the answer to this post (https://stackoverflow.com/questions/34031397/running-docker-on-ubuntu-mounted-host-volume-is-not-writable-from-container) is a good explanation of the z flag, maybe the description of the switch should be more generic?

& is there any need of adding the :Z flag too?

@deviantony
Copy link
Member

deviantony commented May 21, 2018

I believe that we should support both. It's just a matter of UX, for each volume we should be able to specify rw, ro, z, or Z.

@G07cha
Copy link
Contributor

G07cha commented May 21, 2018

If it's effective in most of OS-es then it's good to me! Thanks for the explanation, @Trudels!

@Trudels
Copy link
Author

Trudels commented May 21, 2018

@deviantony actually you can create volumes with ro + z

@deviantony
Copy link
Member

@Trudels you're right, removed my comment.

From Docker docs:

-v, --volume=[host-src:]container-dest[:<options>]: Bind mount a volume.
The comma-delimited `options` are [rw|ro], [z|Z],
[[r]shared|[r]slave|[r]private], and [nocopy].
The 'host-src' is an absolute path or a name value.

I'll try to have a look at the proposed UX in this PR soon.

@Trudels
Copy link
Author

Trudels commented May 21, 2018

alright let me know if you know how i should implement it, or are you taking over? i don't mind..

Greetings

@deviantony
Copy link
Member

Feel free to propose something to support both z & Z. I'm a bit busy on other topics right now.

@Trudels
Copy link
Author

Trudels commented May 21, 2018

alright i'll think about something...

Greets

@deviantony deviantony added status/1-functional-review Indicates that the PR is currently under functional review and removed contrib/tech-review-in-progress labels Sep 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has conflicts status/1-functional-review Indicates that the PR is currently under functional review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants