From b8c7bd5f5a253a8cf678bbb82054f4ac1ae42f80 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 14 Aug 2023 20:21:08 +0300 Subject: [PATCH 1/5] Implement readUser on Windows Signed-off-by: Gabriel Adrian Samfira --- solver/llbsolver/file/backend.go | 48 +++-------------------- solver/llbsolver/file/backend_unix.go | 49 ++++++++++++++++++++++++ solver/llbsolver/file/backend_windows.go | 22 +++++++++++ solver/llbsolver/file/user_linux.go | 3 +- solver/llbsolver/file/user_nolinux.go | 18 --------- solver/llbsolver/file/user_other.go | 19 +++++++++ solver/llbsolver/file/user_windows.go | 45 ++++++++++++++++++++++ solver/llbsolver/ops/file.go | 2 +- 8 files changed, 143 insertions(+), 63 deletions(-) create mode 100644 solver/llbsolver/file/backend_unix.go create mode 100644 solver/llbsolver/file/backend_windows.go delete mode 100644 solver/llbsolver/file/user_nolinux.go create mode 100644 solver/llbsolver/file/user_other.go create mode 100644 solver/llbsolver/file/user_windows.go diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 6656050b9038..2c8bb23e57f2 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -11,6 +11,7 @@ import ( "github.com/containerd/continuity/fs" "github.com/docker/docker/pkg/idtools" + "github.com/moby/buildkit/executor" "github.com/moby/buildkit/snapshot" "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" "github.com/moby/buildkit/solver/pb" @@ -27,46 +28,6 @@ func timestampToTime(ts int64) *time.Time { return &tm } -func mapUserToChowner(user *copy.User, idmap *idtools.IdentityMapping) (copy.Chowner, error) { - if user == nil { - return func(old *copy.User) (*copy.User, error) { - if old == nil { - if idmap == nil { - return nil, nil - } - old = ©.User{} // root - // non-nil old is already mapped - if idmap != nil { - identity, err := idmap.ToHost(idtools.Identity{ - UID: old.UID, - GID: old.GID, - }) - if err != nil { - return nil, err - } - return ©.User{UID: identity.UID, GID: identity.GID}, nil - } - } - return old, nil - }, nil - } - u := *user - if idmap != nil { - identity, err := idmap.ToHost(idtools.Identity{ - UID: user.UID, - GID: user.GID, - }) - if err != nil { - return nil, err - } - u.UID = identity.UID - u.GID = identity.GID - } - return func(*copy.User) (*copy.User, error) { - return &u, nil - }, nil -} - func mkdir(ctx context.Context, d string, action pb.FileActionMkDir, user *copy.User, idmap *idtools.IdentityMapping) error { p, err := fs.RootPath(d, action.Path) if err != nil { @@ -251,6 +212,7 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u * } type Backend struct { + Executor executor.Executor } func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error { @@ -266,7 +228,7 @@ func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, } defer lm.Unmount() - u, err := readUser(action.Owner, user, group) + u, err := readUser(action.Owner, user, group, fb.Executor) if err != nil { return err } @@ -287,7 +249,7 @@ func (fb *Backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount, } defer lm.Unmount() - u, err := readUser(action.Owner, user, group) + u, err := readUser(action.Owner, user, group, fb.Executor) if err != nil { return err } @@ -335,7 +297,7 @@ func (fb *Backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mou } defer lm2.Unmount() - u, err := readUser(action.Owner, user, group) + u, err := readUser(action.Owner, user, group, fb.Executor) if err != nil { return err } diff --git a/solver/llbsolver/file/backend_unix.go b/solver/llbsolver/file/backend_unix.go new file mode 100644 index 000000000000..d01290f300ac --- /dev/null +++ b/solver/llbsolver/file/backend_unix.go @@ -0,0 +1,49 @@ +//go:build !windows +// +build !windows + +package file + +import ( + "github.com/docker/docker/pkg/idtools" + copy "github.com/tonistiigi/fsutil/copy" +) + +func mapUserToChowner(user *copy.User, idmap *idtools.IdentityMapping) (copy.Chowner, error) { + if user == nil { + return func(old *copy.User) (*copy.User, error) { + if old == nil { + if idmap == nil { + return nil, nil + } + old = ©.User{} // root + // non-nil old is already mapped + if idmap != nil { + identity, err := idmap.ToHost(idtools.Identity{ + UID: old.UID, + GID: old.GID, + }) + if err != nil { + return nil, err + } + return ©.User{UID: identity.UID, GID: identity.GID}, nil + } + } + return old, nil + }, nil + } + u := *user + if idmap != nil { + identity, err := idmap.ToHost(idtools.Identity{ + UID: user.UID, + GID: user.GID, + }) + if err != nil { + return nil, err + } + u.UID = identity.UID + u.GID = identity.GID + } + return func(*copy.User) (*copy.User, error) { + return &u, nil + }, nil +} diff --git a/solver/llbsolver/file/backend_windows.go b/solver/llbsolver/file/backend_windows.go new file mode 100644 index 000000000000..03f3bbe7b308 --- /dev/null +++ b/solver/llbsolver/file/backend_windows.go @@ -0,0 +1,22 @@ +package file + +import ( + "github.com/docker/docker/pkg/idtools" + copy "github.com/tonistiigi/fsutil/copy" +) + +func mapUserToChowner(user *copy.User, idmap *idtools.IdentityMapping) (copy.Chowner, error) { + if user == nil || user.SID == "" { + return func(old *copy.User) (*copy.User, error) { + if old == nil || old.SID == "" { + old = ©.User{ + SID: idtools.ContainerAdministratorSidString, + } + } + return old, nil + }, nil + } + return func(*copy.User) (*copy.User, error) { + return user, nil + }, nil +} diff --git a/solver/llbsolver/file/user_linux.go b/solver/llbsolver/file/user_linux.go index 1f17431f5c32..1d2970ce9bde 100644 --- a/solver/llbsolver/file/user_linux.go +++ b/solver/llbsolver/file/user_linux.go @@ -5,6 +5,7 @@ import ( "syscall" "github.com/containerd/continuity/fs" + "github.com/moby/buildkit/executor" "github.com/moby/buildkit/snapshot" "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" "github.com/moby/buildkit/solver/pb" @@ -13,7 +14,7 @@ import ( copy "github.com/tonistiigi/fsutil/copy" ) -func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { +func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Executor) (*copy.User, error) { if chopt == nil { return nil, nil } diff --git a/solver/llbsolver/file/user_nolinux.go b/solver/llbsolver/file/user_nolinux.go deleted file mode 100644 index 80652fd4abe4..000000000000 --- a/solver/llbsolver/file/user_nolinux.go +++ /dev/null @@ -1,18 +0,0 @@ -//go:build !linux -// +build !linux - -package file - -import ( - "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" - "github.com/moby/buildkit/solver/pb" - "github.com/pkg/errors" - copy "github.com/tonistiigi/fsutil/copy" -) - -func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { - if chopt == nil { - return nil, nil - } - return nil, errors.New("only implemented in linux") -} diff --git a/solver/llbsolver/file/user_other.go b/solver/llbsolver/file/user_other.go new file mode 100644 index 000000000000..34f6e8d2d029 --- /dev/null +++ b/solver/llbsolver/file/user_other.go @@ -0,0 +1,19 @@ +//go:build !linux && !windows +// +build !linux,!windows + +package file + +import ( + "github.com/moby/buildkit/executor" + "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" + "github.com/moby/buildkit/solver/pb" + "github.com/pkg/errors" + copy "github.com/tonistiigi/fsutil/copy" +) + +func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Executor) (*copy.User, error) { + if chopt == nil { + return nil, nil + } + return nil, errors.New("only implemented in linux and windows") +} diff --git a/solver/llbsolver/file/user_windows.go b/solver/llbsolver/file/user_windows.go new file mode 100644 index 000000000000..d0eb76b579fe --- /dev/null +++ b/solver/llbsolver/file/user_windows.go @@ -0,0 +1,45 @@ +package file + +import ( + "context" + + "github.com/docker/docker/pkg/idtools" + "github.com/moby/buildkit/executor" + "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" + "github.com/moby/buildkit/solver/pb" + "github.com/moby/buildkit/util/windows" + "github.com/pkg/errors" + copy "github.com/tonistiigi/fsutil/copy" +) + +func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Executor) (*copy.User, error) { + if chopt == nil { + return nil, nil + } + + if chopt.User != nil { + switch u := chopt.User.User.(type) { + case *pb.UserOpt_ByName: + if mu == nil { + return nil, errors.Errorf("invalid missing user mount") + } + mmu, ok := mu.(*Mount) + if !ok { + return nil, errors.Errorf("invalid mount type %T", mu) + } + rootMounts, release, err := mmu.m.Mount() + if err != nil { + return nil, err + } + defer release() + ident, err := windows.ResolveUsernameToSID(context.Background(), exec, rootMounts, u.ByName.Name) + if err != nil { + return nil, err + } + return ©.User{SID: ident.SID}, nil + default: + return ©.User{SID: idtools.ContainerAdministratorSidString}, nil + } + } + return ©.User{SID: idtools.ContainerAdministratorSidString}, nil +} diff --git a/solver/llbsolver/ops/file.go b/solver/llbsolver/ops/file.go index db81201f1ac5..128fffbe2cb9 100644 --- a/solver/llbsolver/ops/file.go +++ b/solver/llbsolver/ops/file.go @@ -167,7 +167,7 @@ func (f *fileOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu inpRefs = append(inpRefs, workerRef.ImmutableRef) } - fs := NewFileOpSolver(f.w, &file.Backend{}, f.refManager) + fs := NewFileOpSolver(f.w, &file.Backend{Executor: f.w.Executor()}, f.refManager) outs, err := fs.Solve(ctx, inpRefs, f.op.Actions, g) if err != nil { return nil, err From 8a369a9eba2bf64d0f0f79eadffd6413248947d4 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 31 Aug 2023 12:39:29 +0300 Subject: [PATCH 2/5] Remove the need for an exported Executor field Signed-off-by: Gabriel Adrian Samfira --- solver/llbsolver/file/backend.go | 17 +++++++++++++---- solver/llbsolver/file/user_linux.go | 6 +++++- solver/llbsolver/file/user_other.go | 6 +++++- solver/llbsolver/file/user_windows.go | 6 ++++++ solver/llbsolver/ops/file.go | 4 +++- 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 2c8bb23e57f2..2f8c4183c850 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -211,8 +211,16 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u * return nil } +// NewFileOpBackend returns a new file operation backend. The executor is currently only used for Windows, +// and it is used to construct the readUserFn field set in the returned Backend. +func NewFileOpBackend(exec executor.Executor) *Backend { + return &Backend{ + readUserFn: getReadUserFn(exec), + } +} + type Backend struct { - Executor executor.Executor + readUserFn func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) } func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error { @@ -228,7 +236,8 @@ func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, } defer lm.Unmount() - u, err := readUser(action.Owner, user, group, fb.Executor) + // u, err := readUser(action.Owner, user, group, fb.Executor) + u, err := fb.readUserFn(action.Owner, user, group) if err != nil { return err } @@ -249,7 +258,7 @@ func (fb *Backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount, } defer lm.Unmount() - u, err := readUser(action.Owner, user, group, fb.Executor) + u, err := fb.readUserFn(action.Owner, user, group) if err != nil { return err } @@ -297,7 +306,7 @@ func (fb *Backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mou } defer lm2.Unmount() - u, err := readUser(action.Owner, user, group, fb.Executor) + u, err := fb.readUserFn(action.Owner, user, group) if err != nil { return err } diff --git a/solver/llbsolver/file/user_linux.go b/solver/llbsolver/file/user_linux.go index 1d2970ce9bde..d204f80a41e1 100644 --- a/solver/llbsolver/file/user_linux.go +++ b/solver/llbsolver/file/user_linux.go @@ -14,7 +14,11 @@ import ( copy "github.com/tonistiigi/fsutil/copy" ) -func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Executor) (*copy.User, error) { +func getReadUserFn(exec executor.Executor) func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { + return readUser +} + +func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { if chopt == nil { return nil, nil } diff --git a/solver/llbsolver/file/user_other.go b/solver/llbsolver/file/user_other.go index 34f6e8d2d029..9e97afce381d 100644 --- a/solver/llbsolver/file/user_other.go +++ b/solver/llbsolver/file/user_other.go @@ -11,7 +11,11 @@ import ( copy "github.com/tonistiigi/fsutil/copy" ) -func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Executor) (*copy.User, error) { +func getReadUserFn(exec executor.Executor) func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { + return readUser +} + +func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { if chopt == nil { return nil, nil } diff --git a/solver/llbsolver/file/user_windows.go b/solver/llbsolver/file/user_windows.go index d0eb76b579fe..10b938b65a6e 100644 --- a/solver/llbsolver/file/user_windows.go +++ b/solver/llbsolver/file/user_windows.go @@ -12,6 +12,12 @@ import ( copy "github.com/tonistiigi/fsutil/copy" ) +func getReadUserFn(exec executor.Executor) func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { + return func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { + return readUser(chopt, mu, mg, exec) + } +} + func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Executor) (*copy.User, error) { if chopt == nil { return nil, nil diff --git a/solver/llbsolver/ops/file.go b/solver/llbsolver/ops/file.go index 128fffbe2cb9..179c167b57e7 100644 --- a/solver/llbsolver/ops/file.go +++ b/solver/llbsolver/ops/file.go @@ -167,7 +167,9 @@ func (f *fileOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu inpRefs = append(inpRefs, workerRef.ImmutableRef) } - fs := NewFileOpSolver(f.w, &file.Backend{Executor: f.w.Executor()}, f.refManager) + backend := file.NewFileOpBackend(f.w.Executor()) + + fs := NewFileOpSolver(f.w, backend, f.refManager) outs, err := fs.Solve(ctx, inpRefs, f.op.Actions, g) if err != nil { return nil, err From fe3ca93c09c00175814c80bd0f06797d6540015f Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Wed, 6 Sep 2023 09:18:21 +0300 Subject: [PATCH 3/5] Move readUser code outside of the file package Signed-off-by: Gabriel Adrian Samfira --- solver/llbsolver/file/backend.go | 9 ++++---- solver/llbsolver/file/refmanager.go | 4 ++++ solver/llbsolver/ops/file.go | 2 +- solver/llbsolver/{file => ops}/user_linux.go | 21 ++++++++++++++----- solver/llbsolver/{file => ops}/user_other.go | 2 +- .../llbsolver/{file => ops}/user_windows.go | 12 ++++++++--- 6 files changed, 36 insertions(+), 14 deletions(-) rename solver/llbsolver/{file => ops}/user_linux.go (86%) rename solver/llbsolver/{file => ops}/user_other.go (97%) rename solver/llbsolver/{file => ops}/user_windows.go (84%) diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 2f8c4183c850..05c094eeb026 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -11,7 +11,6 @@ import ( "github.com/containerd/continuity/fs" "github.com/docker/docker/pkg/idtools" - "github.com/moby/buildkit/executor" "github.com/moby/buildkit/snapshot" "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" "github.com/moby/buildkit/solver/pb" @@ -213,14 +212,16 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u * // NewFileOpBackend returns a new file operation backend. The executor is currently only used for Windows, // and it is used to construct the readUserFn field set in the returned Backend. -func NewFileOpBackend(exec executor.Executor) *Backend { +func NewFileOpBackend(readUserFn ReadUserCallback) *Backend { return &Backend{ - readUserFn: getReadUserFn(exec), + readUserFn: readUserFn, } } +type ReadUserCallback func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) + type Backend struct { - readUserFn func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) + readUserFn ReadUserCallback } func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error { diff --git a/solver/llbsolver/file/refmanager.go b/solver/llbsolver/file/refmanager.go index b9f3b2ea3ca3..cb26ba288665 100644 --- a/solver/llbsolver/file/refmanager.go +++ b/solver/llbsolver/file/refmanager.go @@ -79,6 +79,10 @@ type Mount struct { readonly bool } +func (m *Mount) Mountable() snapshot.Mountable { + return m.m +} + func (m *Mount) Release(ctx context.Context) error { if m.mr != nil { return m.mr.Release(ctx) diff --git a/solver/llbsolver/ops/file.go b/solver/llbsolver/ops/file.go index 179c167b57e7..c9df96f0a5e6 100644 --- a/solver/llbsolver/ops/file.go +++ b/solver/llbsolver/ops/file.go @@ -167,7 +167,7 @@ func (f *fileOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu inpRefs = append(inpRefs, workerRef.ImmutableRef) } - backend := file.NewFileOpBackend(f.w.Executor()) + backend := file.NewFileOpBackend(getReadUserFn(f.w.Executor())) fs := NewFileOpSolver(f.w, backend, f.refManager) outs, err := fs.Solve(ctx, inpRefs, f.op.Actions, g) diff --git a/solver/llbsolver/file/user_linux.go b/solver/llbsolver/ops/user_linux.go similarity index 86% rename from solver/llbsolver/file/user_linux.go rename to solver/llbsolver/ops/user_linux.go index d204f80a41e1..75bb2ed18a67 100644 --- a/solver/llbsolver/file/user_linux.go +++ b/solver/llbsolver/ops/user_linux.go @@ -1,4 +1,4 @@ -package file +package ops import ( "os" @@ -7,6 +7,7 @@ import ( "github.com/containerd/continuity/fs" "github.com/moby/buildkit/executor" "github.com/moby/buildkit/snapshot" + "github.com/moby/buildkit/solver/llbsolver/file" "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" "github.com/moby/buildkit/solver/pb" "github.com/opencontainers/runc/libcontainer/user" @@ -29,11 +30,16 @@ func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) if mu == nil { return nil, errors.Errorf("invalid missing user mount") } - mmu, ok := mu.(*Mount) + mmu, ok := mu.(*file.Mount) if !ok { return nil, errors.Errorf("invalid mount type %T", mu) } - lm := snapshot.LocalMounter(mmu.m) + mountable := mmu.Mountable() + if mountable == nil { + return nil, errors.Errorf("invalid mountable") + } + + lm := snapshot.LocalMounter(mountable) dir, err := lm.Mount() if err != nil { return nil, err @@ -83,11 +89,16 @@ func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) if mg == nil { return nil, errors.Errorf("invalid missing group mount") } - mmg, ok := mg.(*Mount) + mmg, ok := mg.(*file.Mount) if !ok { return nil, errors.Errorf("invalid mount type %T", mg) } - lm := snapshot.LocalMounter(mmg.m) + mountable := mmg.Mountable() + if mountable == nil { + return nil, errors.Errorf("invalid mountable") + } + + lm := snapshot.LocalMounter(mountable) dir, err := lm.Mount() if err != nil { return nil, err diff --git a/solver/llbsolver/file/user_other.go b/solver/llbsolver/ops/user_other.go similarity index 97% rename from solver/llbsolver/file/user_other.go rename to solver/llbsolver/ops/user_other.go index 9e97afce381d..64f2683a9f25 100644 --- a/solver/llbsolver/file/user_other.go +++ b/solver/llbsolver/ops/user_other.go @@ -1,7 +1,7 @@ //go:build !linux && !windows // +build !linux,!windows -package file +package ops import ( "github.com/moby/buildkit/executor" diff --git a/solver/llbsolver/file/user_windows.go b/solver/llbsolver/ops/user_windows.go similarity index 84% rename from solver/llbsolver/file/user_windows.go rename to solver/llbsolver/ops/user_windows.go index 10b938b65a6e..4f95f224ee89 100644 --- a/solver/llbsolver/file/user_windows.go +++ b/solver/llbsolver/ops/user_windows.go @@ -1,10 +1,11 @@ -package file +package ops import ( "context" "github.com/docker/docker/pkg/idtools" "github.com/moby/buildkit/executor" + "github.com/moby/buildkit/solver/llbsolver/file" "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/windows" @@ -29,11 +30,16 @@ func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Execut if mu == nil { return nil, errors.Errorf("invalid missing user mount") } - mmu, ok := mu.(*Mount) + mmu, ok := mu.(*file.Mount) if !ok { return nil, errors.Errorf("invalid mount type %T", mu) } - rootMounts, release, err := mmu.m.Mount() + mountable := mmu.Mountable() + if mountable == nil { + return nil, errors.Errorf("invalid mountable") + } + + rootMounts, release, err := mountable.Mount() if err != nil { return nil, err } From 2f3bda8ecbbe671a4773ebd89bda5da9e94dc6ee Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 5 Oct 2023 19:46:46 +0300 Subject: [PATCH 4/5] Use snapshot.Mountable as an argument type to readUser Signed-off-by: Gabriel Adrian Samfira --- solver/llbsolver/file/backend.go | 57 +++++++++++++++++++++------- solver/llbsolver/ops/file.go | 5 ++- solver/llbsolver/ops/user_linux.go | 28 +++----------- solver/llbsolver/ops/user_other.go | 8 ++-- solver/llbsolver/ops/user_windows.go | 25 ++++-------- 5 files changed, 64 insertions(+), 59 deletions(-) diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 05c094eeb026..7ea0378e8437 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -212,19 +212,22 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u * // NewFileOpBackend returns a new file operation backend. The executor is currently only used for Windows, // and it is used to construct the readUserFn field set in the returned Backend. -func NewFileOpBackend(readUserFn ReadUserCallback) *Backend { - return &Backend{ - readUserFn: readUserFn, +func NewFileOpBackend(readUser ReadUserCallback) (*backend, error) { + if readUser == nil { + return nil, errors.New("readUser callback must be provided") } + return &backend{ + readUser: readUser, + }, nil } -type ReadUserCallback func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) +type ReadUserCallback func(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) -type Backend struct { - readUserFn ReadUserCallback +type backend struct { + readUser ReadUserCallback } -func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error { +func (fb *backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error { mnt, ok := m.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m) @@ -237,8 +240,7 @@ func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, } defer lm.Unmount() - // u, err := readUser(action.Owner, user, group, fb.Executor) - u, err := fb.readUserFn(action.Owner, user, group) + u, err := fb.readUserWrapper(action.Owner, user, group) if err != nil { return err } @@ -246,7 +248,7 @@ func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, return mkdir(ctx, dir, action, u, mnt.m.IdentityMapping()) } -func (fb *Backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkFile) error { +func (fb *backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkFile) error { mnt, ok := m.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m) @@ -259,7 +261,7 @@ func (fb *Backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount, } defer lm.Unmount() - u, err := fb.readUserFn(action.Owner, user, group) + u, err := fb.readUserWrapper(action.Owner, user, group) if err != nil { return err } @@ -267,7 +269,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 { +func (fb *backend) Rm(ctx context.Context, m fileoptypes.Mount, action pb.FileActionRm) error { mnt, ok := m.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m) @@ -283,7 +285,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 { +func (fb *backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mount, action pb.FileActionCopy) error { mnt1, ok := m1.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m1) @@ -307,7 +309,7 @@ func (fb *Backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mou } defer lm2.Unmount() - u, err := fb.readUserFn(action.Owner, user, group) + u, err := fb.readUserWrapper(action.Owner, user, group) if err != nil { return err } @@ -315,6 +317,33 @@ func (fb *Backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mou return docopy(ctx, src, dest, action, u, mnt2.m.IdentityMapping()) } +func (fb *backend) readUserWrapper(owner *pb.ChownOpt, user, group fileoptypes.Mount) (*copy.User, error) { + var userMountable, groupMountable snapshot.Mountable + if user != nil { + usr, ok := user.(*Mount) + if !ok { + return nil, errors.Errorf("invalid mount type %T", user) + } + userMountable = usr.Mountable() + } + + if group != nil { + grp, ok := group.(*Mount) + if !ok { + return nil, errors.Errorf("invalid mount type %T", group) + } + groupMountable = grp.Mountable() + } + + // We don't check the mountables for nil here. Depending on the ChownOpt value, + // one of them may be nil. Allow the readUser function to handle this. + u, err := fb.readUser(owner, userMountable, groupMountable) + if err != nil { + return nil, err + } + return u, nil +} + func cleanPath(s string) (string, error) { s, err := system.CheckSystemDriveAndRemoveDriveLetter(s, runtime.GOOS) if err != nil { diff --git a/solver/llbsolver/ops/file.go b/solver/llbsolver/ops/file.go index c9df96f0a5e6..f17869381a90 100644 --- a/solver/llbsolver/ops/file.go +++ b/solver/llbsolver/ops/file.go @@ -167,7 +167,10 @@ func (f *fileOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu inpRefs = append(inpRefs, workerRef.ImmutableRef) } - backend := file.NewFileOpBackend(getReadUserFn(f.w.Executor())) + backend, err := file.NewFileOpBackend(getReadUserFn(f.w)) + if err != nil { + return nil, err + } fs := NewFileOpSolver(f.w, backend, f.refManager) outs, err := fs.Solve(ctx, inpRefs, f.op.Actions, g) diff --git a/solver/llbsolver/ops/user_linux.go b/solver/llbsolver/ops/user_linux.go index 75bb2ed18a67..68daeceab0ba 100644 --- a/solver/llbsolver/ops/user_linux.go +++ b/solver/llbsolver/ops/user_linux.go @@ -5,21 +5,19 @@ import ( "syscall" "github.com/containerd/continuity/fs" - "github.com/moby/buildkit/executor" "github.com/moby/buildkit/snapshot" - "github.com/moby/buildkit/solver/llbsolver/file" - "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" "github.com/moby/buildkit/solver/pb" + "github.com/moby/buildkit/worker" "github.com/opencontainers/runc/libcontainer/user" "github.com/pkg/errors" copy "github.com/tonistiigi/fsutil/copy" ) -func getReadUserFn(exec executor.Executor) func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { +func getReadUserFn(worker worker.Worker) func(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) { return readUser } -func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { +func readUser(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) { if chopt == nil { return nil, nil } @@ -30,16 +28,8 @@ func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) if mu == nil { return nil, errors.Errorf("invalid missing user mount") } - mmu, ok := mu.(*file.Mount) - if !ok { - return nil, errors.Errorf("invalid mount type %T", mu) - } - mountable := mmu.Mountable() - if mountable == nil { - return nil, errors.Errorf("invalid mountable") - } - lm := snapshot.LocalMounter(mountable) + lm := snapshot.LocalMounter(mu) dir, err := lm.Mount() if err != nil { return nil, err @@ -89,16 +79,8 @@ func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) if mg == nil { return nil, errors.Errorf("invalid missing group mount") } - mmg, ok := mg.(*file.Mount) - if !ok { - return nil, errors.Errorf("invalid mount type %T", mg) - } - mountable := mmg.Mountable() - if mountable == nil { - return nil, errors.Errorf("invalid mountable") - } - lm := snapshot.LocalMounter(mountable) + lm := snapshot.LocalMounter(mg) dir, err := lm.Mount() if err != nil { return nil, err diff --git a/solver/llbsolver/ops/user_other.go b/solver/llbsolver/ops/user_other.go index 64f2683a9f25..f173ae38ca44 100644 --- a/solver/llbsolver/ops/user_other.go +++ b/solver/llbsolver/ops/user_other.go @@ -4,18 +4,18 @@ package ops import ( - "github.com/moby/buildkit/executor" - "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" + "github.com/moby/buildkit/snapshot" "github.com/moby/buildkit/solver/pb" + "github.com/moby/buildkit/worker" "github.com/pkg/errors" copy "github.com/tonistiigi/fsutil/copy" ) -func getReadUserFn(exec executor.Executor) func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { +func getReadUserFn(worker worker.Worker) func(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) { return readUser } -func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { +func readUser(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) { if chopt == nil { return nil, nil } diff --git a/solver/llbsolver/ops/user_windows.go b/solver/llbsolver/ops/user_windows.go index 4f95f224ee89..553a2d210d0f 100644 --- a/solver/llbsolver/ops/user_windows.go +++ b/solver/llbsolver/ops/user_windows.go @@ -4,22 +4,21 @@ import ( "context" "github.com/docker/docker/pkg/idtools" - "github.com/moby/buildkit/executor" - "github.com/moby/buildkit/solver/llbsolver/file" - "github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes" + "github.com/moby/buildkit/snapshot" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/windows" + "github.com/moby/buildkit/worker" "github.com/pkg/errors" copy "github.com/tonistiigi/fsutil/copy" ) -func getReadUserFn(exec executor.Executor) func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { - return func(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount) (*copy.User, error) { - return readUser(chopt, mu, mg, exec) +func getReadUserFn(worker worker.Worker) func(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) { + return func(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) { + return readUser(chopt, mu, mg, worker) } } -func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Executor) (*copy.User, error) { +func readUser(chopt *pb.ChownOpt, mu, mg snapshot.Mountable, worker worker.Worker) (*copy.User, error) { if chopt == nil { return nil, nil } @@ -30,21 +29,13 @@ func readUser(chopt *pb.ChownOpt, mu, mg fileoptypes.Mount, exec executor.Execut if mu == nil { return nil, errors.Errorf("invalid missing user mount") } - mmu, ok := mu.(*file.Mount) - if !ok { - return nil, errors.Errorf("invalid mount type %T", mu) - } - mountable := mmu.Mountable() - if mountable == nil { - return nil, errors.Errorf("invalid mountable") - } - rootMounts, release, err := mountable.Mount() + rootMounts, release, err := mu.Mount() if err != nil { return nil, err } defer release() - ident, err := windows.ResolveUsernameToSID(context.Background(), exec, rootMounts, u.ByName.Name) + ident, err := windows.ResolveUsernameToSID(context.Background(), worker.Executor(), rootMounts, u.ByName.Name) if err != nil { return nil, err } From 2585dd955d0ad21ade190bc59373175dc32ff8e5 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 6 Oct 2023 17:08:54 +0300 Subject: [PATCH 5/5] Fix linting issue Signed-off-by: Gabriel Adrian Samfira --- solver/llbsolver/file/backend.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 7ea0378e8437..311fc5043d35 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -212,22 +212,22 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u * // NewFileOpBackend returns a new file operation backend. The executor is currently only used for Windows, // and it is used to construct the readUserFn field set in the returned Backend. -func NewFileOpBackend(readUser ReadUserCallback) (*backend, error) { +func NewFileOpBackend(readUser ReadUserCallback) (*Backend, error) { if readUser == nil { return nil, errors.New("readUser callback must be provided") } - return &backend{ + return &Backend{ readUser: readUser, }, nil } type ReadUserCallback func(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) -type backend struct { +type Backend struct { readUser ReadUserCallback } -func (fb *backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error { +func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error { mnt, ok := m.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m) @@ -248,7 +248,7 @@ func (fb *backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, return mkdir(ctx, dir, action, u, mnt.m.IdentityMapping()) } -func (fb *backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkFile) error { +func (fb *Backend) Mkfile(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkFile) error { mnt, ok := m.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m) @@ -269,7 +269,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 { +func (fb *Backend) Rm(ctx context.Context, m fileoptypes.Mount, action pb.FileActionRm) error { mnt, ok := m.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m) @@ -285,7 +285,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 { +func (fb *Backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mount, action pb.FileActionCopy) error { mnt1, ok := m1.(*Mount) if !ok { return errors.Errorf("invalid mount type %T", m1) @@ -317,7 +317,7 @@ func (fb *backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mou return docopy(ctx, src, dest, action, u, mnt2.m.IdentityMapping()) } -func (fb *backend) readUserWrapper(owner *pb.ChownOpt, user, group fileoptypes.Mount) (*copy.User, error) { +func (fb *Backend) readUserWrapper(owner *pb.ChownOpt, user, group fileoptypes.Mount) (*copy.User, error) { var userMountable, groupMountable snapshot.Mountable if user != nil { usr, ok := user.(*Mount)