storage: fix UID/GID map handling for shifting containers

Remove the canUseShifting branch from CreateContainer and aways
store the caller's IDMappingOptions.

In layers.go, when the driver supports shifting, set the rechown
target to identity instead of the container's maps.  This avoids
unnecessary UpdateLayerIDMap calls (which break overlay with idmapped
mounts) while still handling the migration case where a layer was
created on a system without shifting support.

Closes: https://redhat.atlassian.net/browse/RHEL-160859
Assisted-by: Claude Opus 4.6
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Author: giuseppe

storage/layers.go

@@ -1674,9 +1674,13 @@ func (r *layerStore) create(id string, parentLayer *Layer, names []string, mount
 		}
 	}
 
+	targetMappings := idMappings
+	if r.driver.SupportsShifting(moreOptions.UIDMap, moreOptions.GIDMap) {
+		targetMappings = &idtools.IDMappings{}
+	}
 	if oldMappings != nil &&
-		(!reflect.DeepEqual(oldMappings.UIDs(), idMappings.UIDs()) || !reflect.DeepEqual(oldMappings.GIDs(), idMappings.GIDs())) {
-		if err = r.driver.UpdateLayerIDMap(id, oldMappings, idMappings, mountLabel); err != nil {
+		(!reflect.DeepEqual(oldMappings.UIDs(), targetMappings.UIDs()) || !reflect.DeepEqual(oldMappings.GIDs(), targetMappings.GIDs())) {
+		if err = r.driver.UpdateLayerIDMap(id, oldMappings, targetMappings, mountLabel); err != nil {
 			cleanupFailureContext = "in UpdateLayerIDMap"
 			return nil, -1, err
 		}

storage/store_test.go

@@ -1,6 +1,8 @@
 package storage
 
 import (
+	"archive/tar"
+	"io"
 	"os"
 	"path/filepath"
 	"testing"
@@ -629,3 +631,100 @@ func TestStoreDelete(t *testing.T) {
 
 	store.Free()
 }
+
+// TestCreateContainerShifting verifies that when the overlay driver supports
+// shifting (idmapped mounts), CreateContainer stores the UID/GID maps on the
+// container layer so that Diff() can reverse-map host UIDs back to container
+// UIDs.  It also verifies that UpdateLayerIDMap is not called (no diff1
+// directory is created).
+func TestCreateContainerShifting(t *testing.T) {
+	reexec.Init()
+
+	if os.Getuid() != 0 {
+		t.Skip("test requires root")
+	}
+
+	uidMap := []idtools.IDMap{{ContainerID: 0, HostID: 1000000, Size: 65536}}
+	gidMap := []idtools.IDMap{{ContainerID: 0, HostID: 1000000, Size: 65536}}
+
+	store := newTestStore(t, StoreOptions{
+		GraphDriverName: "overlay",
+		UIDMap:          uidMap,
+		GIDMap:          gidMap,
+	})
+	defer func() {
+		_, _ = store.Shutdown(true)
+		store.Free()
+	}()
+
+	driver, err := store.GraphDriver()
+	require.NoError(t, err)
+	if !driver.SupportsShifting(uidMap, gidMap) {
+		t.Skip("overlay driver does not support shifting on this kernel")
+	}
+
+	// Create a base layer with a file owned by container UID 0.
+	baseLayer, err := store.CreateLayer("", "", nil, "", false, nil)
+	require.NoError(t, err)
+
+	image, err := store.CreateImage("", nil, baseLayer.ID, "", nil)
+	require.NoError(t, err)
+
+	// Create a container from the image with UID mappings.
+	// Pass the maps via ContainerOptions, as buildah does.
+	container, err := store.CreateContainer("", nil, image.ID, "", "", &ContainerOptions{
+		IDMappingOptions: IDMappingOptions{
+			UIDMap: uidMap,
+			GIDMap: gidMap,
+		},
+	})
+	require.NoError(t, err)
+
+	// Verify the container layer has the UID/GID maps stored.
+	containerLayer, err := store.Layer(container.LayerID)
+	require.NoError(t, err)
+	assert.Equal(t, uidMap, containerLayer.UIDMap, "container layer should have UID maps stored")
+	assert.Equal(t, gidMap, containerLayer.GIDMap, "container layer should have GID maps stored")
+
+	// Verify that UpdateLayerIDMap was NOT called: the overlay driver creates
+	// a "diff1" directory when rotating diff dirs during UpdateLayerIDMap.
+	graphRoot := store.GraphRoot()
+	diff1Path := filepath.Join(graphRoot, "overlay", containerLayer.ID, "diff1")
+	_, err = os.Stat(diff1Path)
+	assert.True(t, os.IsNotExist(err), "diff1 should not exist (UpdateLayerIDMap should not have been called)")
+
+	// Mount the container, write a file with host UID, unmount.
+	mountPoint, err := store.Mount(container.ID, "")
+	require.NoError(t, err)
+
+	testFile := filepath.Join(mountPoint, "testfile")
+	require.NoError(t, os.WriteFile(testFile, []byte("hello"), 0o644))
+	// Simulate what happens in a user namespace: the file gets the host UID.
+	require.NoError(t, os.Chown(testFile, 1000000, 1000000))
+
+	_, err = store.Unmount(container.ID, false)
+	require.NoError(t, err)
+
+	// Generate a diff and verify that Diff() maps host UIDs back to
+	// container UIDs using the stored maps (ToContainer translation).
+	rc, err := store.Diff("", containerLayer.ID, nil)
+	require.NoError(t, err)
+	defer rc.Close()
+
+	tr := tar.NewReader(rc)
+	found := false
+	for {
+		hdr, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		require.NoError(t, err)
+		if hdr.Name == "testfile" {
+			found = true
+			assert.Equal(t, 0, hdr.Uid, "Diff() should translate host UID back to container UID 0")
+			assert.Equal(t, 0, hdr.Gid, "Diff() should translate host GID back to container GID 0")
+			break
+		}
+	}
+	assert.True(t, found, "testfile should be present in the diff")
+}