Skip to content
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.

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

Proposal: Add --chmod flag to ADD/COPY commands (analogous to --chown) #34819

Closed
unshare opened this issue Sep 12, 2017 · 76 comments
Closed

Proposal: Add --chmod flag to ADD/COPY commands (analogous to --chown) #34819

unshare opened this issue Sep 12, 2017 · 76 comments
Labels
area/builder kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny status/accepted

Comments

@unshare
Copy link
Contributor

unshare commented Sep 12, 2017

Description
I've bumped into #34263 being merged and it hit me that --chmod would be nice in conjunction with this feature.

I frequently build Linux images from a Windows machine and I'm kind of annoyed with the 755 mode the added/copied files end up with.

ADD/COPY with --chown and --chmod together will be a readable and maintainable notation delivering precise control over permissions.

I'm aware of moby/buildkit#4242, but I don't think this issue is a duplicate (that one seems to be stale).

@thaJeztah thaJeztah added area/builder kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny labels Sep 12, 2017
@thaJeztah
Copy link
Member

/cc @tonistiigi @AkihiroSuda @duglin

@abuisine
Copy link

I got tons of Dockerfiles RUNning a chmod after a COPY or ADD, as I am not always confident in the execution rights of the files.

@thaJeztah
Copy link
Member

I'm personally ok with this; especially for the Windows case, I agree this makes a good addition. I'll bring this up in a maintainers meeting to get other opinions, but if a contributor wants to work on this, I think this would get accepted.

@thaJeztah
Copy link
Member

Discussing in the maintainers meeting, and we're okay with this proposal; we do need to design the syntax to be usable, and take into account that we probably want to have parity on Windows (both for --chown and --chmod); input / proposals for the would be welcome!

/cc @johnstep @tianon

@unshare
Copy link
Contributor Author

unshare commented Sep 29, 2017

w.r.t. Windows we can adopt Cygwin-like model:

  • owner is the one specified in --chown
  • ACL contains three entries for owner, group and everyone respectively covering rwx bits
  • SUID, SGID and sticky bits are not portable

-OR-

there can be Windows-specific syntax, e.g., we can specify security descriptor with SDDL
yes, such a notation is bulky and challenging, but it delivers precision and unambiguity
besides, people who manage file system permissions on Windows would better be aware of this syntax

@baip
Copy link

baip commented Oct 3, 2017

This would indeed be a nice feature. A use case other than for Windows is when one wants to build a image that can be run as arbitrary non-root user via the docker --user option. In this case, chmod has to be used after COPY to give group read/write permission (assuming group ID is set to 0 via chown). This unfortunately invalidates the layer and makes the resulting image bigger.

@Ahmadposten
Copy link

@thaJeztah
I implemented the proposed chmod flag: #36123
Waiting for it to be reviewed

@ulm0
Copy link

ulm0 commented Mar 14, 2018

I'm facing an issue with permissions; I'm ADDing a .tgz file from an URL to an image, which is built FROM scratch, after some inspection i found out once the file is ADDed the binary into it ends up with -rw------- permissions.

Tried to build the image with a multi-stage Dockerfile, it ADDs the .tgz file, grants +x permissions and then the binary is copied to the FROM scratch stage, for some reason it shows exec format error when running the resulting image.

@0xdevalias
Copy link

0xdevalias commented Mar 14, 2018

@ulm0 Not 100%, but that sounds familiar, and I think it could have been that the binary is not staticly compiled. And since you're running FROM scratch, the libraries it links against aren't present.

Here is a worked example from my own repo for doing it with golang: https://github.com/0xdevalias/docker-gobuster/blob/master/Dockerfile#L6

@ulm0
Copy link

ulm0 commented Mar 14, 2018

@0xdevalias the binary is built using musl, Copying the binary from a local folder works, but using ADD breaks the image, I'm using another approach, this is the Dockerfile I'm using now so i can put this in a CI pipeline

FROM alpine:edge

RUN wget --quiet https://binaries.cockroachdb.com/cockroach-v1.1.6.linux-musl-amd64.tgz -O /tmp/cockroach.tgz && \
    tar xvzf /tmp/cockroach.tgz --strip 1

FROM scratch
ENV COCKROACH_CHANNEL=official-docker
COPY ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
COPY --from=0 /cockroach /cockroach/cockroach

# This causes the binary within the tgz file to end up with 400 permissions in the /cockroach/ folder
# It throws a permission denied error when attempting to run 
# ADD https://binaries.cockroachdb.com/cockroach-v1.1.6.linux-musl-amd64.tgz /cockroach/cockroach

WORKDIR /cockroach/
EXPOSE 26257 8080
ENTRYPOINT ["/cockroach/cockroach"]

@vce-xx
Copy link

vce-xx commented Sep 16, 2018

Any update on this topic ?

@reporter123
Copy link

This being postponed pending moby/buildkit#396. Basically there's major rewrite of the build engine going on. So the current engine is on feature freeze.

@tobia
Copy link

tobia commented Nov 9, 2020

The usefulness of alpha syntax usually comes from the capital X permission, which means "executable only if the file was previously executable or if it is a directory."

This is because a recursive COPY with --chmod=644 would make all directories non-executable, meaning non-traversable; while a --chmod=755 would make all files executable. Both are inappropriate therefore unuseable 99% of the time.

A recursive COPY with --chmod=u=rwX,go=rX (capital Xes) would set directories to 755 and files to 644, unless a given file already had the executable bit in the source filesystem, in which case it would get 755.

Personally, I think both syntaxes are cumbersome. I'd rather have a --umask setting, which is traditionally used to take away permissions., or maybe a UMASK top-level setting.

For example, --umask=022 would mean "not writable by group and others." This has the benefit of leaving the issue of executable files and/or traversable directories to Docker to figure out. Accordingly, --umask=077 would take away all rights except to the owner; --umask=0 would give everyone any available permissions; and so on.

This would probably be easier to adapt to Windows permissions, because it's a simpler specification.

@thernstig
Copy link

thernstig commented Dec 22, 2020

@thaJeztah Was this missing from the release notes for version 20.10.0? See https://docs.docker.com/engine/release-notes/

@thernstig
Copy link

I wrote an issue to support non-octal notation: moby/buildkit#1951

@Kreyren
Copy link

Kreyren commented Jan 22, 2021

I am confused so is this implemented/going to me implemented or?

image

@AkihiroSuda
Copy link
Member

Alraedy Implemented in 20.10 (moby/buildkit#1492), but requires DOCKER_BUILDKIT=1

@jakerobb
Copy link

requires DOCKER_BUILDKIT=1

Is this documented with more clarity somewhere? When and where do I put that?

@Kreyren having your IDE recognize the syntax is an entirely separate issue, and the error output in your screenshot does not indicate an issue with --chmod.

@Kreyren
Copy link

Kreyren commented Jan 22, 2021

@Kreyren having your IDE recognize the syntax is an entirely separate issue @jakerobb

Ye i have no idea how to implement that in vscodium atm, but the DOCKER_BUILDKIT=1 seems to work.

noticed the different issue after i sent the comment, the issue was resolved but i was still unable to use --chmod.. without DOCKER_BUILDKIT.

@rcjsuen
Copy link
Contributor

rcjsuen commented Jan 23, 2021

@Kreyren having your IDE recognize the syntax is an entirely separate issue @jakerobb

Ye i have no idea how to implement that in vscodium atm

The next release of the Docker extension will include this fix. See microsoft/vscode-docker#2624.

@rakshitamaheshwari
Copy link

rakshitamaheshwari commented Oct 14, 2021

The usefulness of alpha syntax usually comes from the capital X permission, which means "executable only if the file was previously executable or if it is a directory."

This is because a recursive COPY with --chmod=644 would make all directories non-executable, meaning non-traversable; while a --chmod=755 would make all files executable. Both are inappropriate therefore unuseable 99% of the time.

A recursive COPY with --chmod=u=rwX,go=rX (capital Xes) would set directories to 755 and files to 644, unless a given file already had the executable bit in the source filesystem, in which case it would get 755.

Personally, I think both syntaxes are cumbersome. I'd rather have a --umask setting, which is traditionally used to take away permissions., or maybe a UMASK top-level setting.

For example, --umask=022 would mean "not writable by group and others." This has the benefit of leaving the issue of executable files and/or traversable directories to Docker to figure out. Accordingly, --umask=077 would take away all rights except to the owner; --umask=0 would give everyone any available permissions; and so on.

This would probably be easier to adapt to Windows permissions, because it's a simpler specification.

@tobia Can you specify the full command for this. Right now I am using 'COPY --chmod=u=rwX,go=rX ./my_dir . ' but this is not working. Do I need to add specific flag for copying recursively?

@adrianwix
Copy link

Where is the documentation of this feature? I have looked into the Dockerfile reference and it's not there. I also can't find some place that documents the new features added by Buildkit. https://docs.docker.com/develop/develop-images/build_enhancements/ has no mention of chmod.

@thernstig
Copy link

@tonistiigi @AkihiroSuda @duglin @thaJeztah this is not documented anywhere in https://docs.docker.com/engine/reference/builder/ which is not great. Is there a chance you have some technical writer to update the docs?

@darkvertex
Copy link

@tonistiigi @AkihiroSuda @duglin @thaJeztah this is not documented anywhere in docs.docker.com/engine/reference/builder which is not great. Is there a chance you have some technical writer to update the docs?

I'm not their official technical writer but I've made a PR since their docs are all opensourced:
docker/cli#3560

@slankka
Copy link

slankka commented Mar 8, 2024

I supposed that docker version 20.10.0 will be the first to support ADD/COPY with --chown --chmod.

the release note did NOT mention that.

@thaJeztah
Copy link
Member

@slankka It's a Dockerfile "front-end" (parser) change, so it even works on 19.03 with BuildKit enabled, as long as your Dockerfile start with the (recommended) syntax directive;

Use the syntax parser directive to declare the Dockerfile syntax version to use for the build. If unspecified, BuildKit uses a bundled version of the Dockerfile frontend. Declaring a syntax version lets you automatically use the latest Dockerfile version without having to upgrade BuildKit or Docker Engine, or even use a custom Dockerfile implementation.

Most users will want to set this parser directive to docker/dockerfile:1, which causes BuildKit to pull the latest stable version of the Dockerfile syntax before the build.

Using the following Dockerfile;

# syntax=docker/dockerfile:1

FROM alpine
COPY --chown=123:456 file.txt .
RUN ls -ln file.txt

On Docker 19.03;

Client: Docker Engine - Community
 Version:           19.03.15
 API version:       1.40
 Go version:        go1.13.15
 Git commit:        99e3ed8
 Built:             Sat Jan 30 03:14:33 2021
 OS/Arch:           linux/arm64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          19.03.15
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       99e3ed8
  Built:            Sat Jan 30 03:20:21 2021
  OS/Arch:          linux/arm64
  Experimental:     false
 containerd:
  Version:          v1.3.9
  GitCommit:        ea765aba0d05254012b0b9e595e995c09186427f
 runc:
  Version:          1.0.0-rc10
  GitCommit:        dc9208a3303feef5b3839f4323d9beb36df0a9dd
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683
DOCKER_BUILDKIT=1 docker build --progress=plain .
#1 [internal] load .dockerignore
#1 transferring context: 2B done
#1 DONE 0.0s

#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 132B done
#2 DONE 0.0s

#3 resolve image config for docker.io/docker/dockerfile:1
#3 DONE 0.8s

#4 docker-image://docker.io/docker/dockerfile:1@sha256:ac85f380a63b13dfcefa...
#4 CACHED

#5 [internal] load build definition from Dockerfile
#5 transferring dockerfile: 132B done
#5 DONE 0.0s

#6 [internal] load metadata for docker.io/library/alpine:latest
#6 DONE 0.4s

#7 [internal] load .dockerignore
#7 DONE 0.0s

#8 [1/3] FROM docker.io/library/alpine:latest@sha256:c5b1261d6d3e4307162693...
#8 DONE 0.0s

#9 [internal] load build context
#9 transferring context: 29B done
#9 DONE 0.0s

#10 [2/3] COPY --chown=123:456 file.txt .
#10 CACHED

#11 [3/3] RUN ls -ln file.txt
#11 0.111 -rw-r--r--    1 123      456              6 Mar  8 08:22 file.txt
#11 DONE 0.1s

#12 exporting to image
#12 exporting layers done
#12 writing image sha256:efe4caba1ac46355a3dae5120e0b0d0297544a2384801bb7a25205ae0e67517b done
#12 DONE 0.0s

If you'd be running the build on a version of docker / BuildKit that doesn't support it, it will still use the latest version of the Dockerfile front-end (which may contain bug fixes), but when using a feature that cannot be supported, it will show an error indicating that the version of BuildKit that's used is too old.

For example on docker 18.09;

DOCKER_BUILDKIT=1 docker build --progress=plain .

#2 [internal] load build definition from Dockerfile
#2       digest: sha256:030a9e782cf78b3da87d5bc590cef50396ed87f1881b903d30f8ecaa22a1dc23
#2         name: "[internal] load build definition from Dockerfile"
#2      started: 2024-03-08 08:31:35.944838385 +0000 UTC
#2    completed: 2024-03-08 08:31:35.955701968 +0000 UTC
#2     duration: 10.863583ms
#2 transferring dockerfile: 132B done

....
....

rpc error: code = Unknown desc = needs BuildKit 0.5 or later: requested prerelease feature file.base  is not supported by build server, please update

However, note that Docker 20.10 reached EOL, so I would not recommend running it.

@Clovel
Copy link

Clovel commented Aug 22, 2024

The usefulness of alpha syntax usually comes from the capital X permission, which means "executable only if the file was previously executable or if it is a directory."

This is because a recursive COPY with --chmod=644 would make all directories non-executable, meaning non-traversable; while a --chmod=755 would make all files executable. Both are inappropriate therefore unuseable 99% of the time.

A recursive COPY with --chmod=u=rwX,go=rX (capital Xes) would set directories to 755 and files to 644, unless a given file already had the executable bit in the source filesystem, in which case it would get 755.

The capital X is the reason I would like symbolic permissions to be supported. Aslo the g=u syntax.

@thaJeztah
Copy link
Member

@Clovel its better to open a ticket / feature request in the BuildKit repository, as that's where the Dockerfile syntax and features are maintained; https://github.com/moby/buildkit

@Clovel
Copy link

Clovel commented Aug 22, 2024

@Clovel its better to open a ticket / feature request in the BuildKit repository, as that's where the Dockerfile syntax and features are maintained; https://github.com/moby/buildkit

I already have, over at moby/buildkit#1951.

I was just commenting in case someone would stumble accross this thread !

@skhhss

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/builder kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny status/accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.