containers / buildah

A tool that facilitates building OCI images.
https://buildah.io
Apache License 2.0
7.24k stars 766 forks source link

COPY does not expand ENV nor ARG variables in --chown #2192

Closed petr-motejlek closed 4 years ago

petr-motejlek commented 4 years ago

Description

I have a Dockerfile that appears to do what I expect it to without raising any errors. Buildah is unable to do the same.

It would appear that the --chown parameter for the COPY instrcution is not expanded (to its corresponding ENV or ARG value) in Buildah, but it is expanded in regular docker build (and actually also in BuildKit builds).

Steps to reproduce the issue:

FROM ubuntu:latest

ENV MYUSER=myuser

RUN useradd --create-home --home /"${MYUSER}" "${MYUSER}"
COPY --chown="${MYUSER}" /something /somewhere
touch something

buildah bud
STEP 1: FROM ubuntu:latest
Getting image source signatures
Copying blob 423ae2b273f4 done
Copying blob de83a2304fa1 done
Copying blob b6b53be908de done
Copying blob f9a83bce3af0 done
Copying config 72300a873c [======================================] 3.3KiB / 3.3KiB
Writing manifest to image destination
Storing signatures
STEP 2: ENV MYUSER=myuser
STEP 3: RUN useradd --create-home --home /"${MYUSER}" "${MYUSER}"
STEP 4: COPY --chown="${MYUSER}" /something /somewhere
error building at STEP "COPY --chown=${MYUSER} /something /somewhere": error determining run uid: user: unknown user error looking up user "${MYUSER}"
docker build .
Sending build context to Docker daemon   2.56kB
Step 1/4 : FROM ubuntu:latest
 ---> 72300a873c2c
Step 2/4 : ENV MYUSER=myuser
 ---> Running in e424a7974883
Removing intermediate container e424a7974883
 ---> bbbb6b51dc07
Step 3/4 : RUN useradd --create-home --home /"${MYUSER}" "${MYUSER}"
 ---> Running in 269e9c100f28
Removing intermediate container 269e9c100f28
 ---> 92a743cee5e2
Step 4/4 : COPY --chown="${MYUSER}" /something /somewhere
 ---> 079735036dd4
Successfully built 079735036dd4

Describe the results you received:

I received very bad results :).

Describe the results you expected:

I expected buildah to actually succeed at creating the image. I am not sure how, if at all, this is related to #1957 or #1994

Output of rpm -q buildah or apt list buildah:

buildah/unknown,now 1.13.2~1 amd64 [installed]

Output of buildah version:

buildah version 1.13.2 (image-spec 1.0.1-dev, runtime-spec 1.0.1-dev)

*Output of `cat /etc/release`:**

DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=18.04
DISTRIB_CODENAME=bionic
DISTRIB_DESCRIPTION="Ubuntu 18.04.3 LTS"
NAME="Ubuntu"
VERSION="18.04.3 LTS (Bionic Beaver)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 18.04.3 LTS"
VERSION_ID="18.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=bionic
UBUNTU_CODENAME=bionic

Output of uname -a:

Linux vagrant 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

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

(I did not touch this file)

# This file is is the configuration file for all tools
# that use the containers/storage library.
# See man 5 containers-storage.conf for more information
# The "container storage" table contains all of the server options.
[storage]

# Default Storage Driver
driver = ""

# Temporary storage location
runroot = "/var/run/containers/storage"

# Primary Read/Write location of container storage
graphroot = "/var/lib/containers/storage"

[storage.options]
# Storage options to be passed to underlying storage drivers

# AdditionalImageStores is used to pass paths to additional Read/Only image stores
# Must be comma separated list.
additionalimagestores = [
]

# Remap-UIDs/GIDs is the mapping from UIDs/GIDs as they should appear inside of
# a container, to the UIDs/GIDs as they should appear outside of the container,
# and the length of the range of UIDs/GIDs.  Additional mapped sets can be
# listed and will be heeded by libraries, but there are limits to the number of
# mappings which the kernel will allow when you later attempt to run a
# container.
#
# remap-uids = 0:1668442479:65536
# remap-gids = 0:1668442479:65536

# Remap-User/Group is a user name which can be used to look up one or more UID/GID
# ranges in the /etc/subuid or /etc/subgid file.  Mappings are set up starting
# with an in-container ID of 0 and then a host-level ID taken from the lowest
# range that matches the specified name, and using the length of that range.
# Additional ranges are then assigned, using the ranges which specify the
# lowest host-level IDs first, to the lowest not-yet-mapped in-container ID,
# until all of the entries have been used for maps.
#
# remap-user = "storage"
# remap-group = "storage"

[storage.options.overlay]
# ignore_chown_errors can be set to allow a non privileged user running with
# a single UID within a user namespace to run containers. The user can pull
# and use any image even those with multiple uids.  Note multiple UIDs will be
# squashed down to the default uid in the container.  These images will have no
# separation between the users in the container. Only supported for the overlay
# and vfs drivers.
#ignore_chown_errors = false

# Path to an helper program to use for mounting the file system instead of mounting it
# directly.
#mount_program = "/usr/bin/fuse-overlayfs"

# mountopt specifies comma separated list of extra mount options
mountopt = "nodev"

# Size is used to set a maximum size of the container image.
# size = ""

[storage.options.thinpool]
# Storage Options for thinpool

# autoextend_percent determines the amount by which pool needs to be
# grown. This is specified in terms of % of pool size. So a value of 20 means
# that when threshold is hit, pool will be grown by 20% of existing
# pool size.
# autoextend_percent = "20"

# autoextend_threshold determines the pool extension threshold in terms
# of percentage of pool size. For example, if threshold is 60, that means when
# pool is 60% full, threshold has been hit.
# autoextend_threshold = "80"

# basesize specifies the size to use when creating the base device, which
# limits the size of images and containers.
# basesize = "10G"

# blocksize specifies a custom blocksize to use for the thin pool.
# blocksize="64k"

# directlvm_device specifies a custom block storage device to use for the
# thin pool. Required if you setup devicemapper.
# directlvm_device = ""

# directlvm_device_force wipes device even if device already has a filesystem.
# directlvm_device_force = "True"

# fs specifies the filesystem type to use for the base device.
# fs="xfs"

# log_level sets the log level of devicemapper.
# 0: LogLevelSuppress 0 (Default)
# 2: LogLevelFatal
# 3: LogLevelErr
# 4: LogLevelWarn
# 5: LogLevelNotice
# 6: LogLevelInfo
# 7: LogLevelDebug
# log_level = "7"

# min_free_space specifies the min free space percent in a thin pool require for
# new device creation to succeed. Valid values are from 0% - 99%.
# Value 0% disables
# min_free_space = "10%"

# mkfsarg specifies extra mkfs arguments to be used when creating the base.
# device.
# mkfsarg = ""

# Size is used to set a maximum size of the container image.
# size = ""

# use_deferred_removal marks devicemapper block device for deferred removal.
# If the thinpool is in use when the driver attempts to remove it, the driver
# tells the kernel to remove it as soon as possible. Note this does not free
# up the disk space, use deferred deletion to fully remove the thinpool.
# use_deferred_removal = "True"

# use_deferred_deletion marks thinpool device for deferred deletion.
# If the device is busy when the driver attempts to delete it, the driver
# will attempt to delete device every 30 seconds until successful.
# If the program using the driver exits, the driver will continue attempting
# to cleanup the next time the driver is used. Deferred deletion permanently
# deletes the device and all data stored in device will be lost.
# use_deferred_deletion = "True"

# xfs_nospace_max_retries specifies the maximum number of retries XFS should
# attempt to complete IO when ENOSPC (no space) error is returned by
# underlying storage device.
# xfs_nospace_max_retries = "0"
carbonin commented 4 years ago

This works for me with the latest buildah source:

$ cat Dockerfile 
FROM ubuntu:latest

ENV MYUSER=myuser

RUN useradd --create-home --home /"${MYUSER}" "${MYUSER}"
COPY --chown="${MYUSER}" /something /somewhere
$ ../buildah bud
STEP 1: FROM ubuntu:latest
Getting image source signatures
Copying blob 78bf9a5ad49e skipped: already exists  
Copying blob 5bed26d33875 skipped: already exists  
Copying blob 930bda195c84 [--------------------------------------] 0.0b / 0.0b
Copying config 4e5021d210 done  
Writing manifest to image destination
Storing signatures
STEP 2: ENV MYUSER=myuser
STEP 3: RUN useradd --create-home --home /"${MYUSER}" "${MYUSER}"
STEP 4: COPY --chown="${MYUSER}" /something /somewhere
STEP 5: COMMIT
Getting image source signatures
Copying blob c8be1b8f4d60 skipped: already exists  
Copying blob 977183d4e999 skipped: already exists  
Copying blob 6597da2e2e52 skipped: already exists  
Copying blob 16542a8fc3be skipped: already exists  
Copying blob 75d61d4fe052 done  
Copying config d83078bfa0 done  
Writing manifest to image destination
Storing signatures
--> d83078bfa09
d83078bfa09d51ae0b2ac6e02021a47ac6ff8bc05ac85d1dc1138821574dc8f2
$ podman run --rm d83078bfa09d51ae0b2ac6e02021a47ac6ff8bc05ac85d1dc1138821574dc8f2 ls -l /somewhere
-rw-rw-r--. 1 myuser myuser 0 Mar 27 19:24 /somewhere
petr-motejlek commented 4 years ago

Maybe I am seeing a different root cause, but technically speaking, the expansion still seems broken (even when using the latest master).

FROM ubuntu:18.04
ARG USER_NAME=my_user
RUN useradd -r -s /bin/false -m -d /home/${USER_NAME} ${USER_NAME}
USER ${USER_NAME}
WORKDIR /home/${USER_NAME}
COPY --chown=${USER_NAME} ./prdel/some_file ./
$ docker run -it --rm -v "$(pwd)":"$(pwd)":ro -w "$(pwd)" --privileged --net=host --security-opt label=disable --security-opt seccomp=unconfined --device /dev/fuse:rw quay.io/buildah/upstream:master sudo buildah bud --format=docker -f prdel/Dockerfile .
STEP 1: FROM ubuntu:18.04
Getting image source signatures
Copying blob 36505266dcc6 done
Copying blob 23884877105a done
Copying blob 2910811b6c42 done
Copying blob bc38caa0f5b9 done
Copying config c3c304cb4f done
Writing manifest to image destination
Storing signatures
STEP 2: ARG USER_NAME=my_user
STEP 3: RUN useradd -r -s /bin/false -m -d /home/${USER_NAME} ${USER_NAME}
STEP 4: USER ${USER_NAME}
STEP 5: WORKDIR /home/${USER_NAME}
STEP 6: COPY --chown="${USER_NAME}" ./prdel/some_file ./
error building at STEP "COPY --chown=${USER_NAME} ./prdel/some_file ./": error determining run uid: user: unknown user error looking up user "${USER_NAME}"
Version:         1.15.0-dev
Go Version:      go1.13.6
Image Spec:      1.0.1-dev
Runtime Spec:    1.0.1-dev
CNI Spec:        0.4.0
libcni Version:  v0.7.2-0.20190904153231-83439463f784
image Version:   5.2.1
Git Commit:      002dffb8
Built:           Sat Feb 22 10:27:15 2020
OS/Arch:         linux/amd64
$ docker build -f prdel/Dockerfile .
[+] Building 3.4s (9/9) FINISHED
 => [internal] load build definition from Dockerfile                                                  0.0s
 => => transferring dockerfile: 238B                                                                  0.0s
 => [internal] load .dockerignore                                                                     0.0s
 => => transferring context: 2B                                                                       0.0s
 => [internal] load metadata for docker.io/library/ubuntu:18.04                                       2.1s
 => CACHED [1/4] FROM docker.io/library/ubuntu:18.04@sha256:3235326357dfb65f1781dbc4df3b834546d8bf91  0.0s
 => [internal] load build context                                                                     0.0s
 => => transferring context: 59B                                                                      0.0s
 => [2/4] RUN useradd -r -s /bin/false -m -d /home/my_user my_user                                    1.0s
 => [3/4] WORKDIR /home/my_user                                                                       0.1s
 => [4/4] COPY --chown=my_user ./prdel/some_file ./                                                   0.1s
 => exporting to image                                                                                0.1s
 => => exporting layers                                                                               0.1s
 => => writing image sha256:3df10933fdcda6d007a562b2b6dadae7c355d6d2f024b9e98da21647089fa852          0.0s

I even tried setting ENV USER_NAME=${USER_NAME}, in case it's only ARGs that are not expanded, alas it's not working.

petr-motejlek commented 4 years ago

Funny thing ... this actually seems to work with an older Buildah :D, so it might very well have seemed working when you originally closed this.

Version:         1.14.3
Go Version:      go1.13.6
Image Spec:      1.0.1-dev
Runtime Spec:    1.0.1-dev
CNI Spec:        0.4.0
libcni Version:
image Version:   5.2.1
Git Commit:
Built:           Thu Jan  1 00:00:00 1970
OS/Arch:         linux/amd64

The problem is, there it seems to expand the variable to an empty string...

(I've added the RUN at the end to print out the files and permissions).

STEP 1: FROM ubuntu:18.04
Getting image source signatures
Copying blob 2910811b6c42 done
Copying blob bc38caa0f5b9 done
Copying blob 36505266dcc6 done
Copying blob 23884877105a done
Copying config c3c304cb4f done
Writing manifest to image destination
Storing signatures
STEP 2: ARG USER_NAME=my_user
STEP 3: RUN useradd -r -s /bin/false -m -d /home/${USER_NAME} ${USER_NAME}
STEP 4: USER my_user
STEP 5: WORKDIR /home/${USER_NAME}
STEP 6: COPY   --chown=${USER_NAME}   ./prdel/some_file   ./
STEP 7: RUN id && ls -laR
uid=999(my_user) gid=999(my_user) groups=999(my_user)
.:
total 20
drwxr-xr-x 2 my_user my_user 4096 Apr 26 10:36 .
drwxr-xr-x 3 root    root    4096 Apr 26 10:36 ..
-rw-r--r-- 1 my_user my_user  220 Apr  4  2018 .bash_logout
-rw-r--r-- 1 my_user my_user 3771 Apr  4  2018 .bashrc
-rw-r--r-- 1 my_user my_user  807 Apr  4  2018 .profile
-rw-r--r-- 1 root    root       0 Apr 26 08:51 some_file
STEP 8: COMMIT
Getting image source signatures
Copying blob b7f7d2967507 skipped: already exists
Copying blob a6ebef4a95c3 skipped: already exists
Copying blob 838a37a24627 skipped: already exists
Copying blob 28ba7458d04b skipped: already exists
Copying blob 2f0d9a3620a0 done
Copying config f803a49ba7 done
Writing manifest to image destination
Storing signatures
--> f803a49ba7c
f803a49ba7c2679956fec19b330d714b044089ec3e869fd960535d192ea4173d

some_file should not be owned by root.

So, I've added an extra ENV USER_NAME=${USER_NAME} below the ARG, and it works, with the old version.

STEP 1: FROM ubuntu:18.04
Getting image source signatures
Copying blob 36505266dcc6 done
Copying blob 23884877105a done
Copying blob bc38caa0f5b9 done
Copying blob 2910811b6c42 done
Copying config c3c304cb4f done
Writing manifest to image destination
Storing signatures
STEP 2: ARG USER_NAME=my_user
STEP 3: ENV USER_NAME=${USER_NAME}
STEP 4: RUN useradd -r -s /bin/false -m -d /home/${USER_NAME} ${USER_NAME}
STEP 5: USER my_user
STEP 6: WORKDIR /home/${USER_NAME}
STEP 7: COPY   --chown=${USER_NAME}   ./prdel/some_file   ./
STEP 8: RUN id && ls -laR
uid=999(my_user) gid=999(my_user) groups=999(my_user)
.:
total 20
drwxr-xr-x 2 my_user my_user 4096 Apr 26 10:41 .
drwxr-xr-x 3 root    root    4096 Apr 26 10:41 ..
-rw-r--r-- 1 my_user my_user  220 Apr  4  2018 .bash_logout
-rw-r--r-- 1 my_user my_user 3771 Apr  4  2018 .bashrc
-rw-r--r-- 1 my_user my_user  807 Apr  4  2018 .profile
-rw-r--r-- 1 my_user my_user    0 Apr 26 08:51 some_file
STEP 9: COMMIT
Getting image source signatures
Copying blob b7f7d2967507 skipped: already exists
Copying blob a6ebef4a95c3 skipped: already exists
Copying blob 838a37a24627 skipped: already exists
Copying blob 28ba7458d04b skipped: already exists
Copying blob 83b4356d5f50 done
Copying config 9f8a3a10a9 done
Writing manifest to image destination
Storing signatures
--> 9f8a3a10a95
9f8a3a10a950304804e743bd5339fb229bf5666df330ac53ed6dd750addabed3
petr-motejlek commented 4 years ago

So, to sum up:

With 1.14.3, ARGs were not expanded within COPY --from=${ARG} (instead they were silently "ignored"), ENVs were expanded, though.

With 1.15.0-dev, nothing's expanded, and it always blows up, no matter whether one uses ARGs only or ENVs...

TomSweeneyRedHat commented 4 years ago

@petr-motejlek the way ARG variables in Dockerfiles are handled changed recently. In short, if the ARG statement is before the FROM statement, then it is no longer used after the FROM statement unless it is reestablished after the FROM statement. i.e.

ARG Release=31
FROM fedora:${Release}
COPY temp.txt /fedora-${Release}.txt 

The above will now end up with a file in the container named /fedora-.txt rather than /fedora-31.txt.

To make this "work", you now need to adjust the Dockerfile slightly and establish the ARG variable again after the FROM statement. You don't need to reset the ARG.

ARG Release=31
FROM fedora:${Release}
ARG Release
COPY temp.txt /fedora-${Release}.txt 

Does this fit your scenario? We also had issues prior with environment variables in a COPY --from${arg}, but I believe that was addressed recently. If you have a Dockerfile that we could examine, we could take a look.

petr-motejlek commented 4 years ago

Hey, Tom.

the way ARG variables in Dockerfiles are handled changed recently

When I started using Docker heavily about 2 years back, it had already been doing that, so that's not recent in my eyes :D.

However, how does this relate to the issue I am reporting, though? My original Dockerfile's do indeed use "this" (having a global ARG before FROM and then "pulling" that ARG into the FROM), but the reproduction scenario above has FROM first, then ARG, so the feature you describe does not seem to apply. Or, did I miss anything?

TomSweeneyRedHat commented 4 years ago

@petr-motejlek let me re-look. I could have sworn your Dockerfile had an ARG statement at the top, I think my covid19 bunkering has caused me to lose a step.

TomSweeneyRedHat commented 4 years ago

@petr-motejlek first off, thx for the re-ping. I'm also seeing the chown command not using the username as it should and I've verified that it does work on older versions and Docker for that matter. I'm not sure what the issue is, but something is off for sure.

petr-motejlek commented 4 years ago

Any chance this could actually be re-opened, as futher investigation is happening?

TomSweeneyRedHat commented 4 years ago

Yes, I'll reopen

TomSweeneyRedHat commented 4 years ago

OK, I don't have a root cause yet, but I've found the issue. The problem happens if you use the ARG command to declare your variable. If you replace ARG with ENV in your last Dockerfile example @petr-motejlek that will work, at least it did in my testing. You shouldn't need to do that, but if you could try to see if that works for you too I'd appreciate it.

We recently had a fix for values set by the ARG statement not getting into our internal list of set variables like the ones from the ENV statement did. That was fixed, but we do some special handling for COPY --chown=blah and I'm guessing we've put that fix lower down the stack than where we need it for COPY statements. I will have to dig.

TomSweeneyRedHat commented 4 years ago

OK, I think I've found the issue down in openshift/builder which we use to do the Dockerfile processing. I've got a rough fix put together, will clean it up and get it out for review tomorrow. The root of the issue is the code looks through the declared environment variables via the ENV statements, but ignores the values defined by ARG.

jeremy-ebler-vineti commented 3 years ago

What version has the fix for this?

rhatdan commented 3 years ago

I believe V1.16.0 and later, from scanning through the git log.

TomSweeneyRedHat commented 3 years ago

Actually made Buildah v1.15.0, so definiely v1.16 and beyond.