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

client: allow non-octal chmod config for fileop.copy #5375

Merged
merged 1 commit into from
Oct 3, 2024
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
43 changes: 43 additions & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
testFileOpCopyAlwaysReplaceExistingDestPaths,
testFileOpRmWildcard,
testFileOpCopyUIDCache,
testFileOpCopyChmodText,
testCallDiskUsage,
testBuildMultiMount,
testBuildHTTPSource,
Expand Down Expand Up @@ -1705,6 +1706,48 @@ func testFileOpCopyRm(t *testing.T, sb integration.Sandbox) {
require.Equal(t, []byte("file2"), dt)
}

func testFileOpCopyChmodText(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

tcases := []struct {
src string
dest string
mode string
}{
{"file", "f1", "go-w"},
{"file", "f2", "o-rwx,g+x"},
{"file", "f3", "u=rwx,g=,o="},
{"file", "f4", "u+rw,g+r,o-x,o+w"},
{"dir", "d1", "a+X"},
{"dir", "d2", "g+rw,o+rw"},
}

st := llb.Image("alpine").
Run(llb.Shlex(`sh -c "mkdir /input && touch /input/file && mkdir /input/dir && chmod 0400 /input/dir && mkdir /expected"`))

for _, tc := range tcases {
st = st.Run(llb.Shlex(`sh -c "cp -a /input/` + tc.src + ` /expected/` + tc.dest + ` && chmod ` + tc.mode + ` /expected/` + tc.dest + `"`))
}
cp := llb.Scratch()

for _, tc := range tcases {
cp = cp.File(llb.Copy(st.Root(), "/input/"+tc.src, "/"+tc.dest, llb.ChmodOpt{ModeStr: tc.mode}))
}

for _, tc := range tcases {
st = st.Run(llb.Shlex(`sh -c '[ "$(stat -c '%A' /expected/`+tc.dest+`)" == "$(stat -c '%A' /actual/`+tc.dest+`)" ]'`), llb.AddMount("/actual", cp, llb.Readonly))
}

def, err := st.Marshal(sb.Context())
require.NoError(t, err)

_, err = c.Solve(sb.Context(), def, SolveOpt{}, nil)
require.NoError(t, err)
}

// moby/buildkit#3291
func testFileOpCopyUIDCache(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)
Expand Down
20 changes: 18 additions & 2 deletions client/llb/fileop.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ func WithUIDGID(uid, gid int) ChownOption {
}
}

type ChmodOpt struct {
Mode os.FileMode
ModeStr string
}

func (co ChmodOpt) SetCopyOption(mi *CopyInfo) {
mi.Mode = &co
}

type ChownOpt struct {
User *UserOpt
Group *UserOpt
Expand Down Expand Up @@ -492,7 +501,7 @@ type CopyOption interface {
}

type CopyInfo struct {
Mode *os.FileMode
Mode *ChmodOpt
FollowSymlinks bool
CopyDirContentsOnly bool
IncludePatterns []string
Expand Down Expand Up @@ -541,7 +550,11 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base
AlwaysReplaceExistingDestPaths: a.info.AlwaysReplaceExistingDestPaths,
}
if a.info.Mode != nil {
c.Mode = int32(*a.info.Mode)
if a.info.Mode.ModeStr != "" {
c.ModeStr = a.info.Mode.ModeStr
} else {
c.Mode = int32(a.info.Mode.Mode)
}
} else {
c.Mode = -1
}
Expand Down Expand Up @@ -574,6 +587,9 @@ func (a *fileActionCopy) addCaps(f *FileOp) {
if a.info.AlwaysReplaceExistingDestPaths {
addCap(&f.constraints, pb.CapFileCopyAlwaysReplaceExistingDestPaths)
}
if a.info.Mode.ModeStr != "" {
addCap(&f.constraints, pb.CapFileCopyModeStringFormat)
}
}

type CreatedTime time.Time
Expand Down
4 changes: 2 additions & 2 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -1359,14 +1359,14 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
copyOpt = append(copyOpt, llb.WithExcludePatterns(cfg.excludePatterns))
}

var mode *os.FileMode
var mode *llb.ChmodOpt
if cfg.chmod != "" {
p, err := strconv.ParseUint(cfg.chmod, 8, 32)
if err != nil || p > 0o7777 {
return errors.Errorf("invalid chmod parameter: '%v'. it should be octal string and between 0 and 07777", cfg.chmod)
}
perm := os.FileMode(p)
mode = &perm
mode = &llb.ChmodOpt{Mode: perm}
}

if cfg.checksum != "" {
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ require (
github.com/sirupsen/logrus v1.9.3
github.com/spdx/tools-golang v0.5.3
github.com/stretchr/testify v1.9.0
github.com/tonistiigi/fsutil v0.0.0-20240926161958-8754824c3c4f
github.com/tonistiigi/fsutil v0.0.0-20241001141129-e98dfb603c6f
github.com/tonistiigi/go-actions-cache v0.0.0-20240327122527-58651d5e11d6
github.com/tonistiigi/go-archvariant v1.0.0
github.com/tonistiigi/go-csvvalue v0.0.0-20240710180619-ddb21b71c0b4
Expand Down Expand Up @@ -166,6 +166,7 @@ require (
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/secure-systems-lab/go-securesystemslib v0.4.0 // indirect
github.com/shibumi/go-pathspec v1.3.0 // indirect
github.com/tonistiigi/dchapes-mode v0.0.0-20241001053921-ca0759fec205 // indirect
github.com/vbatts/tar-split v0.11.5 // indirect
github.com/vishvananda/netns v0.0.4 // indirect
go.opencensus.io v0.24.0 // indirect
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,10 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/tonistiigi/fsutil v0.0.0-20240926161958-8754824c3c4f h1:scejvzjNA30X9ufWPUH/a2MhWg1sQPxeC6N6wm7nWEE=
github.com/tonistiigi/fsutil v0.0.0-20240926161958-8754824c3c4f/go.mod h1:xnG7rCC28GVN8efEm5ijNp56TnNtrYCv75EtTH42yz4=
github.com/tonistiigi/dchapes-mode v0.0.0-20241001053921-ca0759fec205 h1:eUk79E1w8yMtXeHSzjKorxuC8qJOnyXQnLaJehxpJaI=
github.com/tonistiigi/dchapes-mode v0.0.0-20241001053921-ca0759fec205/go.mod h1:3Iuxbr0P7D3zUzBMAZB+ois3h/et0shEz0qApgHYGpY=
github.com/tonistiigi/fsutil v0.0.0-20241001141129-e98dfb603c6f h1:LuAOoRH31KqSNuYFl1grkcl3Sr3+Rv4uQ+e65P0SFLA=
github.com/tonistiigi/fsutil v0.0.0-20241001141129-e98dfb603c6f/go.mod h1:BwWj5HZxE1/r8WnLfSLIMDNyXE9y/XKwQwRqmgx4nKY=
github.com/tonistiigi/go-actions-cache v0.0.0-20240327122527-58651d5e11d6 h1:XFG/Wmm5dFYoqUiVChLumRjRzJm0P9k/qDMhxLqdupU=
github.com/tonistiigi/go-actions-cache v0.0.0-20240327122527-58651d5e11d6/go.mod h1:anhKd3mnC1shAbQj1Q4IJ+w6xqezxnyDYlx/yKa7IXM=
github.com/tonistiigi/go-archvariant v1.0.0 h1:5LC1eDWiBNflnTF1prCiX09yfNHIxDC/aukdhCdTyb0=
Expand Down
4 changes: 3 additions & 1 deletion solver/llbsolver/file/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,9 @@ func docopy(ctx context.Context, src, dest string, action *pb.FileActionCopy, u
ci.ExcludePatterns = action.ExcludePatterns
ci.Chown = ch
ci.Utime = timestampToTime(action.Timestamp)
if m := int(action.Mode); m != -1 {
if action.ModeStr != "" {
ci.ModeStr = action.ModeStr
} else if m := int(action.Mode); m != -1 {
ci.Mode = &m
}
ci.CopyDirContents = action.DirCopyContents
Expand Down
1 change: 1 addition & 0 deletions solver/pb/caps.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const (
CapFileCopyIncludeExcludePatterns apicaps.CapID = "file.copy.includeexcludepatterns"
CapFileRmNoFollowSymlink apicaps.CapID = "file.rm.nofollowsymlink"
CapFileCopyAlwaysReplaceExistingDestPaths apicaps.CapID = "file.copy.alwaysreplaceexistingdestpaths"
CapFileCopyModeStringFormat apicaps.CapID = "file.copy.modestring"

CapConstraints apicaps.CapID = "constraints"
CapPlatform apicaps.CapID = "platform"
Expand Down
Loading
Loading