Split up create config handling of namespaces and security

As it stands, createconfig is a huge struct. This works fine when the only caller is when we create a container with a fully created config. However, if we wish to share code for security and namespace configuration, a single large struct becomes unweildy, as well as difficult to configure with the single createConfigToOCISpec function.

This PR breaks up namespace and security configuration into their own structs, with the eventual goal of allowing the namespace/security fields to be configured by the pod create cli, and allow the infra container to share this with the pod's containers.

Signed-off-by: Peter Hunt <pehunt@redhat.com>

1 files changed, 149 insertions, 271 deletions

diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go
index e054b3b13..244a8d1cd 100644
--- a/pkg/spec/createconfig.go
+++ b/pkg/spec/createconfig.go
@@ -1,7 +1,6 @@
 package createconfig
 
 import (
-	"net"
 	"os"
 	"strconv"
 	"strings"
@@ -12,7 +11,6 @@ import (
 	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/namespaces"
 	"github.com/containers/storage"
-	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/docker/go-connections/nat"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
@@ -55,89 +53,126 @@ type CreateResourceConfig struct {
 	Ulimit            []string //ulimit
 }
 
-// CreateConfig is a pre OCI spec structure.  It represents user input from varlink or the CLI
-type CreateConfig struct {
-	Annotations        map[string]string
-	Args               []string
+// PidConfig configures the pid namespace for the container
+type PidConfig struct {
+	PidMode namespaces.PidMode //pid
+}
+
+// IpcConfig configures the ipc namespace for the container
+type IpcConfig struct {
+	IpcMode namespaces.IpcMode //ipc
+}
+
+// CgroupConfig configures the cgroup namespace for the container
+type CgroupConfig struct {
+	Cgroups      string
+	Cgroupns     string
+	CgroupParent string                // cgroup-parent
+	CgroupMode   namespaces.CgroupMode //cgroup
+}
+
+// UserConfig configures the user namespace for the container
+type UserConfig struct {
+	GroupAdd   []string // group-add
+	IDMappings *storage.IDMappingOptions
+	UsernsMode namespaces.UsernsMode //userns
+	User       string                //user
+}
+
+// UtsConfig configures the uts namespace for the container
+type UtsConfig struct {
+	UtsMode  namespaces.UTSMode //uts
+	NoHosts  bool
+	HostAdd  []string //add-host
+	Hostname string
+}
+
+// NetworkConfig configures the network namespace for the container
+type NetworkConfig struct {
+	DNSOpt       []string //dns-opt
+	DNSSearch    []string //dns-search
+	DNSServers   []string //dns
+	ExposedPorts map[nat.Port]struct{}
+	HTTPProxy    bool
+	IP6Address   string                 //ipv6
+	IPAddress    string                 //ip
+	LinkLocalIP  []string               // link-local-ip
+	MacAddress   string                 //mac-address
+	NetMode      namespaces.NetworkMode //net
+	Network      string                 //network
+	NetworkAlias []string               //network-alias
+	PortBindings nat.PortMap
+	Publish      []string //publish
+	PublishAll   bool     //publish-all
+}
+
+// SecurityConfig configures the security features for the container
+type SecurityConfig struct {
 	CapAdd             []string // cap-add
 	CapDrop            []string // cap-drop
-	CidFile            string
-	ConmonPidFile      string
-	Cgroupns           string
-	Cgroups            string
-	CgroupParent       string            // cgroup-parent
-	Command            []string          // Full command that will be used
-	UserCommand        []string          // User-entered command (or image CMD)
-	Detach             bool              // detach
-	Devices            []string          // device
-	DNSOpt             []string          //dns-opt
-	DNSSearch          []string          //dns-search
-	DNSServers         []string          //dns
-	Entrypoint         []string          //entrypoint
-	Env                map[string]string //env
-	ExposedPorts       map[nat.Port]struct{}
-	GroupAdd           []string // group-add
-	HealthCheck        *manifest.Schema2HealthConfig
-	NoHosts            bool
-	HostAdd            []string //add-host
-	Hostname           string   //hostname
-	HTTPProxy          bool
-	Init               bool   // init
-	InitPath           string //init-path
-	Image              string
-	ImageID            string
-	BuiltinImgVolumes  map[string]struct{} // volumes defined in the image config
-	IDMappings         *storage.IDMappingOptions
-	ImageVolumeType    string                 // how to handle the image volume, either bind, tmpfs, or ignore
-	Interactive        bool                   //interactive
-	IpcMode            namespaces.IpcMode     //ipc
-	IP6Address         string                 //ipv6
-	IPAddress          string                 //ip
-	Labels             map[string]string      //label
-	LinkLocalIP        []string               // link-local-ip
-	LogDriver          string                 // log-driver
-	LogDriverOpt       []string               // log-opt
-	MacAddress         string                 //mac-address
-	Name               string                 //name
-	NetMode            namespaces.NetworkMode //net
-	Network            string                 //network
-	NetworkAlias       []string               //network-alias
-	PidMode            namespaces.PidMode     //pid
-	Pod                string                 //pod
-	PodmanPath         string
-	CgroupMode         namespaces.CgroupMode //cgroup
-	PortBindings       nat.PortMap
-	Privileged         bool     //privileged
-	Publish            []string //publish
-	PublishAll         bool     //publish-all
-	Quiet              bool     //quiet
-	ReadOnlyRootfs     bool     //read-only
-	ReadOnlyTmpfs      bool     //read-only-tmpfs
-	Resources          CreateResourceConfig
-	RestartPolicy      string
-	Rm                 bool              //rm
-	StopSignal         syscall.Signal    // stop-signal
-	StopTimeout        uint              // stop-timeout
-	Sysctl             map[string]string //sysctl
-	Systemd            bool
-	Tmpfs              []string              // tmpfs
-	Tty                bool                  //tty
-	UsernsMode         namespaces.UsernsMode //userns
-	User               string                //user
-	UtsMode            namespaces.UTSMode    //uts
-	Mounts             []spec.Mount
-	MountsFlag         []string // mounts
-	NamedVolumes       []*libpod.ContainerNamedVolume
-	Volumes            []string //volume
-	VolumesFrom        []string
-	WorkDir            string   //workdir
 	LabelOpts          []string //SecurityOpts
 	NoNewPrivs         bool     //SecurityOpts
 	ApparmorProfile    string   //SecurityOpts
 	SeccompProfilePath string   //SecurityOpts
 	SecurityOpts       []string
-	Rootfs             string
-	Syslog             bool // Whether to enable syslog on exit commands
+	Privileged         bool              //privileged
+	ReadOnlyRootfs     bool              //read-only
+	ReadOnlyTmpfs      bool              //read-only-tmpfs
+	Sysctl             map[string]string //sysctl
+}
+
+// CreateConfig is a pre OCI spec structure.  It represents user input from varlink or the CLI
+type CreateConfig struct {
+	Annotations       map[string]string
+	Args              []string
+	CidFile           string
+	ConmonPidFile     string
+	Command           []string          // Full command that will be used
+	UserCommand       []string          // User-entered command (or image CMD)
+	Detach            bool              // detach
+	Devices           []string          // device
+	Entrypoint        []string          //entrypoint
+	Env               map[string]string //env
+	HealthCheck       *manifest.Schema2HealthConfig
+	Init              bool   // init
+	InitPath          string //init-path
+	Image             string
+	ImageID           string
+	BuiltinImgVolumes map[string]struct{} // volumes defined in the image config
+	ImageVolumeType   string              // how to handle the image volume, either bind, tmpfs, or ignore
+	Interactive       bool                //interactive
+	Labels            map[string]string   //label
+	LogDriver         string              // log-driver
+	LogDriverOpt      []string            // log-opt
+	Name              string              //name
+	PodmanPath        string
+	Pod               string //pod
+	Quiet             bool   //quiet
+	Resources         CreateResourceConfig
+	RestartPolicy     string
+	Rm                bool           //rm
+	StopSignal        syscall.Signal // stop-signal
+	StopTimeout       uint           // stop-timeout
+	Systemd           bool
+	Tmpfs             []string // tmpfs
+	Tty               bool     //tty
+	Mounts            []spec.Mount
+	MountsFlag        []string // mounts
+	NamedVolumes      []*libpod.ContainerNamedVolume
+	Volumes           []string //volume
+	VolumesFrom       []string
+	WorkDir           string //workdir
+	Rootfs            string
+	Security          SecurityConfig
+	Syslog            bool // Whether to enable syslog on exit commands
+
+	// Namespaces
+	Pid     PidConfig
+	Ipc     IpcConfig
+	Cgroup  CgroupConfig
+	User    UserConfig
+	Uts     UtsConfig
+	Network NetworkConfig
 }
 
 func u32Ptr(i int64) *uint32     { u := uint32(i); return &u }
@@ -199,7 +234,6 @@ func (c *CreateConfig) createExitCommand(runtime *libpod.Runtime) ([]string, err
 // GetContainerCreateOptions takes a CreateConfig and returns a slice of CtrCreateOptions
 func (c *CreateConfig) getContainerCreateOptions(runtime *libpod.Runtime, pod *libpod.Pod, mounts []spec.Mount, namedVolumes []*libpod.ContainerNamedVolume) ([]libpod.CtrCreateOption, error) {
 	var options []libpod.CtrCreateOption
-	var portBindings []ocicni.PortMapping
 	var err error
 
 	if c.Interactive {
@@ -216,15 +250,6 @@ func (c *CreateConfig) getContainerCreateOptions(runtime *libpod.Runtime, pod *l
 		logrus.Debugf("adding container to pod %s", c.Pod)
 		options = append(options, runtime.WithPod(pod))
 	}
-	if c.Cgroups == "disabled" {
-		options = append(options, libpod.WithNoCgroups())
-	}
-	if len(c.PortBindings) > 0 {
-		portBindings, err = c.CreatePortBindings()
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to create port bindings")
-		}
-	}
 
 	if len(mounts) != 0 || len(namedVolumes) != 0 {
 		destinations := []string{}
@@ -253,187 +278,72 @@ func (c *CreateConfig) getContainerCreateOptions(runtime *libpod.Runtime, pod *l
 	// does not have one
 	options = append(options, libpod.WithEntrypoint(c.Entrypoint))
 
-	networks := make([]string, 0)
-	userNetworks := c.NetMode.UserDefined()
-	if IsPod(userNetworks) {
-		userNetworks = ""
-	}
-	if userNetworks != "" {
-		for _, netName := range strings.Split(userNetworks, ",") {
-			if netName == "" {
-				return nil, errors.Wrapf(err, "container networks %q invalid", networks)
-			}
-			networks = append(networks, netName)
-		}
-	}
-
-	if c.NetMode.IsNS() {
-		ns := c.NetMode.NS()
-		if ns == "" {
-			return nil, errors.Errorf("invalid empty user-defined network namespace")
-		}
-		_, err := os.Stat(ns)
-		if err != nil {
-			return nil, err
-		}
-	} else if c.NetMode.IsContainer() {
-		connectedCtr, err := runtime.LookupContainer(c.NetMode.Container())
-		if err != nil {
-			return nil, errors.Wrapf(err, "container %q not found", c.NetMode.Container())
-		}
-		options = append(options, libpod.WithNetNSFrom(connectedCtr))
-	} else if !c.NetMode.IsHost() && !c.NetMode.IsNone() {
-		hasUserns := c.UsernsMode.IsContainer() || c.UsernsMode.IsNS() || len(c.IDMappings.UIDMap) > 0 || len(c.IDMappings.GIDMap) > 0
-		postConfigureNetNS := hasUserns && !c.UsernsMode.IsHost()
-		options = append(options, libpod.WithNetNS(portBindings, postConfigureNetNS, string(c.NetMode), networks))
-	}
-
-	if c.CgroupMode.IsNS() {
-		ns := c.CgroupMode.NS()
-		if ns == "" {
-			return nil, errors.Errorf("invalid empty user-defined network namespace")
-		}
-		_, err := os.Stat(ns)
-		if err != nil {
-			return nil, err
-		}
-	} else if c.CgroupMode.IsContainer() {
-		connectedCtr, err := runtime.LookupContainer(c.CgroupMode.Container())
-		if err != nil {
-			return nil, errors.Wrapf(err, "container %q not found", c.CgroupMode.Container())
-		}
-		options = append(options, libpod.WithCgroupNSFrom(connectedCtr))
-	}
+	// TODO: MNT, USER, CGROUP
+	options = append(options, libpod.WithStopSignal(c.StopSignal))
+	options = append(options, libpod.WithStopTimeout(c.StopTimeout))
 
-	if c.UsernsMode.IsNS() {
-		ns := c.UsernsMode.NS()
-		if ns == "" {
-			return nil, errors.Errorf("invalid empty user-defined user namespace")
-		}
-		_, err := os.Stat(ns)
-		if err != nil {
-			return nil, err
-		}
-		options = append(options, libpod.WithIDMappings(*c.IDMappings))
-	} else if c.UsernsMode.IsContainer() {
-		connectedCtr, err := runtime.LookupContainer(c.UsernsMode.Container())
-		if err != nil {
-			return nil, errors.Wrapf(err, "container %q not found", c.UsernsMode.Container())
-		}
-		options = append(options, libpod.WithUserNSFrom(connectedCtr))
-	} else {
-		options = append(options, libpod.WithIDMappings(*c.IDMappings))
+	logPath := getLoggingPath(c.LogDriverOpt)
+	if logPath != "" {
+		options = append(options, libpod.WithLogPath(logPath))
 	}
 
-	if c.PidMode.IsContainer() {
-		connectedCtr, err := runtime.LookupContainer(c.PidMode.Container())
-		if err != nil {
-			return nil, errors.Wrapf(err, "container %q not found", c.PidMode.Container())
-		}
-
-		options = append(options, libpod.WithPIDNSFrom(connectedCtr))
+	if c.LogDriver != "" {
+		options = append(options, libpod.WithLogDriver(c.LogDriver))
 	}
 
-	if c.IpcMode.IsContainer() {
-		connectedCtr, err := runtime.LookupContainer(c.IpcMode.Container())
-		if err != nil {
-			return nil, errors.Wrapf(err, "container %q not found", c.IpcMode.Container())
-		}
-
-		options = append(options, libpod.WithIPCNSFrom(connectedCtr))
+	secOpts, err := c.Security.ToCreateOptions()
+	if err != nil {
+		return nil, err
 	}
+	options = append(options, secOpts...)
 
-	if IsPod(string(c.UtsMode)) {
-		options = append(options, libpod.WithUTSNSFromPod(pod))
+	nsOpts, err := c.Cgroup.ToCreateOptions(runtime)
+	if err != nil {
+		return nil, err
 	}
-	if c.UtsMode.IsContainer() {
-		connectedCtr, err := runtime.LookupContainer(c.UtsMode.Container())
-		if err != nil {
-			return nil, errors.Wrapf(err, "container %q not found", c.UtsMode.Container())
-		}
+	options = append(options, nsOpts...)
 
-		options = append(options, libpod.WithUTSNSFrom(connectedCtr))
+	nsOpts, err = c.Ipc.ToCreateOptions(runtime)
+	if err != nil {
+		return nil, err
 	}
+	options = append(options, nsOpts...)
 
-	// TODO: MNT, USER, CGROUP
-	options = append(options, libpod.WithStopSignal(c.StopSignal))
-	options = append(options, libpod.WithStopTimeout(c.StopTimeout))
-	if len(c.DNSSearch) > 0 {
-		options = append(options, libpod.WithDNSSearch(c.DNSSearch))
-	}
-	if len(c.DNSServers) > 0 {
-		if len(c.DNSServers) == 1 && strings.ToLower(c.DNSServers[0]) == "none" {
-			options = append(options, libpod.WithUseImageResolvConf())
-		} else {
-			options = append(options, libpod.WithDNS(c.DNSServers))
-		}
-	}
-	if len(c.DNSOpt) > 0 {
-		options = append(options, libpod.WithDNSOption(c.DNSOpt))
-	}
-	if c.NoHosts {
-		options = append(options, libpod.WithUseImageHosts())
-	}
-	if len(c.HostAdd) > 0 && !c.NoHosts {
-		options = append(options, libpod.WithHosts(c.HostAdd))
-	}
-	logPath := getLoggingPath(c.LogDriverOpt)
-	if logPath != "" {
-		options = append(options, libpod.WithLogPath(logPath))
+	nsOpts, err = c.Pid.ToCreateOptions(runtime)
+	if err != nil {
+		return nil, err
 	}
+	options = append(options, nsOpts...)
 
-	if c.LogDriver != "" {
-		options = append(options, libpod.WithLogDriver(c.LogDriver))
+	nsOpts, err = c.Network.ToCreateOptions(runtime, &c.User)
+	if err != nil {
+		return nil, err
 	}
+	options = append(options, nsOpts...)
 
-	if c.IPAddress != "" {
-		ip := net.ParseIP(c.IPAddress)
-		if ip == nil {
-			return nil, errors.Wrapf(define.ErrInvalidArg, "cannot parse %s as IP address", c.IPAddress)
-		} else if ip.To4() == nil {
-			return nil, errors.Wrapf(define.ErrInvalidArg, "%s is not an IPv4 address", c.IPAddress)
-		}
-		options = append(options, libpod.WithStaticIP(ip))
+	nsOpts, err = c.Uts.ToCreateOptions(runtime, pod)
+	if err != nil {
+		return nil, err
 	}
+	options = append(options, nsOpts...)
 
-	if c.MacAddress != "" {
-		mac, err := net.ParseMAC(c.MacAddress)
-		if err != nil {
-			return nil, errors.Wrapf(define.ErrInvalidArg, "cannot parse %s as MAC address: %v", c.MacAddress, err)
-		}
-		options = append(options, libpod.WithStaticMAC(mac))
+	nsOpts, err = c.User.ToCreateOptions(runtime)
+	if err != nil {
+		return nil, err
 	}
-
-	options = append(options, libpod.WithPrivileged(c.Privileged))
+	options = append(options, nsOpts...)
 
 	useImageVolumes := c.ImageVolumeType == TypeBind
 	// Gather up the options for NewContainer which consist of With... funcs
 	options = append(options, libpod.WithRootFSFromImage(c.ImageID, c.Image, useImageVolumes))
-	options = append(options, libpod.WithSecLabels(c.LabelOpts))
 	options = append(options, libpod.WithConmonPidFile(c.ConmonPidFile))
 	options = append(options, libpod.WithLabels(c.Labels))
-	options = append(options, libpod.WithUser(c.User))
-	if c.IpcMode.IsHost() {
-		options = append(options, libpod.WithShmDir("/dev/shm"))
-
-	} else if c.IpcMode.IsContainer() {
-		ctr, err := runtime.LookupContainer(c.IpcMode.Container())
-		if err != nil {
-			return nil, errors.Wrapf(err, "container %q not found", c.IpcMode.Container())
-		}
-		options = append(options, libpod.WithShmDir(ctr.ShmDir()))
-	}
 	options = append(options, libpod.WithShmSize(c.Resources.ShmSize))
-	options = append(options, libpod.WithGroups(c.GroupAdd))
 	if c.Rootfs != "" {
 		options = append(options, libpod.WithRootFS(c.Rootfs))
 	}
 	// Default used if not overridden on command line
 
-	if c.CgroupParent != "" {
-		options = append(options, libpod.WithCgroupParent(c.CgroupParent))
-	}
-
 	if c.RestartPolicy != "" {
 		if c.RestartPolicy == "unless-stopped" {
 			return nil, errors.Wrapf(define.ErrInvalidArg, "the unless-stopped restart policy is not supported")
@@ -467,38 +377,6 @@ func (c *CreateConfig) getContainerCreateOptions(runtime *libpod.Runtime, pod *l
 	return options, nil
 }
 
-// CreatePortBindings iterates ports mappings and exposed ports into a format CNI understands
-func (c *CreateConfig) CreatePortBindings() ([]ocicni.PortMapping, error) {
-	return NatToOCIPortBindings(c.PortBindings)
-}
-
-// NatToOCIPortBindings iterates a nat.portmap slice and creates []ocicni portmapping slice
-func NatToOCIPortBindings(ports nat.PortMap) ([]ocicni.PortMapping, error) {
-	var portBindings []ocicni.PortMapping
-	for containerPb, hostPb := range ports {
-		var pm ocicni.PortMapping
-		pm.ContainerPort = int32(containerPb.Int())
-		for _, i := range hostPb {
-			var hostPort int
-			var err error
-			pm.HostIP = i.HostIP
-			if i.HostPort == "" {
-				hostPort = containerPb.Int()
-			} else {
-				hostPort, err = strconv.Atoi(i.HostPort)
-				if err != nil {
-					return nil, errors.Wrapf(err, "unable to convert host port to integer")
-				}
-			}
-
-			pm.HostPort = int32(hostPort)
-			pm.Protocol = containerPb.Proto()
-			portBindings = append(portBindings, pm)
-		}
-	}
-	return portBindings, nil
-}
-
 // AddPrivilegedDevices iterates through host devices and adds all
 // host devices to the spec
 func (c *CreateConfig) AddPrivilegedDevices(g *generate.Generator) error {