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

Only chown network files within container metadata #34224

Merged
merged 1 commit into from Nov 2, 2017

Conversation

Projects
None yet
5 participants
@estesp
Contributor

estesp commented Jul 23, 2017

If the user specifies a mountpath from the host, we should not be
attempting to chown files outside the daemon's metadata directory
(represented by daemon.repository at init time).

This forces users who want to use user namespaces to handle the
ownership needs of any external files mounted as network files
(/etc/resolv.conf, /etc/hosts, /etc/hostname) separately from the
daemon. In all other volume/bind mount situations we have taken this
same line--we don't chown host filesystem content.

Docker-DCO-1.1-Signed-off-by: Phil Estes estesp@linux.vnet.ibm.com

@thaJeztah my only concern here is change in behavior if anyone has relied on mounting network files from a host system location and gotten this automatic "chown" behavior. Something we have to consider I guess.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Oct 26, 2017

Member

@estesp discussing with @vieux; and this LGTM; can you add a small test?

Member

thaJeztah commented Oct 26, 2017

@estesp discussing with @vieux; and this LGTM; can you add a small test?

@vieux vieux assigned vieux and unassigned aaronlehmann Oct 26, 2017

@thaJeztah thaJeztah removed this from backlog in maintainers-session Oct 26, 2017

@estesp estesp requested review from dnephin and vdemeester as code owners Oct 31, 2017

@estesp

This comment has been minimized.

Show comment
Hide comment
@estesp

estesp Oct 31, 2017

Contributor

Thanks @thaJeztah; test added!

Contributor

estesp commented Oct 31, 2017

Thanks @thaJeztah; test added!

Only chown network files within container metadata
If the user specifies a mountpath from the host, we should not be
attempting to chown files outside the daemon's metadata directory
(represented by `daemon.repository` at init time).

This forces users who want to use user namespaces to handle the
ownership needs of any external files mounted as network files
(/etc/resolv.conf, /etc/hosts, /etc/hostname) separately from the
daemon. In all other volume/bind mount situations we have taken this
same line--we don't chown host file content.

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
@vieux

vieux approved these changes Nov 2, 2017

LGTM

@vieux vieux merged commit 462d791 into moby:master Nov 2, 2017

7 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 37630 has succeeded
Details
janky Jenkins build Docker-PRs 46330 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 6739 has succeeded
Details
userns Jenkins build Docker-PRs-userns 14870 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 17900 has succeeded
Details
z Jenkins build Docker-PRs-s390x 6546 has succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment