podman run use pod userns even with --pod-id-file

The pod was set after we checked the namespace and the namespace code
only checked the --pod flag but didn't consider --pod-id-file option.
As such fix the check to first set the pod option on the spec then use
that for the namespace. Also make sure we always use an empty default
otherwise it would be impossible in the backend to know if a user
requested a specific userns or not, i.e. even in case of a set
PODMAN_USERNS env a container should still get the userns from the pod
and not use the var in this case. Therefore unset it from the default
cli value.

There are more issues here around --pod-id-file and cli validation that
does not consider the option as conflicting with --userns like --pod
does but I decided to fix the bug at hand and don't try to fix the
entire mess which most likely would take days.

Fixes #22931

Signed-off-by: Paul Holzinger <[email protected]>

cmd/podman/common/create.go

@@ -1,8 +1,6 @@
 package common
 
 import (
-	"os"
-
 	"github.com/containers/common/pkg/auth"
 	"github.com/containers/common/pkg/completion"
 	commonFlag "github.com/containers/common/pkg/flag"
@@ -723,7 +721,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 
 		usernsFlagName := "userns"
 		createFlags.String(
-			usernsFlagName, os.Getenv("PODMAN_USERNS"),
+			usernsFlagName, "",
 			"User namespace to use",
 		)
 		_ = cmd.RegisterFlagCompletionFunc(usernsFlagName, AutocompleteUserNamespace)

cmd/podman/kube/play.go

@@ -134,7 +134,7 @@ func playFlags(cmd *cobra.Command) {
 	_ = cmd.RegisterFlagCompletionFunc(logOptFlagName, common.AutocompleteLogOpt)
 
 	usernsFlagName := "userns"
-	flags.StringVar(&playOptions.Userns, usernsFlagName, os.Getenv("PODMAN_USERNS"),
+	flags.StringVar(&playOptions.Userns, usernsFlagName, "",
 		"User namespace to use",
 	)
 	_ = cmd.RegisterFlagCompletionFunc(usernsFlagName, common.AutocompleteUserNamespace)

pkg/specgenutil/specgen.go

@@ -222,7 +222,8 @@ func setNamespaces(rtc *config.Config, s *specgen.SpecGenerator, c *entities.Con
 		}
 	}
 	userns := c.UserNS
-	if userns == "" && c.Pod == "" {
+	// caller must make sure s.Pod is set before calling this function.
+	if userns == "" && s.Pod == "" {
 		if ns, ok := os.LookupEnv("PODMAN_USERNS"); ok {
 			userns = ns
 		} else {
@@ -388,6 +389,22 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 		s.StartupHealthConfig.Successes = int(c.StartupHCSuccesses)
 	}
 
+	if len(s.Pod) == 0 || len(c.Pod) > 0 {
+		s.Pod = c.Pod
+	}
+
+	if len(c.PodIDFile) > 0 {
+		if len(s.Pod) > 0 {
+			return errors.New("cannot specify both --pod and --pod-id-file")
+		}
+		podID, err := ReadPodIDFile(c.PodIDFile)
+		if err != nil {
+			return err
+		}
+		s.Pod = podID
+	}
+
+	// Important s.Pod must be set above here.
 	if err := setNamespaces(rtc, s, c); err != nil {
 		return err
 	}
@@ -408,21 +425,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 		s.PublishExposedPorts = &c.PublishAll
 	}
 
-	if len(s.Pod) == 0 || len(c.Pod) > 0 {
-		s.Pod = c.Pod
-	}
-
-	if len(c.PodIDFile) > 0 {
-		if len(s.Pod) > 0 {
-			return errors.New("cannot specify both --pod and --pod-id-file")
-		}
-		podID, err := ReadPodIDFile(c.PodIDFile)
-		if err != nil {
-			return err
-		}
-		s.Pod = podID
-	}
-
 	expose, err := CreateExpose(c.Expose)
 	if err != nil {
 		return err

test/e2e/run_userns_test.go

@@ -422,4 +422,41 @@ var _ = Describe("Podman UserNS support", func() {
 			podmanTest.RestartRemoteService()
 		}
 	})
+
+	It("podman pod userns inherited for containers", func() {
+		podName := "testPod"
+		podIDFile := filepath.Join(podmanTest.TempDir, "podid")
+		podCreate := podmanTest.Podman([]string{"pod", "create", "--pod-id-file", podIDFile, "--uidmap", "0:0:1000", "--name", podName})
+		podCreate.WaitWithDefaultTimeout()
+		Expect(podCreate).Should(ExitCleanly())
+
+		// The containers should not use PODMAN_USERNS as they must inherited the userns from the pod.
+		os.Setenv("PODMAN_USERNS", "keep-id")
+		defer os.Unsetenv("PODMAN_USERNS")
+
+		expectedMapping := `         0          0       1000
+         0          0       1000
+`
+		// rootless mapping is split in two ranges
+		if isRootless() {
+			expectedMapping = `         0          0          1
+         1          1        999
+         0          0          1
+         1          1        999
+`
+		}
+
+		session := podmanTest.Podman([]string{"run", "--pod", podName, ALPINE, "cat", "/proc/self/uid_map", "/proc/self/gid_map"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+		output := string(session.Out.Contents())
+		Expect(output).To(Equal(expectedMapping))
+
+		// https://github.com/containers/podman/issues/22931
+		session = podmanTest.Podman([]string{"run", "--pod-id-file", podIDFile, ALPINE, "cat", "/proc/self/uid_map", "/proc/self/gid_map"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+		output = string(session.Out.Contents())
+		Expect(output).To(Equal(expectedMapping))
+	})
 })