cri: Fix userns with Dockerfile VOLUME mounts that need copy

rata · rata · commit 41d74aee2024 · 2025-08-27T12:02:26.000+02:00
If a Dockerfile is using a `VOLUME` directive and the directory exists in the rootfs, like in this example: FROM docker.io/library/alpine:latest VOLUME [ "/run" ] The alpine container image already contains a "/run" directory. This will force the code in WithVolumes() to copy its content to the new volume created for the VOLUME directive. This copies the content as well as the ownership. However, as we perform the mounts from the host POV without being inside a userns, the idmap option will just shift the IDs in ways that will screw up the ownerships when copied. We should only use the idmap option when running the container inside a userns, so the ownerships are fine (the userns will do a shift and the idmap another, to make it all seem as if there was no UID/GID shift in the first place). This PR does just that, remove the idmap option from mounts so we copy the files without any ID transformations. It's simpler and easier to reason about if we just don't mount with the idmap option here: all files are copied just fine without ID transformations and ID transformation is applied via the idmap option at mount time when running the pod. Also, note that `VOLUME` directives that refer to directories that don't exist on the rootfs work fine (`VOLUME [ "/rata" ]` for example), as there is no copy done in that case so the permissions weren't changed. Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com> (cherry picked from commit 41953f7) Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
diff --git a/core/mount/temp.go b/core/mount/temp.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/containerd/log"
 )
@@ -104,6 +105,25 @@ func RemoveVolatileOption(mounts []Mount) []Mount {
 	return mounts
 }
 
+// RemoveIDMapOption copies and removes the uidmap/gidmap options on any of the mounts using it.
+func RemoveIDMapOption(mounts []Mount) []Mount {
+	var out []Mount
+	for i, m := range mounts {
+		for j, opt := range m.Options {
+			if strings.HasPrefix(opt, "uidmap") || strings.HasPrefix(opt, "gidmap") {
+				if out == nil {
+					out = copyMounts(mounts)
+				}
+				out[i].Options = append(out[i].Options[:j], out[i].Options[j+1:]...)
+			}
+		}
+	}
+	if out != nil {
+		return out
+	}
+	return mounts
+}
+
 // copyMounts creates a copy of the original slice to allow for modification and not altering the original
 func copyMounts(in []Mount) []Mount {
 	out := make([]Mount, len(in))
diff --git a/internal/cri/opts/container.go b/internal/cri/opts/container.go
@@ -76,6 +76,11 @@ func WithVolumes(volumeMounts map[string]string, platform imagespec.Platform) co
 		}
 		mounts = mount.RemoveVolatileOption(mounts)
 
+		// Always copy files without UID/GID transformations.
+		// The ID transformations are only needed when running inside a userns, that is not
+		// the case here.
+		mounts = mount.RemoveIDMapOption(mounts)
+
 		root, err := os.MkdirTemp("", "ctd-volume")
 		if err != nil {
 			return err
