Inconsistent behavior when COPYing multiple files to /dest without trailing slash #4167
Comments
|
Also: in some cases, rather than copying just one item, buildah fails with this error 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 . |
|
I'll take a look thanks. |
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * 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 /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <[email protected]>
|
@chmeliik I think we should not support
I think PR is here: #4183 but lets wait for maintainers and their take on this. |
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * 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 /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <[email protected]>
|
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: Arguably this never should have worked in the first place, but enforcing the trailing slash could be a breaking change for this use case. |
|
On docker if it matches multiple file this fails with 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 ) |
|
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. |
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * 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 /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <[email protected]>
@chmeliik Recent commit addresses this and a test verifies this as well. |
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * 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 /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <[email protected]>
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * 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 /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <[email protected]>
|
A friendly reminder that this issue had no activity for 30 days. |
|
A friendly reminder that this issue had no activity for 30 days. |
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * 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 /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <[email protected]>
When using `ADD` or `COPY` enforce the condition that destination must look like a directory by making sure that destination ends with a slash `/`. This ensures that we don't hit undefined behaviour for example with the use case `COPY some/* /oncontainer` it is not clear that `oncontainer` will be a directory or a file, users expect it to be a directory if `*` wildcard resolves to multiple files and logically if wildcard resolves to single file then it should be a directory, since this condition is undefined and not in parity with docker so lets drop support for it. Docker's defined rule: * 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 /. Reference: https://docs.docker.com/engine/reference/builder/#add Closes: containers#4167 Signed-off-by: Aditya R <[email protected]>
According to https://docs.docker.com/engine/reference/builder/#copy,
For a COPY instruction like
COPY spam/* /spamorCOPY 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:
Dockerfile
spam/a.txt
spam/b.txt
spam/c.txt
buildah bud -t test .podman run --rm -ti test:latest find /spam -type f -exec echo {} + -exec cat {} +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 buildahorapt list buildah:Output of
buildah version:Output of
podman versionif reporting apodman buildissue:Output of
cat /etc/*release:Output of
uname -a:Output of
cat /etc/containers/storage.conf:No such file
The text was updated successfully, but these errors were encountered: