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

Implement readUser on Windows #3518

Merged
merged 5 commits into from
Oct 20, 2023
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
87 changes: 44 additions & 43 deletions solver/llbsolver/file/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,46 +27,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 = &copy.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 &copy.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 {
Expand Down Expand Up @@ -250,7 +210,21 @@ 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(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 snapshot.Mountable) (*copy.User, error)

type Backend struct {
readUser ReadUserCallback
}

func (fb *Backend) Mkdir(ctx context.Context, m, user, group fileoptypes.Mount, action pb.FileActionMkDir) error {
Expand All @@ -266,7 +240,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 := fb.readUserWrapper(action.Owner, user, group)
if err != nil {
return err
}
Expand All @@ -287,7 +261,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 := fb.readUserWrapper(action.Owner, user, group)
if err != nil {
return err
}
Expand Down Expand Up @@ -335,14 +309,41 @@ 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 := fb.readUserWrapper(action.Owner, user, group)
if err != nil {
return err
}

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)
Copy link
Member

Choose a reason for hiding this comment

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

There is still something iffy about these type casts but looks like the previous code had a similar problem, so no update is needed with this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. There are a few architectural boundaries being broken here which could do with some refactor, but it will require touching a lot of code. This pattern is present in a number of places and should probably be tackled at some point as part of a broader cleanup.

Thanks for the review and the merge!

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 {
Expand Down
49 changes: 49 additions & 0 deletions solver/llbsolver/file/backend_unix.go
Original file line number Diff line number Diff line change
@@ -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 = &copy.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 &copy.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
}
22 changes: 22 additions & 0 deletions solver/llbsolver/file/backend_windows.go
Original file line number Diff line number Diff line change
@@ -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 = &copy.User{
SID: idtools.ContainerAdministratorSidString,
}
}
return old, nil
}, nil
}
return func(*copy.User) (*copy.User, error) {
return user, nil
}, nil
}
4 changes: 4 additions & 0 deletions solver/llbsolver/file/refmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 0 additions & 18 deletions solver/llbsolver/file/user_nolinux.go

This file was deleted.

7 changes: 6 additions & 1 deletion solver/llbsolver/ops/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,12 @@ 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)
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)
if err != nil {
return nil, err
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
package file
package ops

import (
"os"
"syscall"

"github.com/containerd/continuity/fs"
"github.com/moby/buildkit/snapshot"
"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 readUser(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 snapshot.Mountable) (*copy.User, error) {
if chopt == nil {
return nil, nil
}
Expand All @@ -24,11 +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.(*Mount)
if !ok {
return nil, errors.Errorf("invalid mount type %T", mu)
}
lm := snapshot.LocalMounter(mmu.m)

lm := snapshot.LocalMounter(mu)
dir, err := lm.Mount()
if err != nil {
return nil, err
Expand Down Expand Up @@ -78,11 +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.(*Mount)
if !ok {
return nil, errors.Errorf("invalid mount type %T", mg)
}
lm := snapshot.LocalMounter(mmg.m)

lm := snapshot.LocalMounter(mg)
dir, err := lm.Mount()
if err != nil {
return nil, err
Expand Down
23 changes: 23 additions & 0 deletions solver/llbsolver/ops/user_other.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//go:build !linux && !windows
// +build !linux,!windows

package ops

import (
"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(worker worker.Worker) func(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) {
return readUser
}

func readUser(chopt *pb.ChownOpt, mu, mg snapshot.Mountable) (*copy.User, error) {
if chopt == nil {
return nil, nil
}
return nil, errors.New("only implemented in linux and windows")
}
48 changes: 48 additions & 0 deletions solver/llbsolver/ops/user_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package ops

import (
"context"

"github.com/docker/docker/pkg/idtools"
"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(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 snapshot.Mountable, worker worker.Worker) (*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")
}

rootMounts, release, err := mu.Mount()
if err != nil {
return nil, err
}
defer release()
ident, err := windows.ResolveUsernameToSID(context.Background(), worker.Executor(), rootMounts, u.ByName.Name)
if err != nil {
return nil, err
}
return &copy.User{SID: ident.SID}, nil
default:
return &copy.User{SID: idtools.ContainerAdministratorSidString}, nil
}
}
return &copy.User{SID: idtools.ContainerAdministratorSidString}, nil
}