Merge pull request #6051 from rhatdan/containers.conf

Fixes for test/e2e/containers_conf_test.go

4 files changed, 99 insertions, 95 deletions

diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go
index a8b74b504..5c065cdda 100644
--- a/pkg/specgen/generate/namespaces.go
+++ b/pkg/specgen/generate/namespaces.go
@@ -7,7 +7,6 @@ import (
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/define"
-	"github.com/containers/libpod/pkg/cgroups"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/libpod/pkg/specgen"
 	"github.com/containers/libpod/pkg/util"
@@ -49,51 +48,26 @@ func GetDefaultNamespaceMode(nsType string, cfg *config.Config, pod *libpod.Pod)
 		}
 	}
 
-	// If we have containers.conf and are not using cgroupns, use that.
-	if cfg != nil && nsType != "cgroup" {
-		switch nsType {
-		case "pid":
-			return specgen.ParseNamespace(cfg.Containers.PidNS)
-		case "ipc":
-			return specgen.ParseNamespace(cfg.Containers.IPCNS)
-		case "uts":
-			return specgen.ParseNamespace(cfg.Containers.UTSNS)
-		case "user":
-			return specgen.ParseUserNamespace(cfg.Containers.UserNS)
-		case "net":
-			ns, _, err := specgen.ParseNetworkNamespace(cfg.Containers.NetNS)
-			return ns, err
-		}
+	if cfg == nil {
+		cfg = &config.Config{}
 	}
-
 	switch nsType {
-	case "pid", "ipc", "uts":
-		// PID, IPC, UTS both default to private, do nothing
+	case "pid":
+		return specgen.ParseNamespace(cfg.Containers.PidNS)
+	case "ipc":
+		return specgen.ParseNamespace(cfg.Containers.IPCNS)
+	case "uts":
+		return specgen.ParseNamespace(cfg.Containers.UTSNS)
 	case "user":
-		// User namespace always defaults to host
-		toReturn.NSMode = specgen.Host
-	case "net":
-		// Net defaults to Slirp on rootless, Bridge otherwise.
-		if rootless.IsRootless() {
-			toReturn.NSMode = specgen.Slirp
-		} else {
-			toReturn.NSMode = specgen.Bridge
-		}
+		return specgen.ParseUserNamespace(cfg.Containers.UserNS)
 	case "cgroup":
-		// Cgroup is host for v1, private for v2.
-		// We can't trust c/common for this, as it only assumes private.
-		cgroupsv2, err := cgroups.IsCgroup2UnifiedMode()
-		if err != nil {
-			return toReturn, err
-		}
-		if !cgroupsv2 {
-			toReturn.NSMode = specgen.Host
-		}
-	default:
-		return toReturn, errors.Wrapf(define.ErrInvalidArg, "invalid namespace type %s passed", nsType)
+		return specgen.ParseCgroupNamespace(cfg.Containers.CgroupNS)
+	case "net":
+		ns, _, err := specgen.ParseNetworkNamespace(cfg.Containers.NetNS)
+		return ns, err
 	}
 
-	return toReturn, nil
+	return toReturn, errors.Wrapf(define.ErrInvalidArg, "invalid namespace type %q passed", nsType)
 }
 
 // GenerateNamespaceOptions generates container creation options for all
diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go
index 7993777fb..8136c0993 100644
--- a/pkg/specgen/generate/oci.go
+++ b/pkg/specgen/generate/oci.go
@@ -267,6 +267,13 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 			return nil, err
 		}
 	} else {
+		// add default devices from containers.conf
+		for _, device := range rtc.Containers.Devices {
+			if err := DevicesFromPath(&g, device); err != nil {
+				return nil, err
+			}
+		}
+		// add default devices specified by caller
 		for _, device := range s.Devices {
 			if err := DevicesFromPath(&g, device.Path); err != nil {
 				return nil, err
@@ -297,7 +304,7 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 	}
 	configSpec := g.Config
 
-	if err := securityConfigureGenerator(s, &g, newImage); err != nil {
+	if err := securityConfigureGenerator(s, &g, newImage, rtc); err != nil {
 		return nil, err
 	}
 
diff --git a/pkg/specgen/generate/security.go b/pkg/specgen/generate/security.go
index e2da9e976..d2229b06f 100644
--- a/pkg/specgen/generate/security.go
+++ b/pkg/specgen/generate/security.go
@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	"github.com/containers/common/pkg/capabilities"
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/specgen"
@@ -55,76 +56,61 @@ func setLabelOpts(s *specgen.SpecGenerator, runtime *libpod.Runtime, pidConfig s
 	return nil
 }
 
-func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, newImage *image.Image) error {
+func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, newImage *image.Image, rtc *config.Config) error {
+	var (
+		caplist []string
+		err     error
+	)
 	// HANDLE CAPABILITIES
 	// NOTE: Must happen before SECCOMP
 	if s.Privileged {
 		g.SetupPrivileged(true)
-	}
-
-	useNotRoot := func(user string) bool {
-		if user == "" || user == "root" || user == "0" {
-			return false
+		caplist = capabilities.AllCapabilities()
+	} else {
+		caplist, err = rtc.Capabilities(s.User, s.CapAdd, s.CapDrop)
+		if err != nil {
+			return err
 		}
-		return true
-	}
-	configSpec := g.Config
-	var err error
-	var caplist []string
-	bounding := configSpec.Process.Capabilities.Bounding
-	if useNotRoot(s.User) {
-		configSpec.Process.Capabilities.Bounding = caplist
-	}
-	caplist, err = capabilities.MergeCapabilities(configSpec.Process.Capabilities.Bounding, s.CapAdd, s.CapDrop)
-	if err != nil {
-		return err
-	}
-	privCapsRequired := []string{}
 
-	// If the container image specifies an label with a
-	// capabilities.ContainerImageLabel then split the comma separated list
-	// of capabilities and record them.  This list indicates the only
-	// capabilities, required to run the container.
-	var capsRequiredRequested []string
-	for key, val := range s.Labels {
-		if util.StringInSlice(key, capabilities.ContainerImageLabels) {
-			capsRequiredRequested = strings.Split(val, ",")
+		privCapsRequired := []string{}
+
+		// If the container image specifies an label with a
+		// capabilities.ContainerImageLabel then split the comma separated list
+		// of capabilities and record them.  This list indicates the only
+		// capabilities, required to run the container.
+		var capsRequiredRequested []string
+		for key, val := range s.Labels {
+			if util.StringInSlice(key, capabilities.ContainerImageLabels) {
+				capsRequiredRequested = strings.Split(val, ",")
+			}
 		}
-	}
-	if !s.Privileged && len(capsRequiredRequested) > 0 {
+		if !s.Privileged && len(capsRequiredRequested) > 0 {
 
-		// Pass capRequiredRequested in CapAdd field to normalize capabilities names
-		capsRequired, err := capabilities.MergeCapabilities(nil, capsRequiredRequested, nil)
-		if err != nil {
-			logrus.Errorf("capabilities requested by user or image are not valid: %q", strings.Join(capsRequired, ","))
-		} else {
-			// Verify all capRequiered are in the capList
-			for _, cap := range capsRequired {
-				if !util.StringInSlice(cap, caplist) {
-					privCapsRequired = append(privCapsRequired, cap)
+			// Pass capRequiredRequested in CapAdd field to normalize capabilities names
+			capsRequired, err := capabilities.MergeCapabilities(nil, capsRequiredRequested, nil)
+			if err != nil {
+				logrus.Errorf("capabilities requested by user or image are not valid: %q", strings.Join(capsRequired, ","))
+			} else {
+				// Verify all capRequiered are in the capList
+				for _, cap := range capsRequired {
+					if !util.StringInSlice(cap, caplist) {
+						privCapsRequired = append(privCapsRequired, cap)
+					}
 				}
 			}
-		}
-		if len(privCapsRequired) == 0 {
-			caplist = capsRequired
-		} else {
-			logrus.Errorf("capabilities requested by user or image are not allowed by default: %q", strings.Join(privCapsRequired, ","))
+			if len(privCapsRequired) == 0 {
+				caplist = capsRequired
+			} else {
+				logrus.Errorf("capabilities requested by user or image are not allowed by default: %q", strings.Join(privCapsRequired, ","))
+			}
 		}
 	}
-
+	configSpec := g.Config
 	configSpec.Process.Capabilities.Bounding = caplist
 	configSpec.Process.Capabilities.Permitted = caplist
 	configSpec.Process.Capabilities.Inheritable = caplist
 	configSpec.Process.Capabilities.Effective = caplist
 	configSpec.Process.Capabilities.Ambient = caplist
-	if useNotRoot(s.User) {
-		caplist, err = capabilities.MergeCapabilities(bounding, s.CapAdd, s.CapDrop)
-		if err != nil {
-			return err
-		}
-	}
-	configSpec.Process.Capabilities.Bounding = caplist
-
 	// HANDLE SECCOMP
 	if s.SeccompProfilePath != "unconfined" {
 		seccompConfig, err := getSeccompConfig(s, configSpec, newImage)
diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go
index 396563267..11dee1986 100644
--- a/pkg/specgen/namespaces.go
+++ b/pkg/specgen/namespaces.go
@@ -3,6 +3,8 @@ package specgen
 import (
 	"strings"
 
+	"github.com/containers/libpod/pkg/cgroups"
+	"github.com/containers/libpod/pkg/rootless"
 	"github.com/pkg/errors"
 )
 
@@ -163,7 +165,7 @@ func ParseNamespace(ns string) (Namespace, error) {
 		toReturn.NSMode = FromPod
 	case ns == "host":
 		toReturn.NSMode = Host
-	case ns == "private":
+	case ns == "private", ns == "":
 		toReturn.NSMode = Private
 	case strings.HasPrefix(ns, "ns:"):
 		split := strings.SplitN(ns, ":", 2)
@@ -186,6 +188,31 @@ func ParseNamespace(ns string) (Namespace, error) {
 	return toReturn, nil
 }
 
+// ParseCgroupNamespace parses a cgroup namespace specification in string
+// form.
+func ParseCgroupNamespace(ns string) (Namespace, error) {
+	toReturn := Namespace{}
+	// Cgroup is host for v1, private for v2.
+	// We can't trust c/common for this, as it only assumes private.
+	cgroupsv2, err := cgroups.IsCgroup2UnifiedMode()
+	if err != nil {
+		return toReturn, err
+	}
+	if cgroupsv2 {
+		switch ns {
+		case "host":
+			toReturn.NSMode = Host
+		case "private", "":
+			toReturn.NSMode = Private
+		default:
+			return toReturn, errors.Errorf("unrecognized namespace mode %s passed", ns)
+		}
+	} else {
+		toReturn.NSMode = Host
+	}
+	return toReturn, nil
+}
+
 // ParseUserNamespace parses a user namespace specification in string
 // form.
 func ParseUserNamespace(ns string) (Namespace, error) {
@@ -205,6 +232,9 @@ func ParseUserNamespace(ns string) (Namespace, error) {
 	case ns == "keep-id":
 		toReturn.NSMode = KeepID
 		return toReturn, nil
+	case ns == "":
+		toReturn.NSMode = Host
+		return toReturn, nil
 	}
 	return ParseNamespace(ns)
 }
@@ -215,11 +245,18 @@ func ParseUserNamespace(ns string) (Namespace, error) {
 func ParseNetworkNamespace(ns string) (Namespace, []string, error) {
 	toReturn := Namespace{}
 	var cniNetworks []string
+	// Net defaults to Slirp on rootless
 	switch {
 	case ns == "slirp4netns":
 		toReturn.NSMode = Slirp
 	case ns == "pod":
 		toReturn.NSMode = FromPod
+	case ns == "":
+		if rootless.IsRootless() {
+			toReturn.NSMode = Slirp
+		} else {
+			toReturn.NSMode = Bridge
+		}
 	case ns == "bridge":
 		toReturn.NSMode = Bridge
 	case ns == "none":