Skip to content

[WIP] allow user namespace remapping using snapshotters #3885

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.

Sign up for GitHub

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

Closed
liaojh1998 wants to merge 1 commit into from
Closed

[WIP] allow user namespace remapping using snapshotters #3885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions container_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1377,9 +1377,9 @@ func testUserNamespaces(t *testing.T, readonlyRootFS bool) {
}),
)}
if readonlyRootFS {
opts = append([]NewContainerOpts{WithRemappedSnapshotView(id, image, 1000, 2000)}, opts...)
opts = append([]NewContainerOpts{WithRemappedSnapshotView(id, image, 1000, 2000, remapperChown)}, opts...)
} else {
opts = append([]NewContainerOpts{WithRemappedSnapshot(id, image, 1000, 2000)}, opts...)
opts = append([]NewContainerOpts{WithRemappedSnapshot(id, image, 1000, 2000, remapperChown)}, opts...)
}

container, err := client.NewContainer(ctx, id, opts...)
Expand Down
49 changes: 36 additions & 13 deletions container_opts_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,35 @@ import (
"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/mount"
"github.com/containerd/containerd/snapshots"
"github.com/opencontainers/image-spec/identity"
)

// Remapper specifies the type of remapping to be done
type Remapper string

const (
remapperChown Remapper = "chown"
remapperSnapshotter Remapper = "snapshotter"
)

// WithRemappedSnapshot creates a new snapshot and remaps the uid/gid for the
// filesystem to be used by a container with user namespaces
func WithRemappedSnapshot(id string, i Image, uid, gid uint32) NewContainerOpts {
return withRemappedSnapshotBase(id, i, uid, gid, false)
func WithRemappedSnapshot(id string, i Image, uid, gid uint32, remapper Remapper) NewContainerOpts {
return withRemappedSnapshotBase(id, i, uid, gid, false, remapper)
}

// WithRemappedSnapshotView is similar to WithRemappedSnapshot but rootfs is mounted as read-only.
func WithRemappedSnapshotView(id string, i Image, uid, gid uint32) NewContainerOpts {
return withRemappedSnapshotBase(id, i, uid, gid, true)
func WithRemappedSnapshotView(id string, i Image, uid, gid uint32, remapper Remapper) NewContainerOpts {
return withRemappedSnapshotBase(id, i, uid, gid, true, remapper)
}

func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool) NewContainerOpts {
func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool, remapper Remapper) NewContainerOpts {
return func(ctx context.Context, client *Client, c *containers.Container) error {
if remapper != remapperChown && remapper != remapperSnapshotter {
return fmt.Errorf("remapper must be either '%s' or '%s'", remapperChown, remapperSnapshotter)
}

diffIDs, err := i.(*image).i.RootFS(ctx, client.ContentStore(), client.platform)
if err != nil {
return err
Expand All @@ -52,6 +65,7 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool
var (
parent = identity.ChainID(diffIDs).String()
usernsID = fmt.Sprintf("%s-%d-%d", parent, uid, gid)
opts = []snapshots.Opt{}
)
c.Snapshotter, err = client.resolveSnapshotterName(ctx, c.Snapshotter)
if err != nil {
Expand All @@ -61,30 +75,39 @@ func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool
if err != nil {
return err
}
if remapper == remapperSnapshotter {
opts = append(opts,
snapshots.WithLabels(map[string]string{
"containerd.io/snapshot/uidmapping": fmt.Sprintf("%d:%d:%d", 0, uid, 65535),
"containerd.io/snapshot/gidmapping": fmt.Sprintf("%d:%d:%d", 0, gid, 65535),
}))
}
if _, err := snapshotter.Stat(ctx, usernsID); err == nil {
if _, err := snapshotter.Prepare(ctx, id, usernsID); err == nil {
if _, err := snapshotter.Prepare(ctx, id, usernsID, opts...); err == nil {
c.SnapshotKey = id
c.Image = i.Name()
return nil
} else if !errdefs.IsNotFound(err) {
return err
}
}
mounts, err := snapshotter.Prepare(ctx, usernsID+"-remap", parent)
mounts, err := snapshotter.Prepare(ctx, usernsID+"-remap", parent, opts...)
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan Jan 6, 2020

Choose a reason for hiding this comment

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

This doesn't seem necessary if the snapshotter is supporting the mapping. I think a better approach would be to leave the remapping functions alone and just provide a helper which can set the labels and just use WithNewSnapshot

Copy link
Copy Markdown
Contributor Author

@liaojh1998 liaojh1998 Jan 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Okay. I'm thinking of an implementation that uses a switch statement in WithRemappedSnapshot() and WithRemappedSnapshotView() to call WithNewSnapshot() based on the remapper. Would this be a better approach or should I leave all of the remapping functions alone?

cc @AkihiroSuda

Copy link
Copy Markdown
Contributor Author

@liaojh1998 liaojh1998 Jan 23, 2020

Choose a reason for hiding this comment

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

ping @dmcgowan

if err != nil {
return err
}
if err := remapRootFS(ctx, mounts, uid, gid); err != nil {
snapshotter.Remove(ctx, usernsID)
return err
if remapper == remapperChown {
if err := remapRootFS(ctx, mounts, uid, gid); err != nil {
snapshotter.Remove(ctx, usernsID)
return err
}
}
if err := snapshotter.Commit(ctx, usernsID, usernsID+"-remap"); err != nil {
if err := snapshotter.Commit(ctx, usernsID, usernsID+"-remap", opts...); err != nil {
return err
}
if readonly {
_, err = snapshotter.View(ctx, id, usernsID)
_, err = snapshotter.View(ctx, id, usernsID, opts...)
} else {
_, err = snapshotter.Prepare(ctx, id, usernsID)
_, err = snapshotter.Prepare(ctx, id, usernsID, opts...)
}
if err != nil {
return err
Expand Down