Fix COPY --from with userns and non zero uid/gid files

[segevfiner]

- What I did
Fixed using COPY --from with userns enabled and files that have non-zero UID/GID.

FROM busybox AS foo
RUN touch x && chown 1:1 x

FROM busybox
COPY --from=foo x /

- How I did it
Modified remapIDs to map from host to container instead of container to host.

- How to verify it
Need to make sure there isn't a code path that does need this to map from container->host, as than I'm unsure how to fix this and it will obviously break that.

The described Dockerfile can be made into a test, but I was unsure on where to place it.

- Description for the changelog

Fix COPY --from with userns and non-zero UID/GID files.

Fixes #34645

[thaJeztah]

ping @estesp @tonistiigi PTAL

[codecov]

Codecov Report

Merging #38823 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #38823      +/-   ##
==========================================
- Coverage   36.49%   36.45%   -0.04%     
==========================================
  Files         613      613              
  Lines       45886    45886              
==========================================
- Hits        16746    16728      -18     
- Misses      26847    26860      +13     
- Partials     2293     2298       +5

[thaJeztah]

@segevfiner wondering; would #38599 also solve this situation?

[tonistiigi]

This doesn't seem to be the correct solution. remapIDs is used in many places, eg. on regular unpack. For example, this change breaks regular copy from build context. failed to copy files: failed to copy file: Host ID 0 cannot be mapped to a container ID

#38599 is probably a better approach for this as we don't actually need to remap files between stages.

[segevfiner]

That would be rather strange actually since this still needs to map from host to container ID in that case.

This should only break if it is used somewhere to copy files from container to host.

But if there is a better solution, than sure, merge that instead, this is just a quick solution I came up with and haven't throughly tested that it doesn't break anything, though I did run the test suite and tested that it fixes the issue.

[tonistiigi]

though I did run the test suite and tested that it fixes the issue.

I started the userns suite to show the failing ones https://jenkins.dockerproject.org/job/Docker-PRs-userns/14871/console

#38599 was merged. Could you test if it fixes all of your issues as well.

[segevfiner]

I tried it with latest master @ 29de017, and the issue still happens. Do feel free to try using the Dockerfile in the PR description and userns-remap=default to verify.

I guess that means we still need a fix here, we need to make remapIDs map from host->container or container->host depending on the direction of the copy, but I'm not sure how to figure how which direction the copy is going... Any ideas?

[whereisaaron]

The userns feature current breaks Docker builds in various ways. Both builds using official containers for e.g. python and multi-stage builds that copy files between stages get borked in the presence of userns, with the dreaded failed to copy files: failed to copy file: Container ID 166536 cannot be mapped to a host ID error. So @segevfiner a fix is still needed.

https://bitbucket.org/site/master/issues/17241/copying-files-in-multistage-docker

[andrewhsu]

I'll have a go at reproducing the original issue this PR is trying to fix. Not sure if it is still a thing that needs to be fixed after #38599 has been merged.

[andrewhsu]

I'm able to reproduce the error with docker-ce 19.03.4 legacy builder, but the issue that this PR is trying to fix does not exist with docker-ce 19.03.4 enabled with buildkit builds. I'd recommend using docker build with buildkit builds to work around this issue since that is what the docker engine is migrating towards as the default in the future.

This PR should be closed.

[tonistiigi]

Closing as this doesn't seem to be the correct solution based on #38823 (comment) . Lets continue tracking the legacy builder problem in the issue #34645. Reopen if you have an updated solution.