Skip to content

Add user namespace ID mapping support for overlaybd - #398

Open
arc9693 wants to merge 3 commits into
containerd:mainfrom
arc9693:main
Open

Add user namespace ID mapping support for overlaybd#398
arc9693 wants to merge 3 commits into
containerd:mainfrom
arc9693:main

Conversation

@arc9693

@arc9693 arc9693 commented Aug 28, 2026

Copy link
Copy Markdown

What this PR does / why we need it:
This PR adds opt-in user namespace / ID-mapped mount support to overlaybd-snapshotter when remapIDs is enabled in the snapshotter config.

This change:

  • Adds a remapIDs snapshotter config option (default: false) and enables it only after a host support probe (kernel ID-mapped overlay support, d_type on the snapshotter root, and user namespace FD creation).
  • For overlaybd block lowers, pre-idmaps the parent block mount to snapshots/<active-id>/block/idmapped-lower via containerd’s mount.GetUsernsFD and mount.IDMapMount, then uses that path as overlay lowerdir without overlay uidmap/gidmap (avoids double-shifting).
  • Propagates uidmap/gidmap mount options from snapshot labels for normal overlay mounts when remapIDs is enabled.
  • Sets writable upperdir ownership to the mapped container root when mapping labels are present.
  • Unmounts idmapped-lower during snapshot removal.

Without --remap-labels, existing containerd chown/remap behavior is unchanged. With remapIDs: false (default), behavior is unchanged for all workloads.

Fixes #354

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

Test plan

Unit tests

go test ./pkg/snapshot/... -count=1

Manual (overlaybd image + remapIDs: true in /etc/overlaybd-snapshotter/config.json)

Manual validation was done with the overlaybd ctr binary built from this repository (./bin/ctr), not the generic system ctr.

  1. Fast path with --remap-labels:

    ctr run --rm -t --snapshotter=overlaybd \
      --uidmap 0:100000:65536 --gidmap 0:100000:65536 --remap-labels \
      registry.hub.docker.com/overlaybd/redis:6.2.1_obd test /bin/sh -c 'ls /bin/sh'
    • Expect success in ~sub-second (warm) prepare time
    • Snapshotter log: idmapped block device mount: ... -> .../idmapped-lower
    • findmnt shows lowerdir=.../idmapped-lower
  2. Backward-compatible slow path without --remap-labels:

    ctr run --rm -t --snapshotter=overlaybd \
      --uidmap 0:100000:65536 --gidmap 0:100000:65536 \
      registry.hub.docker.com/overlaybd/redis:6.2.1_obd test /bin/sh -c 'ls /bin/sh'
    • Expect success (~15–20s first run via containerd chown/remap path)
  3. Two concurrent containers with different maps show different host UIDs on files under .../block/idmapped-lower/bin/sh (e.g. 100000 vs 200000).

  4. After --rm, idmapped-lower for that snapshot is removed.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds opt-in user-namespace / ID-mapped mount support to the overlaybd snapshotter when remapIDs is enabled, aiming to avoid containerd’s expensive chown-walk fallback for userns workloads while keeping default behavior unchanged.

Changes:

  • Introduces remapIDs snapshotter config gating, enabled only after a host support probe (kernel/overlay idmap support, d_type, userns FD creation).
  • Adds logic to pre-idmap overlaybd block-device lowers (via mount.GetUsernsFD + mount.IDMapMount) and propagates uidmap=/gidmap= overlay options for normal overlay mounts when labels are present.
  • Adjusts upperdir ownership based on mapping labels and unmounts the idmapped-lower mount during snapshot removal; adds unit tests around mapping parsing / option propagation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/snapshot/overlay.go Core remapIDs wiring, idmapped-lower handling for block lowers, uid/gid mapping option propagation, ownership adjustments, and removal cleanup.
pkg/snapshot/overlay_test.go Adds unit tests for mapping parsing and option propagation (plus a lightweight option-prefix helper).
pkg/snapshot/idmap_linux.go Linux-only host support probe for enabling remapIDs.
pkg/snapshot/idmap_linux_test.go Tests that remapIDs only enables when the support probe succeeds.
pkg/snapshot/docker.go Updates one docker fallback path to the new normalOverlayMount signature (but another call site still needs updating).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/snapshot/docker.go Outdated
Comment thread pkg/snapshot/overlay.go
Comment on lines +265 to +268
remapIDs := false
if bootConfig.RemapIDs {
supported, err := remapSupportProbe(root)
if err != nil {
Comment thread pkg/snapshot/overlay_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

pkg/snapshot/overlay.go:1231

  • If mount.IDMapMount fails due to the destination already being mounted (e.g., concurrent calls for the same snapshot), this currently falls back to the unmapped lower even though the idmapped mount may exist. Re-check whether dst is mounted on error and treat that case as success to avoid inconsistent behavior and potential double-shifting.
	if err := mount.IDMapMount(original, dst, int(usernsFd.Fd())); err != nil {
		log.G(ctx).WithError(err).Warn("failed to idmap block device mountpoint, using unmapped block lower")
		return original, nil
	}

pkg/snapshot/overlay.go:1510

  • createSnapshot only applies Lchown when both mappedUID and mappedGID are set. If one mapping parses successfully and the other doesn’t (or is absent), ownership is left at the default even though os.Lchown supports passing -1 to leave one side unchanged. This can lead to incorrect upperdir ownership in partially-specified/partially-parsed mapping cases.
	if mappedUID != -1 && mappedGID != -1 {
		if err := os.Lchown(filepath.Join(td, "fs"), mappedUID, mappedGID); err != nil {
			return "", snapshots.Info{}, fmt.Errorf("failed to chown: %w", err)
		}
	}

pkg/snapshot/overlay.go:268

  • remapSupportProbe is only defined in idmap_linux.go (linux build tag), but overlay.go is built on all platforms. This makes non-linux builds fail with an undefined identifier. Provide a !linux stub for remapSupportProbe (returning false, nil), or move the variable definition to a non-tagged file and override it from linux-only code.
	remapIDs := false
	if bootConfig.RemapIDs {
		supported, err := remapSupportProbe(root)
		if err != nil {

pkg/snapshot/overlay_test.go:167

  • This test name says it covers basedOnBlockDeviceMount, but it never calls that function (it only checks a locally-constructed options slice). Rename the test to reflect what it actually verifies, or update it to exercise basedOnBlockDeviceMount directly.
func TestBasedOnBlockDeviceMount_omitsOverlayIdmapWhenLowerPreMapped(t *testing.T) {

Signed-off-by: Archana Choudhary <archana.choudhary.9693@gmail.com>
Signed-off-by: Archana Choudhary <archana.choudhary.9693@gmail.com>
Signed-off-by: Archana Choudhary <archana.choudhary.9693@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comment thread pkg/snapshot/docker.go
Comment on lines 296 to 300

// Normal image: fall back to standard overlay mount
// s.ParentIDs already contains [initLayerID, imageLayer1ID, ...]
return o.normalOverlayMount(s), nil
return o.normalOverlayMount(s, info), nil
}
Comment thread pkg/snapshot/overlay.go
Comment on lines +1434 to +1439
if v, ok := info.Labels[labelSnapshotUIDMapping]; ok {
options = append(options, "uidmap="+v)
}
if v, ok := info.Labels[labelSnapshotGIDMapping]; ok {
options = append(options, "gidmap="+v)
}
Comment on lines +21 to +22
// remapSupportProbe is overridden in tests on Linux.
var remapSupportProbe = detectRemapIDsSupport
@arc9693

arc9693 commented Aug 31, 2026

Copy link
Copy Markdown
Author

Hey @BigVan , Can you please review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support idmap mounts in snapshotter to avoid expensive chown walk with user namespaces

2 participants