Merge pull request #5971 from giuseppe/fix-userns-tests

v2, podman: fix and enable all run_userns_test.go tests

6 files changed, 98 insertions, 66 deletions

diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go
index abec1213c..488843f41 100644
--- a/cmd/podman/common/specgen.go
+++ b/cmd/podman/common/specgen.go
@@ -209,10 +209,15 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 		}
 	}
 
-	s.IDMappings, err = util.ParseIDMapping(ns.UsernsMode(c.UserNS), c.UIDMap, c.GIDMap, c.SubUIDName, c.SubGIDName)
+	userNS := ns.UsernsMode(c.UserNS)
+	s.IDMappings, err = util.ParseIDMapping(userNS, c.UIDMap, c.GIDMap, c.SubUIDName, c.SubGIDName)
 	if err != nil {
 		return err
 	}
+	// If some mappings are specified, assume a private user namespace
+	if userNS.IsDefaultValue() && (!s.IDMappings.HostUIDMapping || !s.IDMappings.HostGIDMapping) {
+		s.UserNS.NSMode = specgen.Private
+	}
 
 	s.Terminal = c.TTY
 	ep, err := ExposedPorts(c.Expose, c.Net.PublishPorts, c.PublishAll, nil)
@@ -246,20 +251,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 		s.NetNS = c.Net.Network
 	}
 
-	// TODO this is going to have to be done the libpod/server end of things
-	// USER
-	//user := c.String("user")
-	//if user == "" {
-	//	switch {
-	//	case usernsMode.IsKeepID():
-	//		user = fmt.Sprintf("%d:%d", rootless.GetRootlessUID(), rootless.GetRootlessGID())
-	//	case data == nil:
-	//		user = "0"
-	//	default:
-	//		user = data.Config.User
-	//	}
-	//}
-
 	// STOP SIGNAL
 	signalString := "TERM"
 	if sig := c.StopSignal; len(sig) > 0 {
diff --git a/pkg/namespaces/namespaces.go b/pkg/namespaces/namespaces.go
index 2cb3c3f20..2ffbde977 100644
--- a/pkg/namespaces/namespaces.go
+++ b/pkg/namespaces/namespaces.go
@@ -31,7 +31,7 @@ func (n CgroupMode) IsHost() bool {
 
 // IsDefaultValue indicates whether the cgroup namespace has the default value.
 func (n CgroupMode) IsDefaultValue() bool {
-	return n == ""
+	return n == "" || n == defaultType
 }
 
 // IsNS indicates a cgroup namespace passed in by path (ns:<path>)
@@ -102,6 +102,11 @@ func (n UsernsMode) IsAuto() bool {
 	return parts[0] == "auto"
 }
 
+// IsDefaultValue indicates whether the user namespace has the default value.
+func (n UsernsMode) IsDefaultValue() bool {
+	return n == "" || n == defaultType
+}
+
 // GetAutoOptions returns a AutoUserNsOptions with the settings to setup automatically
 // a user namespace.
 func (n UsernsMode) GetAutoOptions() (*storage.AutoUserNsOptions, error) {
diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go
index 2e0e088bf..a8b74b504 100644
--- a/pkg/specgen/generate/namespaces.go
+++ b/pkg/specgen/generate/namespaces.go
@@ -10,6 +10,7 @@ import (
 	"github.com/containers/libpod/pkg/cgroups"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/libpod/pkg/specgen"
+	"github.com/containers/libpod/pkg/util"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
 	"github.com/pkg/errors"
@@ -175,6 +176,13 @@ func GenerateNamespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod
 
 	// User
 	switch s.UserNS.NSMode {
+	case specgen.KeepID:
+		if rootless.IsRootless() {
+			s.User = ""
+		} else {
+			// keep-id as root doesn't need a user namespace
+			s.UserNS.NSMode = specgen.Host
+		}
 	case specgen.FromPod:
 		if pod == nil || infraCtr == nil {
 			return nil, errNoInfra
@@ -378,6 +386,18 @@ func specConfigureNamespaces(s *specgen.SpecGenerator, g *generate.Generator, rt
 		if err := g.RemoveLinuxNamespace(string(spec.UserNamespace)); err != nil {
 			return err
 		}
+	case specgen.KeepID:
+		var (
+			err      error
+			uid, gid int
+		)
+		s.IDMappings, uid, gid, err = util.GetKeepIDMapping()
+		if err != nil {
+			return err
+		}
+		g.SetProcessUID(uint32(uid))
+		g.SetProcessGID(uint32(gid))
+		fallthrough
 	case specgen.Private:
 		if err := g.AddOrReplaceLinuxNamespace(string(spec.UserNamespace), ""); err != nil {
 			return err
diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go
index fffbd6d9e..cee49ff51 100644
--- a/pkg/specgen/namespaces.go
+++ b/pkg/specgen/namespaces.go
@@ -76,6 +76,17 @@ func (n *Namespace) IsPod() bool {
 func (n *Namespace) IsPrivate() bool {
 	return n.NSMode == Private
 }
+
+// IsAuto indicates the namespace is auto
+func (n *Namespace) IsAuto() bool {
+	return n.NSMode == Auto
+}
+
+// IsKeepID indicates the namespace is KeepID
+func (n *Namespace) IsKeepID() bool {
+	return n.NSMode == KeepID
+}
+
 func validateUserNS(n *Namespace) error {
 	if n == nil {
 		return nil
@@ -186,12 +197,11 @@ func ParseUserNamespace(ns string) (Namespace, error) {
 		if len(split) != 2 {
 			return toReturn, errors.Errorf("invalid setting for auto: mode")
 		}
-		toReturn.NSMode = KeepID
+		toReturn.NSMode = Auto
 		toReturn.Value = split[1]
 		return toReturn, nil
 	case ns == "keep-id":
 		toReturn.NSMode = KeepID
-		toReturn.NSMode = FromContainer
 		return toReturn, nil
 	}
 	return ParseNamespace(ns)
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 64331cf66..917f57742 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -330,6 +330,58 @@ func ParseSignal(rawSignal string) (syscall.Signal, error) {
 	return sig, nil
 }
 
+// GetKeepIDMapping returns the mappings and the user to use when keep-id is used
+func GetKeepIDMapping() (*storage.IDMappingOptions, int, int, error) {
+	options := storage.IDMappingOptions{
+		HostUIDMapping: true,
+		HostGIDMapping: true,
+	}
+	uid, gid := 0, 0
+	if rootless.IsRootless() {
+		min := func(a, b int) int {
+			if a < b {
+				return a
+			}
+			return b
+		}
+
+		uid = rootless.GetRootlessUID()
+		gid = rootless.GetRootlessGID()
+
+		uids, gids, err := rootless.GetConfiguredMappings()
+		if err != nil {
+			return nil, -1, -1, errors.Wrapf(err, "cannot read mappings")
+		}
+		maxUID, maxGID := 0, 0
+		for _, u := range uids {
+			maxUID += u.Size
+		}
+		for _, g := range gids {
+			maxGID += g.Size
+		}
+
+		options.UIDMap, options.GIDMap = nil, nil
+
+		options.UIDMap = append(options.UIDMap, idtools.IDMap{ContainerID: 0, HostID: 1, Size: min(uid, maxUID)})
+		options.UIDMap = append(options.UIDMap, idtools.IDMap{ContainerID: uid, HostID: 0, Size: 1})
+		if maxUID > uid {
+			options.UIDMap = append(options.UIDMap, idtools.IDMap{ContainerID: uid + 1, HostID: uid + 1, Size: maxUID - uid})
+		}
+
+		options.GIDMap = append(options.GIDMap, idtools.IDMap{ContainerID: 0, HostID: 1, Size: min(gid, maxGID)})
+		options.GIDMap = append(options.GIDMap, idtools.IDMap{ContainerID: gid, HostID: 0, Size: 1})
+		if maxGID > gid {
+			options.GIDMap = append(options.GIDMap, idtools.IDMap{ContainerID: gid + 1, HostID: gid + 1, Size: maxGID - gid})
+		}
+
+		options.HostUIDMapping = false
+		options.HostGIDMapping = false
+
+	}
+	// Simply ignore the setting and do not setup an inner namespace for root as it is a no-op
+	return &options, uid, gid, nil
+}
+
 // ParseIDMapping takes idmappings and subuid and subgid maps and returns a storage mapping
 func ParseIDMapping(mode namespaces.UsernsMode, uidMapSlice, gidMapSlice []string, subUIDMap, subGIDMap string) (*storage.IDMappingOptions, error) {
 	options := storage.IDMappingOptions{
@@ -350,53 +402,8 @@ func ParseIDMapping(mode namespaces.UsernsMode, uidMapSlice, gidMapSlice []strin
 		return &options, nil
 	}
 	if mode.IsKeepID() {
-		if len(uidMapSlice) > 0 || len(gidMapSlice) > 0 {
-			return nil, errors.New("cannot specify custom mappings with --userns=keep-id")
-		}
-		if len(subUIDMap) > 0 || len(subGIDMap) > 0 {
-			return nil, errors.New("cannot specify subuidmap or subgidmap with --userns=keep-id")
-		}
-		if rootless.IsRootless() {
-			min := func(a, b int) int {
-				if a < b {
-					return a
-				}
-				return b
-			}
-
-			uid := rootless.GetRootlessUID()
-			gid := rootless.GetRootlessGID()
-
-			uids, gids, err := rootless.GetConfiguredMappings()
-			if err != nil {
-				return nil, errors.Wrapf(err, "cannot read mappings")
-			}
-			maxUID, maxGID := 0, 0
-			for _, u := range uids {
-				maxUID += u.Size
-			}
-			for _, g := range gids {
-				maxGID += g.Size
-			}
-
-			options.UIDMap, options.GIDMap = nil, nil
-
-			options.UIDMap = append(options.UIDMap, idtools.IDMap{ContainerID: 0, HostID: 1, Size: min(uid, maxUID)})
-			options.UIDMap = append(options.UIDMap, idtools.IDMap{ContainerID: uid, HostID: 0, Size: 1})
-			if maxUID > uid {
-				options.UIDMap = append(options.UIDMap, idtools.IDMap{ContainerID: uid + 1, HostID: uid + 1, Size: maxUID - uid})
-			}
-
-			options.GIDMap = append(options.GIDMap, idtools.IDMap{ContainerID: 0, HostID: 1, Size: min(gid, maxGID)})
-			options.GIDMap = append(options.GIDMap, idtools.IDMap{ContainerID: gid, HostID: 0, Size: 1})
-			if maxGID > gid {
-				options.GIDMap = append(options.GIDMap, idtools.IDMap{ContainerID: gid + 1, HostID: gid + 1, Size: maxGID - gid})
-			}
-
-			options.HostUIDMapping = false
-			options.HostGIDMapping = false
-		}
-		// Simply ignore the setting and do not setup an inner namespace for root as it is a no-op
+		options.HostUIDMapping = false
+		options.HostGIDMapping = false
 		return &options, nil
 	}
 
diff --git a/test/e2e/run_userns_test.go b/test/e2e/run_userns_test.go
index a4e99ab71..25f12ec2e 100644
--- a/test/e2e/run_userns_test.go
+++ b/test/e2e/run_userns_test.go
@@ -22,7 +22,6 @@ var _ = Describe("Podman UserNS support", func() {
 	)
 
 	BeforeEach(func() {
-		Skip(v2fail)
 		if os.Getenv("SKIP_USERNS") != "" {
 			Skip("Skip userns tests.")
 		}