Merge pull request containerd#10722 from henry118/uidmap2

Support multiple uid/gid mappings [2/2]

client/snapshotter_opts_unix.go

@@ -39,9 +39,24 @@ const (
 // to shift the filesystem ownership (user namespace mapping) automatically; currently
 // supported by the fuse-overlayfs and overlay snapshotters
 func WithRemapperLabels(ctrUID, hostUID, ctrGID, hostGID, length uint32) snapshots.Opt {
+	uidMap := []specs.LinuxIDMapping{{ContainerID: ctrUID, HostID: hostUID, Size: length}}
+	gidMap := []specs.LinuxIDMapping{{ContainerID: ctrGID, HostID: hostGID, Size: length}}
+	return WithUserNSRemapperLabels(uidMap, gidMap)
+}
+
+// WithUserNSRemapperLabels creates the labels used by any supporting snapshotter
+// to shift the filesystem ownership (user namespace mapping) automatically; currently
+// supported by the fuse-overlayfs and overlay snapshotters
+func WithUserNSRemapperLabels(uidmaps, gidmaps []specs.LinuxIDMapping) snapshots.Opt {
+	idMap := userns.IDMap{
+		UidMap: uidmaps,
+		GidMap: gidmaps,
+	}
+	uidmapLabel, gidmapLabel := idMap.Marshal()
 	return snapshots.WithLabels(map[string]string{
-		snapshots.LabelSnapshotUIDMapping: fmt.Sprintf("%d:%d:%d", ctrUID, hostUID, length),
-		snapshots.LabelSnapshotGIDMapping: fmt.Sprintf("%d:%d:%d", ctrGID, hostGID, length)})
+		snapshots.LabelSnapshotUIDMapping: uidmapLabel,
+		snapshots.LabelSnapshotGIDMapping: gidmapLabel,
+	})
 }
 
 func resolveSnapshotOptions(ctx context.Context, client *Client, snapshotterName string, snapshotter snapshots.Snapshotter, parent string, opts ...snapshots.Opt) (string, error) {
@@ -89,27 +104,15 @@ func resolveSnapshotOptions(ctx context.Context, client *Client, snapshotterName
 		return "", fmt.Errorf("snapshotter %q doesn't support idmap mounts on this host, configure `slow_chown` to allow a slower and expensive fallback", snapshotterName)
 	}
 
-	var uidMap, gidMap specs.LinuxIDMapping
-	_, err = fmt.Sscanf(uidMapLabel, "%d:%d:%d", &uidMap.ContainerID, &uidMap.HostID, &uidMap.Size)
-	if err != nil {
-		return "", fmt.Errorf("uidMapLabel unparsable: %w", err)
-	}
-	_, err = fmt.Sscanf(gidMapLabel, "%d:%d:%d", &gidMap.ContainerID, &gidMap.HostID, &gidMap.Size)
-	if err != nil {
-		return "", fmt.Errorf("gidMapLabel unparsable: %w", err)
+	rsn := remappedSnapshot{Parent: parent}
+	if err = rsn.IDMap.Unmarshal(uidMapLabel, gidMapLabel); err != nil {
+		return "", fmt.Errorf("failed to unmarshal uid/gid map snapshotter labels: %w", err)
 	}
 
-	if uidMap.ContainerID != 0 || gidMap.ContainerID != 0 {
-		return "", fmt.Errorf("Container UID/GID of 0 only supported currently (%d/%d)", uidMap.ContainerID, gidMap.ContainerID)
+	if _, err := rsn.IDMap.RootPair(); err != nil {
+		return "", fmt.Errorf("container UID/GID mapping entries of 0 are required but not found")
 	}
 
-	rsn := remappedSnapshot{
-		Parent: parent,
-		IDMap: userns.IDMap{
-			UidMap: []specs.LinuxIDMapping{uidMap},
-			GidMap: []specs.LinuxIDMapping{gidMap},
-		},
-	}
 	usernsID, err := rsn.ID()
 	if err != nil {
 		return "", fmt.Errorf("failed to remap snapshot: %w", err)

cmd/ctr/commands/run/run_unix.go

@@ -178,11 +178,7 @@ func NewContainer(ctx context.Context, client *containerd.Client, cliContext *cl
 				// fuse-overlayfs - https://github.com/containerd/fuse-overlayfs-snapshotter
 				// overlay - in case of idmapped mount points are supported by host kernel (Linux kernel 5.19)
 				if cliContext.Bool("remap-labels") {
-					// TODO: the optimization code path on id mapped mounts only supports single mapping entry today.
-					if len(uidSpec) > 1 || len(gidSpec) > 1 {
-						return nil, errors.New("'remap-labels' option does not support multiple mappings")
-					}
-					cOpts = append(cOpts, containerd.WithNewSnapshot(id, image, containerd.WithRemapperLabels(0, uidSpec[0].HostID, 0, gidSpec[0].HostID, uidSpec[0].Size)))
+					cOpts = append(cOpts, containerd.WithNewSnapshot(id, image, containerd.WithUserNSRemapperLabels(uidSpec, gidSpec)))
 				} else {
 					cOpts = append(cOpts, containerd.WithUserNSRemappedSnapshot(id, image, uidSpec, gidSpec))
 				}

core/mount/mount_idmapped_linux.go

@@ -26,39 +26,55 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-// TODO: Support multiple mappings in future
-func parseIDMapping(mapping string) ([]syscall.SysProcIDMap, error) {
+func parseIDMapping(mapping string) (syscall.SysProcIDMap, error) {
+	var retval syscall.SysProcIDMap
+
 	parts := strings.Split(mapping, ":")
 	if len(parts) != 3 {
-		return nil, fmt.Errorf("user namespace mappings require the format `container-id:host-id:size`")
+		return retval, fmt.Errorf("user namespace mappings require the format `container-id:host-id:size`")
 	}
 
 	cID, err := strconv.Atoi(parts[0])
 	if err != nil {
-		return nil, fmt.Errorf("invalid container id for user namespace remapping, %w", err)
+		return retval, fmt.Errorf("invalid container id for user namespace remapping, %w", err)
 	}
 
 	hID, err := strconv.Atoi(parts[1])
 	if err != nil {
-		return nil, fmt.Errorf("invalid host id for user namespace remapping, %w", err)
+		return retval, fmt.Errorf("invalid host id for user namespace remapping, %w", err)
 	}
 
 	size, err := strconv.Atoi(parts[2])
 	if err != nil {
-		return nil, fmt.Errorf("invalid size for user namespace remapping, %w", err)
+		return retval, fmt.Errorf("invalid size for user namespace remapping, %w", err)
 	}
 
 	if cID < 0 || hID < 0 || size < 0 {
-		return nil, fmt.Errorf("invalid mapping %s, all IDs and size must be positive integers", mapping)
+		return retval, fmt.Errorf("invalid mapping %s, all IDs and size must be positive integers", mapping)
+	}
+
+	retval = syscall.SysProcIDMap{
+		ContainerID: cID,
+		HostID:      hID,
+		Size:        size,
 	}
 
-	return []syscall.SysProcIDMap{
-		{
-			ContainerID: cID,
-			HostID:      hID,
-			Size:        size,
-		},
-	}, nil
+	return retval, nil
+}
+
+func parseIDMappingList(mappings string) ([]syscall.SysProcIDMap, error) {
+	var (
+		res     []syscall.SysProcIDMap
+		maplist = strings.Split(mappings, ",")
+	)
+	for _, m := range maplist {
+		r, err := parseIDMapping(m)
+		if err != nil {
+			return nil, err
+		}
+		res = append(res, r)
+	}
+	return res, nil
 }
 
 // IDMapMount clones the mount at source to target, applying GID/UID idmapping of the user namespace for target path
@@ -93,15 +109,15 @@ func IDMapMountWithAttrs(source, target string, usernsFd int, attrSet uint64, at
 	return nil
 }
 
-// GetUsernsFD forks the current process and creates a user namespace using
-// the specified mappings.
+// GetUsernsFD forks the current process and creates a user namespace using the specified mappings.
+// Expected syntax of ID mapping parameter is "%d:%d:%d[,%d:%d:%d,...]"
 func GetUsernsFD(uidmap, gidmap string) (_usernsFD *os.File, _ error) {
-	uidMaps, err := parseIDMapping(uidmap)
+	uidMaps, err := parseIDMappingList(uidmap)
 	if err != nil {
 		return nil, err
 	}
 
-	gidMaps, err := parseIDMapping(gidmap)
+	gidMaps, err := parseIDMappingList(gidmap)
 	if err != nil {
 		return nil, err
 	}

core/mount/mount_idmapped_linux_test.go

@@ -102,6 +102,11 @@ func testGetUsernsFD(t *testing.T) {
 			gidMaps: "0:1000:100",
 			hasErr:  false,
 		},
+		{
+			uidMaps: "0:1000:100,100:2000:200",
+			gidMaps: "0:1000:100,100:2000:200",
+			hasErr:  false,
+		},
 		{
 			uidMaps: "100:1000:100",
 			gidMaps: "0:-1:100",
@@ -117,6 +122,11 @@ func testGetUsernsFD(t *testing.T) {
 			gidMaps: "0:1000:-1",
 			hasErr:  true,
 		},
+		{
+			uidMaps: "100:1000:100",
+			gidMaps: "0:1000:-1,100:1000:100",
+			hasErr:  true,
+		},
 	} {
 		t.Run(fmt.Sprintf("#%v", idx), func(t *testing.T) {
 			_, err := GetUsernsFD(tc.uidMaps, tc.gidMaps)