dockerfile: add --chmod support for COPY/ADD command #1492

Chenbinx · 2020-05-15T02:49:30Z

I found that llb.CopyInfo already support mode option, but I'm not sure why there is no support for --chmod in COPY/ADD command. So I made little change to support it.

By the way, --chmod is really useful for us to make a smaller docker image in a easy way.

Signed-off-by: Chen Bin chen.bin11@zte.com.cn

tonistiigi

Please add integration tests for this https://github.com/moby/buildkit/blob/master/frontend/dockerfile/dockerfile_test.go

@AkihiroSuda @tiborvass @thaJeztah Do we take this directly to stable channel or should it go through experimental?

tonistiigi · 2020-05-15T04:43:51Z

frontend/dockerfile/dockerfile2llb/convert.go

 	if useFileOp(opt.buildArgValues, opt.llbCaps) {
-		return dispatchCopyFileOp(d, c, sourceState, isAddCommand, cmdToPrint, chown, opt)
+		return dispatchCopyFileOp(d, c, sourceState, isAddCommand, cmdToPrint, chown, chmod, opt)


As this is only supported in fileop it should error if it is not supported in the llbcaps.

Ok, I'll add the it test

AkihiroSuda · 2020-05-17T11:47:51Z

Do we take this directly to stable channel or should it go through experimental?

Seems safe for stable channel.
Aside from this one, I think we are ready to move --mount=type=(bind|cache|secret|ssh) out of experimental.

FernandoMiguel · 2020-05-17T11:54:36Z

Aside from this one, I think we are ready to move --mount=type=(bind|cache|secret|ssh) out of experimental.

tonistiigi

Something wrong with the commits. Merge commits are not allowed in PRs

tonistiigi · 2020-06-01T19:20:41Z

frontend/dockerfile/dockerfile2llb/convert.go

+		return dispatchCopyFileOp(d, c, sourceState, isAddCommand, cmdToPrint, chown, chmod, loc, opt)
+	}
+
+	if chmod != "" {


if caps != nil && caps.Supports(pb.CapFileBase) != nil { return errorss.Wrap(caps.Supports(pb.CapFileBase), "chmod is not supported") }

tonistiigi

^

tonistiigi · 2020-06-10T04:01:28Z

@heychenbin ping

Chenbinx · 2020-06-10T06:09:09Z

@heychenbin ping

Sorry, I did not see the notify message, I'll amend it.

Because of the network problem, I can not run integration test right now. I will test it and re-push it when I come back home. Sorry for replying so late.

Signed-off-by: Chenbin <chen.bin11@zte.com.cn>

thaJeztah · 2020-06-10T15:31:26Z

Perhaps it would be good to have this in experimental first 🤔. I recall there was some discussion on the moby issue (moby/moby#34819) related to "how will we support this for Windows images". I understand BuildKit doesn't have Windows support, so not a real issue (yet); but having some plan on how to evolve in that direction may be good (where "plan" could also mean: won't be supported)

Another thing to look at is (and we should look at that as well for the existing ADD --chown) is if the permissions are set before or after decompressing archives. The current ADD --chown does set the permissions on the archive (before decompressing), which could be considered a "bug" or a "by design" (TBD)

tonistiigi

I'm ok with this being in the stable channel. Don't see how we could remove it.

PTAL @AkihiroSuda @hinshun @tiborvass

hinshun · 2020-06-11T16:43:07Z

frontend/dockerfile/dockerfile2llb/convert.go

+		if opt.llbCaps != nil && opt.llbCaps.Supports(pb.CapFileBase) != nil {
+			return errors.Wrap(opt.llbCaps.Supports(pb.CapFileBase), "chmod is not supported")
+		}
+		return errors.New("chmod is not supported")


If chmod != "" it always errors? I think this part needs to be removed or refactored.

@hinshun only on old buildkit instances that don't have fileop support. Otherwise, it is an early return on line 836.

Oh I see, so if BuildKit doesn't have fileop and chmod is specified, it should still return error... a bit confusing. My bad.

Nuc1eoN · 2020-07-02T18:07:26Z

Another thing to look at is (and we should look at that as well for the existing ADD --chown) is if the permissions are set before or after decompressing archives. The current ADD --chown does set the permissions on the archive (before decompressing), which could be considered a "bug" or a "by design" (TBD)

I agree that this would be a bad design. Has it been addressed at all?

tonistiigi · 2020-07-02T18:25:49Z

The current ADD --chown does set the permissions on the archive (before decompressing),

This is not quite correct. The real behavior is that chown/chmod only works for file inputs and not archive inputs. This somewhat makes sense because archives already contain uid info per file(unlike context files that are normalized to root). It's also completely separate code path and historically these flags only existed on COPY that doesn't support archives.

We can just make it clear in docs that this is the expected behavior.

I don't see a reason to not allow a behavior change in here as well. It's not very likely that people today are already using the flags on archive inputs but expect the current behavior (although there is a case with multiple inputs). But atm, for the --chmod flag we should just keep the behavior consistent with --chown. If someone wants to look into supporting archives, I think this would be a LLB level change.

moi90 · 2020-12-02T14:10:06Z

Great addition! Which is the first version that includes this?

thernstig · 2021-01-11T12:53:04Z

@tonistiigi Do you know in which release this was included? I could not find it in the release notes of 2.10.0

tonistiigi · 2021-01-11T17:31:21Z

@thernstig It is in 20.10 (cc @tiborvass )

thernstig · 2021-01-11T20:23:42Z

Thanks @tonistiigi - hope the release notes can get an update. It's quite a great feature.

thernstig · 2021-01-13T08:31:34Z

@Chenbinx @tonistiigi I tried this, but it seems this does not work :

COPY --chmod=g=u --from=builder /server/ /server/

g=u is a way to chmod so that the group the same permissions as the user like so: chmod -R g=u dir

Is there no way to achieve this with the new --chmod? Or would it be a new feature request? Or a mistake in the implementation? Or is my syntax possibly incorrect with the double =?

rcjsuen · 2021-01-20T23:47:12Z

hope the release notes can get an update. It's quite a great feature.

@thernstig I opened docker/cli#2941 to get this documented.

- add support for the --chmod flag for `ADD` - add support for the --chmod flag for `COPY` - add tests for the --chmod flag Buildkit supports setting a new mode on a file during `COPY`/`ADD` instructions with the --chmod flag. See: moby/buildkit#1492 fixes: hadolint/hadolint#609

- add support for the --chmod flag for `ADD` - add support for the --chmod flag for `COPY` - add tests for the --chmod flag - add tests for pretty printer Buildkit supports setting a new mode on a file during `COPY`/`ADD` instructions with the --chmod flag. See: moby/buildkit#1492 fixes: hadolint/hadolint#609

…..." Added in moby/buildkit#1492

tonistiigi reviewed May 15, 2020

View reviewed changes

Chenbinx force-pushed the chmod_dev branch 3 times, most recently from 3953cc1 to b45e7ca Compare May 17, 2020 11:25

tonistiigi reviewed Jun 1, 2020

View reviewed changes

tonistiigi requested changes Jun 3, 2020

View reviewed changes

Chenbinx force-pushed the chmod_dev branch 2 times, most recently from 05067c4 to 9547c88 Compare June 10, 2020 07:04

add chmod in COPY and ADD command

b7fd382

Signed-off-by: Chenbin <chen.bin11@zte.com.cn>

Chenbinx force-pushed the chmod_dev branch from 9547c88 to b7fd382 Compare June 10, 2020 13:39

Chenbinx requested a review from tonistiigi June 10, 2020 13:59

tonistiigi approved these changes Jun 11, 2020

View reviewed changes

hinshun requested changes Jun 11, 2020

View reviewed changes

hinshun approved these changes Jun 11, 2020

View reviewed changes

AkihiroSuda approved these changes Jun 12, 2020

View reviewed changes

tiborvass approved these changes Jun 13, 2020

View reviewed changes

tiborvass merged commit 929fff0 into moby:master Jun 13, 2020

This was referenced Jun 14, 2020

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

Closed

Add chmod flag moby/moby#37361

Closed

Adds a --chmod flag to ADD and COPY moby/moby#36123

Closed

tonistiigi mentioned this pull request Jul 2, 2020

dockerfile: experimental ADD/COPY --chmod=... #1080

Closed

alex1989hu mentioned this pull request Jul 22, 2020

Use distroless static images metallb/metallb#677

Closed

avsm mentioned this pull request Jul 26, 2020

CI check suggestion: dune-project and opam files are in sync ocurrent/ocaml-ci#220

Closed

thaJeztah mentioned this pull request Oct 8, 2020

Shell out to local SSH client as alternative to a paramiko connection docker/docker-py#2680

Merged

stalkerg mentioned this pull request Oct 20, 2020

COPY with excluded files is not possible moby/moby#15771

Open

mthalman mentioned this pull request Dec 12, 2020

Add support for the chmod flag mthalman/DockerfileModel#80

Merged

thernstig mentioned this pull request Jan 13, 2021

--chmod support for COPY and ADD microsoft/vscode-docker#2624

Closed

rcjsuen mentioned this pull request Jan 20, 2021

Add documentation of --chmod flag in ADD and COPY instructions to builder.md docker/cli#2941

Closed

thernstig mentioned this pull request Jan 21, 2021

dockerfile: Support --chmod with non-octal notation, such as --chmod=g=u #1951

Open

Smolevich mentioned this pull request Jan 24, 2021

Add small improvements radio-t/radio-t-site#200

Closed

seanyoung mentioned this pull request Feb 3, 2021

Dockerfile ADD and COPY support --chmod=755 flag containers/buildah#2961

Closed

enys mentioned this pull request Apr 20, 2021

invalid flag --chmod, with Copy command hadolint/hadolint#609

Closed

3 tasks

m-ildefons mentioned this pull request Apr 20, 2021

syntax: add support for --chmod flags #609 hadolint/language-docker#61

Merged

thaJeztah mentioned this pull request May 18, 2021

ADD --chmod with remote source does not set file mode #2114

Closed

jpculp mentioned this pull request Aug 4, 2021

Remove RUN chmod +x lines from host container Dockerfiles bottlerocket-os/bottlerocket-admin-container#44

Closed

alanivey added a commit to baosystems/docker-dhis2 that referenced this pull request Jan 7, 2022

refactor(dockerfile): "ADD --chmod ..." replaces separate "RUN chmod …

96f121d

…..." Added in moby/buildkit#1492

oskarkook mentioned this pull request Apr 2, 2022

Use ADD --chmod=755 flatpak/flatpak-github-actions#76

Merged

LeoColomb mentioned this pull request Apr 25, 2022

Update YOURLS docker-library/official-images#12298

Merged

sam-thibault mentioned this pull request Mar 16, 2023

Add documentation of --chmod flag in ADD and COPY instructions to builder.md #3721

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dockerfile: add --chmod support for COPY/ADD command #1492

dockerfile: add --chmod support for COPY/ADD command #1492

Chenbinx commented May 15, 2020

tonistiigi left a comment

tonistiigi May 15, 2020

Chenbinx May 15, 2020

AkihiroSuda commented May 17, 2020

FernandoMiguel commented May 17, 2020

tonistiigi left a comment

tonistiigi Jun 1, 2020

tonistiigi left a comment

tonistiigi commented Jun 10, 2020

Chenbinx commented Jun 10, 2020 •

edited

thaJeztah commented Jun 10, 2020

tonistiigi left a comment

hinshun Jun 11, 2020

tonistiigi Jun 11, 2020

hinshun Jun 11, 2020

Nuc1eoN commented Jul 2, 2020

tonistiigi commented Jul 2, 2020

moi90 commented Dec 2, 2020

thernstig commented Jan 11, 2021

tonistiigi commented Jan 11, 2021

thernstig commented Jan 11, 2021

thernstig commented Jan 13, 2021

rcjsuen commented Jan 20, 2021

dockerfile: add --chmod support for COPY/ADD command #1492

dockerfile: add --chmod support for COPY/ADD command #1492

Conversation

Chenbinx commented May 15, 2020

tonistiigi left a comment

Choose a reason for hiding this comment

tonistiigi May 15, 2020

Choose a reason for hiding this comment

Chenbinx May 15, 2020

Choose a reason for hiding this comment

AkihiroSuda commented May 17, 2020

FernandoMiguel commented May 17, 2020

tonistiigi left a comment

Choose a reason for hiding this comment

tonistiigi Jun 1, 2020

Choose a reason for hiding this comment

tonistiigi left a comment

Choose a reason for hiding this comment

tonistiigi commented Jun 10, 2020

Chenbinx commented Jun 10, 2020 • edited

thaJeztah commented Jun 10, 2020

tonistiigi left a comment

Choose a reason for hiding this comment

hinshun Jun 11, 2020

Choose a reason for hiding this comment

tonistiigi Jun 11, 2020

Choose a reason for hiding this comment

hinshun Jun 11, 2020

Choose a reason for hiding this comment

Nuc1eoN commented Jul 2, 2020

tonistiigi commented Jul 2, 2020

moi90 commented Dec 2, 2020

thernstig commented Jan 11, 2021

tonistiigi commented Jan 11, 2021

thernstig commented Jan 11, 2021

thernstig commented Jan 13, 2021

rcjsuen commented Jan 20, 2021

Chenbinx commented Jun 10, 2020 •

edited