Skip to content

builder: fix COPY --from should preserve ownership #38599

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 22, 2019
Merged

builder: fix COPY --from should preserve ownership #38599

merged 1 commit into from
Mar 22, 2019

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jan 21, 2019 •
edited
Loading

When copying between stages, or copying from an image, ownership of the copied files should not be changed (unless the --chown option is set, in which case ownership of copied files should be updated to the specified user/group).

- Description for the changelog

- builder: fix `COPY --from` should preserve ownership

@thaJeztah thaJeztah requested a review from tonistiigi as a code owner January 21, 2019 00:52
@thaJeztah thaJeztah force-pushed the builder_fix_copy_permissions branch from 1c3a167 to d335b2c Compare January 21, 2019 01:12
@codecov
Copy link

codecov bot commented Jan 21, 2019 •
edited
Loading

Codecov Report

Merging #38599 into master will decrease coverage by 0.22%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #38599      +/-   ##
==========================================
- Coverage   36.47%   36.25%   -0.23%     
==========================================
  Files         613      620       +7     
  Lines       45814    48057    +2243     
==========================================
+ Hits        16709    17421     +712     
- Misses      26823    28250    +1427     
- Partials     2282     2386     +104

@thaJeztah thaJeztah force-pushed the builder_fix_copy_permissions branch 4 times, most recently from 08fc497 to de5d2d4 Compare January 21, 2019 02:08
@thaJeztah
Copy link
Member Author

ping @AkihiroSuda @tonistiigi PTAL

This was referenced Jan 21, 2019
Closed
Open
@tonistiigi tonistiigi mentioned this pull request Feb 7, 2019
Open
@thaJeztah thaJeztah mentioned this pull request Feb 12, 2019
Open
@frebib
Copy link

frebib commented Feb 15, 2019

Does this also apply to extra permission bits such as sticky or set[ug]id?

@thaJeztah thaJeztah force-pushed the builder_fix_copy_permissions branch from de5d2d4 to 5336a4c Compare February 28, 2019 16:59
@thaJeztah
Copy link
Member Author

Does this also apply to extra permission bits such as sticky or set[ug]id?

No, this should only handle permissions; I think those should work if you use BuiltKit as builder (DOCKER_BUILDKIT=1), but haven't tried.

@thaJeztah
Copy link
Member Author

Rebased to re-run CI; ping @AkihiroSuda @tonistiigi @tiborvass PTAL

@thaJeztah thaJeztah mentioned this pull request Mar 12, 2019
Closed
@thaJeztah
builder: fix COPY --from should preserve ownership
6d87f19 
When copying between stages, or copying from an image,
ownership of the copied files should not be changed, unless
the `--chown` option is set (in which case ownership of copied
files should be updated to the specified user/group).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the builder_fix_copy_permissions branch from 5336a4c to 6d87f19 Compare March 12, 2019 23:55
@thaJeztah
Copy link
Member Author

Rebased (again) to re-run CI; ping @AkihiroSuda @tonistiigi @tiborvass PTAL

@thaJeztah
Copy link
Member Author

@tonistiigi @tiborvass this LGTY?

@tiborvass tiborvass merged commit 29de017 into moby:master Mar 22, 2019
@huggla huggla mentioned this pull request Mar 22, 2019
Closed
@thaJeztah thaJeztah deleted the builder_fix_copy_permissions branch March 22, 2019 17:36
@thaJeztah thaJeztah mentioned this pull request Apr 3, 2019
Closed
@whereisaaron
Copy link

@thaJeztah does this change (partially) fix #34645 where some single-stage build and all (almost all) multi-stage builds that copy files fail if dockerd --userns-remap=default is used? This is causing drama for e.g. BitBucket users.

@thaJeztah
Copy link
Member Author

Haven't tried it myself, but given that no ownership is changed, I expect "yes"

You can download a nightly build or install a beta/release candidate for Docker 19.03 if you want to test

This was referenced Aug 8, 2019
Merged
Merged
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 17, 2020
@thaJeztah thaJeztah mentioned this pull request Apr 17, 2020
Closed
@tianon tianon mentioned this pull request Jul 9, 2020
Closed
@thaJeztah thaJeztah mentioned this pull request Nov 23, 2020
Open
@thaJeztah thaJeztah mentioned this pull request Sep 21, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiborvass tiborvass tiborvass approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@tonistiigi tonistiigi Awaiting requested review from tonistiigi tonistiigi is a code owner

Assignees

@justincormack justincormack

Labels
area/builder impact/changelog process/cherry-picked status/2-code-review
Projects
None yet
Milestone
20.10.0
Development

Successfully merging this pull request may close these issues.

Multi-stage COPY --from should preserve ownership/permissions
7 participants
@thaJeztah @frebib @whereisaaron @tiborvass @AkihiroSuda @justincormack @GordonTheTurtle