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

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

Merged
merged 1 commit into from
Jun 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"math"
"net/url"
"os"
"path"
"path/filepath"
"sort"
Expand Down Expand Up @@ -493,7 +494,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
case *instructions.WorkdirCommand:
err = dispatchWorkdir(d, c, true, &opt)
case *instructions.AddCommand:
err = dispatchCopy(d, c.SourcesAndDest, opt.buildContext, true, c, c.Chown, c.Location(), opt)
err = dispatchCopy(d, c.SourcesAndDest, opt.buildContext, true, c, c.Chown, c.Chmod, c.Location(), opt)
if err == nil {
for _, src := range c.Sources() {
if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
Expand Down Expand Up @@ -528,7 +529,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
if len(cmd.sources) != 0 {
l = cmd.sources[0].state
}
err = dispatchCopy(d, c.SourcesAndDest, l, false, c, c.Chown, c.Location(), opt)
err = dispatchCopy(d, c.SourcesAndDest, l, false, c, c.Chown, c.Chmod, c.Location(), opt)
if err == nil && len(cmd.sources) == 0 {
for _, src := range c.Sources() {
d.ctxPaths[path.Join("/", filepath.ToSlash(src))] = struct{}{}
Expand Down Expand Up @@ -722,7 +723,7 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo
return nil
}

func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceState llb.State, isAddCommand bool, cmdToPrint fmt.Stringer, chown string, loc []parser.Range, opt dispatchOpt) error {
func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceState llb.State, isAddCommand bool, cmdToPrint fmt.Stringer, chown string, chmod string, loc []parser.Range, opt dispatchOpt) error {
pp, err := pathRelativeToWorkingDir(d.state, c.Dest())
if err != nil {
return err
Expand All @@ -738,6 +739,15 @@ func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceS
copyOpt = append(copyOpt, llb.WithUser(chown))
}

var mode *os.FileMode
if chmod != "" {
p, err := strconv.ParseUint(chmod, 8, 32)
if err == nil {
perm := os.FileMode(p)
mode = &perm
}
}

commitMessage := bytes.NewBufferString("")
if isAddCommand {
commitMessage.WriteString("ADD")
Expand Down Expand Up @@ -780,6 +790,7 @@ func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceS
}
} else {
opts := append([]llb.CopyOption{&llb.CopyInfo{
Mode: mode,
FollowSymlinks: true,
CopyDirContentsOnly: true,
AttemptUnpack: isAddCommand,
Expand Down Expand Up @@ -820,9 +831,16 @@ func dispatchCopyFileOp(d *dispatchState, c instructions.SourcesAndDest, sourceS
return commitToHistory(&d.image, commitMessage.String(), true, &d.state)
}

func dispatchCopy(d *dispatchState, c instructions.SourcesAndDest, sourceState llb.State, isAddCommand bool, cmdToPrint fmt.Stringer, chown string, loc []parser.Range, opt dispatchOpt) error {
func dispatchCopy(d *dispatchState, c instructions.SourcesAndDest, sourceState llb.State, isAddCommand bool, cmdToPrint fmt.Stringer, chown string, chmod string, loc []parser.Range, opt dispatchOpt) error {
if useFileOp(opt.buildArgValues, opt.llbCaps) {
return dispatchCopyFileOp(d, c, sourceState, isAddCommand, cmdToPrint, chown, loc, opt)
return dispatchCopyFileOp(d, c, sourceState, isAddCommand, cmdToPrint, chown, chmod, loc, opt)
}

if chmod != "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

img := llb.Image(opt.copyImage, llb.MarkImageInternal, llb.Platform(opt.buildPlatforms[0]), WithInternalName("helper image for file operations"))
Expand Down
72 changes: 72 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ var fileOpTests = []integration.Test{
testNoSnapshotLeak,
testCopySymlinks,
testCopyChown,
testCopyChmod,
testCopyOverrideFiles,
testCopyVarSubstitution,
testCopyWildcards,
Expand Down Expand Up @@ -2914,6 +2915,77 @@ COPY --from=base /out /
require.Equal(t, "1000 nogroup\n", string(dt))
}

func testCopyChmod(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)
isFileOp := getFileOp(t, sb)

dockerfile := []byte(`
FROM busybox AS base

RUN mkdir -m 0777 /out
COPY --chmod=0644 foo /
COPY --chmod=777 bar /baz
COPY --chmod=0 foo /foobis

RUN stat -c "%04a" /foo > /out/fooperm
RUN stat -c "%04a" /baz > /out/barperm
RUN stat -c "%04a" /foobis > /out/foobisperm
FROM scratch
COPY --from=base /out /
`)

dir, err := tmpdir(
fstest.CreateFile("Dockerfile", dockerfile, 0600),
fstest.CreateFile("foo", []byte(`foo-contents`), 0600),
fstest.CreateFile("bar", []byte(`bar-contents`), 0700),
)

require.NoError(t, err)
defer os.RemoveAll(dir)

c, err := client.New(context.TODO(), sb.Address())
require.NoError(t, err)
defer c.Close()

destDir, err := ioutil.TempDir("", "buildkit")
require.NoError(t, err)
defer os.RemoveAll(destDir)

_, err = f.Solve(context.TODO(), c, client.SolveOpt{
Exports: []client.ExportEntry{
{
Type: client.ExporterLocal,
OutputDir: destDir,
},
},
FrontendAttrs: map[string]string{
"build-arg:BUILDKIT_DISABLE_FILEOP": strconv.FormatBool(!isFileOp),
},
LocalDirs: map[string]string{
builder.DefaultLocalNameDockerfile: dir,
builder.DefaultLocalNameContext: dir,
},
}, nil)

if !isFileOp {
require.Contains(t, err.Error(), "chmod is not supported")
return
}
require.NoError(t, err)

dt, err := ioutil.ReadFile(filepath.Join(destDir, "fooperm"))
require.NoError(t, err)
require.Equal(t, "0644\n", string(dt))

dt, err = ioutil.ReadFile(filepath.Join(destDir, "barperm"))
require.NoError(t, err)
require.Equal(t, "0777\n", string(dt))

dt, err = ioutil.ReadFile(filepath.Join(destDir, "foobisperm"))
require.NoError(t, err)
require.Equal(t, "0000\n", string(dt))
}

func testCopyOverrideFiles(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)
isFileOp := getFileOp(t, sb)
Expand Down
2 changes: 2 additions & 0 deletions frontend/dockerfile/instructions/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ type AddCommand struct {
withNameAndCode
SourcesAndDest
Chown string
Chmod string
}

// Expand variables
Expand All @@ -209,6 +210,7 @@ type CopyCommand struct {
SourcesAndDest
From string
Chown string
Chmod string
}

// Expand variables
Expand Down
4 changes: 4 additions & 0 deletions frontend/dockerfile/instructions/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,15 @@ func parseAdd(req parseRequest) (*AddCommand, error) {
return nil, errNoDestinationArgument("ADD")
}
flChown := req.flags.AddString("chown", "")
flChmod := req.flags.AddString("chmod", "")
if err := req.flags.Parse(); err != nil {
return nil, err
}
return &AddCommand{
SourcesAndDest: SourcesAndDest(req.args),
withNameAndCode: newWithNameAndCode(req),
Chown: flChown.Value,
Chmod: flChmod.Value,
}, nil
}

Expand All @@ -255,6 +257,7 @@ func parseCopy(req parseRequest) (*CopyCommand, error) {
}
flChown := req.flags.AddString("chown", "")
flFrom := req.flags.AddString("from", "")
flChmod := req.flags.AddString("chmod", "")
if err := req.flags.Parse(); err != nil {
return nil, err
}
Expand All @@ -263,6 +266,7 @@ func parseCopy(req parseRequest) (*CopyCommand, error) {
From: flFrom.Value,
withNameAndCode: newWithNameAndCode(req),
Chown: flChown.Value,
Chmod: flChmod.Value,
}, nil
}

Expand Down
2 changes: 2 additions & 0 deletions solver/llbsolver/file/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ func (fb *Backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount,

return mkfile(ctx, dir, action, u, mnt.m.IdentityMapping())
}

func (fb *Backend) Rm(ctx context.Context, m fileoptypes.Mount, action pb.FileActionRm) error {
mnt, ok := m.(*Mount)
if !ok {
Expand All @@ -309,6 +310,7 @@ func (fb *Backend) Rm(ctx context.Context, m fileoptypes.Mount, action pb.FileAc

return rm(ctx, dir, action)
}

func (fb *Backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mount, action pb.FileActionCopy) error {
mnt1, ok := m1.(*Mount)
if !ok {
Expand Down