Merge pull request #5363 from mheon/add_ctr_validate

Add validate() for containers

libpod/container_validate.go

@@ -0,0 +1,99 @@
+package libpod
+
+import (
+	"github.com/containers/libpod/libpod/define"
+	"github.com/containers/libpod/pkg/rootless"
+	spec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+)
+
+// Validate that the configuration of a container is valid.
+func (c *Container) validate() error {
+	imageIDSet := c.config.RootfsImageID != ""
+	imageNameSet := c.config.RootfsImageName != ""
+	rootfsSet := c.config.Rootfs != ""
+
+	// If one of RootfsImageIDor RootfsImageName are set, both must be set.
+	if (imageIDSet || imageNameSet) && !(imageIDSet && imageNameSet) {
+		return errors.Wrapf(define.ErrInvalidArg, "both RootfsImageName and RootfsImageID must be set if either is set")
+	}
+
+	// Cannot set RootfsImageID and Rootfs at the same time
+	if imageIDSet && rootfsSet {
+		return errors.Wrapf(define.ErrInvalidArg, "cannot set both an image ID and rootfs for a container")
+	}
+
+	// Must set at least one of RootfsImageID or Rootfs
+	if !(imageIDSet || rootfsSet) {
+		return errors.Wrapf(define.ErrInvalidArg, "must set root filesystem source to either image or rootfs")
+	}
+
+	// Cannot make a network namespace if we are joining another container's
+	// network namespace
+	if c.config.CreateNetNS && c.config.NetNsCtr != "" {
+		return errors.Wrapf(define.ErrInvalidArg, "cannot both create a network namespace and join another container's network namespace")
+	}
+
+	// Not creating cgroups has a number of requirements, mostly related to
+	// the PID namespace.
+	if c.config.NoCgroups || c.config.CgroupsMode == "disabled" {
+		if c.config.PIDNsCtr != "" {
+			return errors.Wrapf(define.ErrInvalidArg, "cannot join another container's PID namespace if not creating cgroups")
+		}
+
+		if c.config.CgroupParent != "" {
+			return errors.Wrapf(define.ErrInvalidArg, "cannot set cgroup parent if not creating cgroups")
+		}
+
+		// Ensure we have a PID namespace
+		if c.config.Spec.Linux == nil {
+			return errors.Wrapf(define.ErrInvalidArg, "must provide Linux namespace configuration in OCI spec when using NoCgroups")
+		}
+		foundPid := false
+		for _, ns := range c.config.Spec.Linux.Namespaces {
+			if ns.Type == spec.PIDNamespace {
+				foundPid = true
+				if ns.Path != "" {
+					return errors.Wrapf(define.ErrInvalidArg, "containers not creating CGroups must create a private PID namespace - cannot use another")
+				}
+				break
+			}
+		}
+		if !foundPid {
+			return errors.Wrapf(define.ErrInvalidArg, "containers not creating CGroups must create a private PID namespace")
+		}
+	}
+
+	// Rootless has some requirements, compared to networks.
+	if rootless.IsRootless() {
+		if len(c.config.Networks) > 0 {
+			return errors.Wrapf(define.ErrInvalidArg, "cannot join CNI networks if running rootless")
+		}
+
+		// TODO: Should we make sure network mode is set to Slirp if set
+		// at all?
+	}
+
+	// Can only set static IP or MAC is creating a network namespace.
+	if !c.config.CreateNetNS && (c.config.StaticIP != nil || c.config.StaticMAC != nil) {
+		return errors.Wrapf(define.ErrInvalidArg, "cannot set static IP or MAC address if not creating a network namespace")
+	}
+
+	// Cannot set static IP or MAC if joining >1 CNI network.
+	if len(c.config.Networks) > 1 && (c.config.StaticIP != nil || c.config.StaticMAC != nil) {
+		return errors.Wrapf(define.ErrInvalidArg, "cannot set static IP or MAC address if joining more than one CNI network")
+	}
+
+	// Using image resolv.conf conflicts with various DNS settings.
+	if c.config.UseImageResolvConf &&
+		(len(c.config.DNSSearch) > 0 || len(c.config.DNSServer) > 0 ||
+			len(c.config.DNSOption) > 0) {
+		return errors.Wrapf(define.ErrInvalidArg, "cannot configure DNS options if using image's resolv.conf")
+	}
+
+	if c.config.UseImageHosts && len(c.config.HostAdd) > 0 {
+		return errors.Wrapf(define.ErrInvalidArg, "cannot add to /etc/hosts if using image's /etc/hosts")
+	}
+
+	return nil
+}

libpod/options.go

@@ -599,13 +599,6 @@ func WithRootFSFromImage(imageID string, imageName string) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if ctr.config.RootfsImageID != "" || ctr.config.RootfsImageName != "" {
-			return errors.Wrapf(define.ErrInvalidArg, "container already configured with root filesystem")
-		}
-		if ctr.config.Rootfs != "" {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot set both an image ID and a rootfs for a container")
-		}
-
 		ctr.config.RootfsImageID = imageID
 		ctr.config.RootfsImageName = imageName
 
@@ -815,10 +808,6 @@ func WithNetNSFrom(nsCtr *Container) CtrCreateOption {
 			return err
 		}
 
-		if ctr.config.CreateNetNS {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot join another container's net ns as we are making a new net ns")
-		}
-
 		ctr.config.NetNsCtr = nsCtr.ID()
 
 		return nil
@@ -839,10 +828,6 @@ func WithPIDNSFrom(nsCtr *Container) CtrCreateOption {
 			return err
 		}
 
-		if ctr.config.NoCgroups {
-			return errors.Wrapf(define.ErrInvalidArg, "container has disabled creation of CGroups, which is incompatible with sharing a PID namespace")
-		}
-
 		ctr.config.PIDNsCtr = nsCtr.ID()
 
 		return nil
@@ -921,16 +906,8 @@ func WithDependencyCtrs(ctrs []*Container) CtrCreateOption {
 		deps := make([]string, 0, len(ctrs))
 
 		for _, dep := range ctrs {
-			if !dep.valid {
+			if err := checkDependencyContainer(dep, ctr); err != nil {
-				return errors.Wrapf(define.ErrCtrRemoved, "container %s is not valid", dep.ID())
+				return err
-			}
-
-			if dep.ID() == ctr.ID() {
-				return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
-			}
-
-			if ctr.config.Pod != "" && dep.config.Pod != ctr.config.Pod {
-				return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, dep.ID())
 			}
 
 			deps = append(deps, dep.ID())
@@ -952,20 +929,6 @@ func WithNetNS(portMappings []ocicni.PortMapping, postConfigureNetNS bool, netmo
 			return define.ErrCtrFinalized
 		}
 
-		if rootless.IsRootless() {
-			if len(networks) > 0 {
-				return errors.Wrapf(define.ErrInvalidArg, "cannot use CNI networks with rootless containers")
-			}
-		}
-
-		if len(networks) > 1 && (ctr.config.StaticIP != nil || ctr.config.StaticMAC != nil) {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot join more than one CNI network if configuring a static IP or MAC address")
-		}
-
-		if ctr.config.NetNsCtr != "" {
-			return errors.Wrapf(define.ErrInvalidArg, "container is already set to join another container's net ns, cannot create a new net ns")
-		}
-
 		ctr.config.PostConfigureNetNS = postConfigureNetNS
 		ctr.config.NetMode = namespaces.NetworkMode(netmode)
 		ctr.config.CreateNetNS = true
@@ -988,14 +951,6 @@ func WithStaticIP(ip net.IP) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !ctr.config.CreateNetNS {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot set a static IP if the container is not creating a network namespace")
-		}
-
-		if len(ctr.config.Networks) > 1 {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot set a static IP if joining more than 1 CNI network")
-		}
-
 		ctr.config.StaticIP = ip
 
 		return nil
@@ -1013,14 +968,6 @@ func WithStaticMAC(mac net.HardwareAddr) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !ctr.config.CreateNetNS {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot set a static MAC if the container is not creating a network namespace")
-		}
-
-		if len(ctr.config.Networks) > 1 {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot set a static MAC if joining more than 1 CNI network")
-		}
-
 		ctr.config.StaticMAC = mac
 
 		return nil
@@ -1114,10 +1061,6 @@ func WithCgroupParent(parent string) CtrCreateOption {
 			return errors.Wrapf(define.ErrInvalidArg, "cgroup parent cannot be empty")
 		}
 
-		if ctr.config.NoCgroups {
-			return errors.Wrapf(define.ErrInvalidArg, "CgroupParent conflicts with NoCgroups")
-		}
-
 		ctr.config.CgroupParent = parent
 
 		return nil
@@ -1130,9 +1073,6 @@ func WithDNSSearch(searchDomains []string) CtrCreateOption {
 		if ctr.valid {
 			return define.ErrCtrFinalized
 		}
-		if ctr.config.UseImageResolvConf {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot add DNS search domains if container will not create /etc/resolv.conf")
-		}
 		ctr.config.DNSSearch = searchDomains
 		return nil
 	}
@@ -1144,9 +1084,6 @@ func WithDNS(dnsServers []string) CtrCreateOption {
 		if ctr.valid {
 			return define.ErrCtrFinalized
 		}
-		if ctr.config.UseImageResolvConf {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot add DNS servers if container will not create /etc/resolv.conf")
-		}
 		var dns []net.IP
 		for _, i := range dnsServers {
 			result := net.ParseIP(i)
@@ -1166,9 +1103,6 @@ func WithDNSOption(dnsOptions []string) CtrCreateOption {
 		if ctr.valid {
 			return define.ErrCtrFinalized
 		}
-		if ctr.config.UseImageResolvConf {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot add DNS options if container will not create /etc/resolv.conf")
-		}
 		ctr.config.DNSOption = dnsOptions
 		return nil
 	}
@@ -1181,10 +1115,6 @@ func WithHosts(hosts []string) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if ctr.config.UseImageHosts {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot add hosts if container will not create /etc/hosts")
-		}
-
 		ctr.config.HostAdd = hosts
 		return nil
 	}
@@ -1282,9 +1212,6 @@ func WithRootFS(rootfs string) CtrCreateOption {
 		if _, err := os.Stat(rootfs); err != nil {
 			return errors.Wrapf(err, "error checking path %q", rootfs)
 		}
-		if ctr.config.RootfsImageID != "" {
-			return errors.Wrapf(define.ErrInvalidArg, "cannot set both an image ID and a rootfs for a container")
-		}
 		ctr.config.Rootfs = rootfs
 		return nil
 	}
@@ -1314,12 +1241,6 @@ func WithUseImageResolvConf() CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if len(ctr.config.DNSServer) != 0 ||
-			len(ctr.config.DNSSearch) != 0 ||
-			len(ctr.config.DNSOption) != 0 {
-			return errors.Wrapf(define.ErrInvalidArg, "not creating resolv.conf conflicts with DNS options")
-		}
-
 		ctr.config.UseImageResolvConf = true
 
 		return nil
@@ -1334,10 +1255,6 @@ func WithUseImageHosts() CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if len(ctr.config.HostAdd) != 0 {
-			return errors.Wrapf(define.ErrInvalidArg, "not creating /etc/hosts conflicts with adding to the hosts file")
-		}
-
 		ctr.config.UseImageHosts = true
 
 		return nil

libpod/runtime_ctr.go

@@ -133,7 +133,12 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options ..
 	return r.setupContainer(ctx, ctr)
 }
 
-func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Container, err error) {
+func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Container, err error) {
+	// Validate the container
+	if err := ctr.validate(); err != nil {
+		return nil, err
+	}
+
 	// Allocate a lock for the container
 	lock, err := r.lockManager.AllocateLock()
 	if err != nil {
@@ -190,27 +195,6 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Contai
 		ctr.config.Name = name
 	}
 
-	// If CGroups are disabled, we MUST create a PID namespace.
-	// Otherwise, the OCI runtime won't be able to stop our container.
-	if ctr.config.NoCgroups {
-		if ctr.config.Spec.Linux == nil {
-			return nil, errors.Wrapf(define.ErrInvalidArg, "must provide Linux namespace configuration in OCI spec when using NoCgroups")
-		}
-		foundPid := false
-		for _, ns := range ctr.config.Spec.Linux.Namespaces {
-			if ns.Type == spec.PIDNamespace {
-				foundPid = true
-				if ns.Path != "" {
-					return nil, errors.Wrapf(define.ErrInvalidArg, "containers not creating CGroups must create a private PID namespace - cannot use another")
-				}
-				break
-			}
-		}
-		if !foundPid {
-			return nil, errors.Wrapf(define.ErrInvalidArg, "containers not creating CGroups must create a private PID namespace")
-		}
-	}
-
 	// Check CGroup parent sanity, and set it if it was not set.
 	// Only if we're actually configuring CGroups.
 	if !ctr.config.NoCgroups {