buildah icon indicating copy to clipboard operation
buildah copied to clipboard

Inconsistent behavior when COPYing multiple files to /dest without trailing slash

Open chmeliik opened this issue 3 years ago • 9 comments

According to https://docs.docker.com/engine/reference/builder/#copy,

If multiple <src> resources are specified, either directly or due to the use of a wildcard, then <dest> must be a directory, and it must end with a slash /.

For a COPY instruction like COPY spam/* /spam or COPY spam/a.txt spam/b.txt spam/c.txt /spam, docker fails the build saying the destination needs a trailing slash.

Buildah does not fail the build, which is probably fine, but the behavior differs between using a wildcard and listing them all explicitly. With a wildcard, only one item gets copied and becomes /spam. When listing explicitly, all the items get copied into /spam/.

Steps to reproduce the issue:

  1. build inputs:

Dockerfile

FROM alpine:latest

COPY spam/* /spam
# COPY spam/a.txt spam/b.txt spam/c.txt /spam

spam/a.txt

Listen -- strange women lying in ponds distributing swords is no basis for a system of government. Supreme executive power derives from a mandate from the masses, not from some farcical aquatic ceremony.

spam/b.txt

I mean, if I went around sayin' I was an emperor just because some moistened bint had lobbed a scimitar at me they'd put me away!

spam/c.txt

Ah, now we see the violence inherent in the system. Oh! Come and see the violence inherent in the system! HELP! HELP! I'm being repressed!
  1. run buildah bud -t test .
  2. run podman run --rm -ti test:latest find /spam -type f -exec echo {} + -exec cat {} +
/spam
Ah, now we see the violence inherent in the system. Oh! Come and see the violence inherent in the system! HELP! HELP! I'm being repressed!
  1. comment the first COPY, uncomment the second, try again
/spam/a.txt /spam/b.txt /spam/c.txt
Listen -- strange women lying in ponds distributing swords is no basis for a system of government. Supreme executive power derives from a mandate from the masses, not from some farcical aquatic ceremony.
I mean, if I went around sayin' I was an emperor just because some moistened bint had lobbed a scimitar at me they'd put me away!
Ah, now we see the violence inherent in the system. Oh! Come and see the violence inherent in the system! HELP! HELP! I'm being repressed!

Describe the results you received:

Inconsistent behavior between COPY with wildcard and COPY with multiple sources listed explicitly

Describe the results you expected:

Consistent behavior, if not with docker then just within buildah (wildcard copy should create a directory and copy all files to it)

Output of rpm -q buildah or apt list buildah:

buildah-1.26.2-2.fc36.x86_64

Output of buildah version:

Version:         1.26.2
Go Version:      go1.18.4
Image Spec:      1.0.2-dev
Runtime Spec:    1.0.2-dev
CNI Spec:        1.0.0
libcni Version:  v1.1.0
image Version:   5.21.1
Git Commit:
Built:           Tue Jul 19 22:20:28 2022
OS/Arch:         linux/amd64
BuildPlatform:   linux/amd64

Output of podman version if reporting a podman build issue:

Client:       Podman Engine
Version:      4.1.1
API Version:  4.1.1
Go Version:   go1.18.3
Built:        Wed Jun 22 18:17:44 2022
OS/Arch:      linux/amd64

Output of cat /etc/*release:

NAME="Fedora Linux"
VERSION="36 (Workstation Edition)"
ID=fedora
VERSION_ID=36
VERSION_CODENAME=""
PLATFORM_ID="platform:f36"
PRETTY_NAME="Fedora Linux 36 (Workstation Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:36"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f36/system-administrators-guide
/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=36
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=36
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Workstation Edition"
VARIANT_ID=workstation

Output of uname -a:

Linux fedora 5.18.11-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Jul 12 22:52:35 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of cat /etc/containers/storage.conf:

No such file

chmeliik avatar Aug 04 '22 14:08 chmeliik

Also: in some cases, rather than copying just one item, buildah fails with this error

error building at STEP "COPY velero/* /dest": error renaming "/home/acmiel/investigation/podman-copy-error/velero/*": internal error: renamed 42 items when we expected to only rename 1

The reproducer I found is with https://github.com/openshift/velero (current ref: 6d7629ce9cf1437cdf8ccabfd7d4512c90beb068), but I have no idea what specifically about it makes buildah fail rather than copy something.

cat << EOF > Dockerfile
FROM alpine:latest

COPY velero/* /dest
EOF

git clone https://github.com/openshift/velero.git

buildah bud -t test .

chmeliik avatar Aug 04 '22 15:08 chmeliik

@nalind @flouthoc PTAL

rhatdan avatar Aug 07 '22 11:08 rhatdan

I'll take a look thanks.

flouthoc avatar Aug 08 '22 10:08 flouthoc

@chmeliik I think we should not support COPY somedir/* /dir since it results in two undefined behavior

  • Assume somedir/* resolves to a single file then logically /dir must be a file cause its a single source.
  • Assume somedir/* resolves to multiple files then it automatically becomes a directory.

I think docker's rule is correct here since it asks users to explicitly specify if source is a directory or not, so I think we should enforce same rule at the buildah's implementation end of ADD.

If multiple <src> resources are specified, either directly or due to the use of a wildcard, then <dest> must be a directory, and it must end with a slash /.

PR is here: https://github.com/containers/buildah/pull/4183 but lets wait for maintainers and their take on this. cc @containers/buildah-maintainers

flouthoc avatar Aug 11 '22 05:08 flouthoc

That does make sense to me. But now that you mention wildcards that resolve to a single file, I think I've seen Dockerfiles that rely on this behavior. I can't remember the exact use case, but it was conceptually something like this:

COPY versioned-artifact-*.tar.gz /dest/artifact.tar.gz

Arguably this never should have worked in the first place, but enforcing the trailing slash could be a breaking change for this use case.

chmeliik avatar Aug 11 '22 14:08 chmeliik

On docker if it matches multiple file this fails with

Step 2/3 : COPY hello*.hello /hello
When using COPY with more than one source file, the destination must be a directory and end with a /

but works if it resolves to one file.

On buildkit this works in a completely undefined behavio,r it copies the last created file if more than one is found instead of creating a directory and copying all of them. ( Looks like buildkit bug )

flouthoc avatar Aug 11 '22 15:08 flouthoc

I think this should be checked when you are sure there are more then one file in the SOURCE, Then check the dest to make sure it is a directory. Not examining the string.

rhatdan avatar Aug 12 '22 02:08 rhatdan

That does make sense to me. But now that you mention wildcards that resolve to a single file, I think I've seen Dockerfiles that rely on this behavior. I can't remember the exact use case, but it was conceptually something like this:

COPY versioned-artifact-*.tar.gz /dest/artifact.tar.gz

Arguably this never should have worked in the first place, but enforcing the trailing slash could be a breaking change for this use case.

@chmeliik Recent commit addresses this and a test verifies this as well.

flouthoc avatar Aug 23 '22 11:08 flouthoc

A friendly reminder that this issue had no activity for 30 days.

github-actions[bot] avatar Sep 23 '22 00:09 github-actions[bot]

A friendly reminder that this issue had no activity for 30 days.

github-actions[bot] avatar Oct 24 '22 00:10 github-actions[bot]