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 <[email protected]>
containers · Nov 8, 2019 · dcf3c74 · dcf3c74
1 parent 3463a71
commit dcf3c74
Show file tree

Hide file tree

Showing 12 changed files with 907 additions and 710 deletions.
diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go
@@ -26,7 +26,6 @@ import (
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/go-connections/nat"
 	"github.com/docker/go-units"
-	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/opentracing/opentracing-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -195,72 +194,6 @@ func CreateContainer(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 	return ctr, createConfig, nil
 }
 
-func parseSecurityOpt(config *cc.CreateConfig, securityOpts []string, runtime *libpod.Runtime) error {
-	var (
-		labelOpts []string
-	)
-
-	if config.PidMode.IsHost() {
-		labelOpts = append(labelOpts, label.DisableSecOpt()...)
-	} else if config.PidMode.IsContainer() {
-		ctr, err := runtime.LookupContainer(config.PidMode.Container())
-		if err != nil {
-			return errors.Wrapf(err, "container %q not found", config.PidMode.Container())
-		}
-		secopts, err := label.DupSecOpt(ctr.ProcessLabel())
-		if err != nil {
-			return errors.Wrapf(err, "failed to duplicate label %q ", ctr.ProcessLabel())
-		}
-		labelOpts = append(labelOpts, secopts...)
-	}
-
-	if config.IpcMode.IsHost() {
-		labelOpts = append(labelOpts, label.DisableSecOpt()...)
-	} else if config.IpcMode.IsContainer() {
-		ctr, err := runtime.LookupContainer(config.IpcMode.Container())
-		if err != nil {
-			return errors.Wrapf(err, "container %q not found", config.IpcMode.Container())
-		}
-		secopts, err := label.DupSecOpt(ctr.ProcessLabel())
-		if err != nil {
-			return errors.Wrapf(err, "failed to duplicate label %q ", ctr.ProcessLabel())
-		}
-		labelOpts = append(labelOpts, secopts...)
-	}
-
-	for _, opt := range securityOpts {
-		if opt == "no-new-privileges" {
-			config.NoNewPrivs = true
-		} else {
-			con := strings.SplitN(opt, "=", 2)
-			if len(con) != 2 {
-				return fmt.Errorf("invalid --security-opt 1: %q", opt)
-			}
-
-			switch con[0] {
-			case "label":
-				labelOpts = append(labelOpts, con[1])
-			case "apparmor":
-				config.ApparmorProfile = con[1]
-			case "seccomp":
-				config.SeccompProfilePath = con[1]
-			default:
-				return fmt.Errorf("invalid --security-opt 2: %q", opt)
-			}
-		}
-	}
-
-	if config.SeccompProfilePath == "" {
-		var err error
-		config.SeccompProfilePath, err = libpod.DefaultSeccompPath()
-		if err != nil {
-			return err
-		}
-	}
-	config.LabelOpts = labelOpts
-	return nil
-}
-
 func configureEntrypoint(c *GenericCLIResults, data *inspect.ImageData) []string {
 	entrypoint := []string{}
 	if c.IsSet("entrypoint") {
@@ -348,11 +281,6 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		rootfs = c.InputArgs[0]
 	}
 
-	sysctl, err := validateSysctl(c.StringSlice("sysctl"))
-	if err != nil {
-		return nil, errors.Wrapf(err, "invalid value for sysctl")
-	}
-
 	if c.String("memory") != "" {
 		memoryLimit, err = units.RAMInBytes(c.String("memory"))
 		if err != nil {
@@ -691,61 +619,96 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		pidsLimit = 0
 	}
 
+	pid := &cc.PidConfig{
+		PidMode: pidMode,
+	}
+	ipc := &cc.IpcConfig{
+		IpcMode: ipcMode,
+	}
+
+	cgroup := &cc.CgroupConfig{
+		Cgroups:      c.String("cgroups"),
+		Cgroupns:     c.String("cgroupns"),
+		CgroupParent: c.String("cgroup-parent"),
+		CgroupMode:   cgroupMode,
+	}
+
+	userns := &cc.UserConfig{
+		GroupAdd:   c.StringSlice("group-add"),
+		IDMappings: idmappings,
+		UsernsMode: usernsMode,
+		User:       user,
+	}
+
+	uts := &cc.UtsConfig{
+		UtsMode:  utsMode,
+		NoHosts:  c.Bool("no-hosts"),
+		HostAdd:  c.StringSlice("add-host"),
+		Hostname: c.String("hostname"),
+	}
+
+	net := &cc.NetworkConfig{
+		DNSOpt:       c.StringSlice("dns-opt"),
+		DNSSearch:    c.StringSlice("dns-search"),
+		DNSServers:   c.StringSlice("dns"),
+		HTTPProxy:    c.Bool("http-proxy"),
+		MacAddress:   c.String("mac-address"),
+		Network:      network,
+		NetMode:      netMode,
+		IPAddress:    c.String("ip"),
+		Publish:      c.StringSlice("publish"),
+		PublishAll:   c.Bool("publish-all"),
+		PortBindings: portBindings,
+	}
+
+	sysctl, err := validateSysctl(c.StringSlice("sysctl"))
+	if err != nil {
+		return nil, errors.Wrapf(err, "invalid value for sysctl")
+	}
+
+	secConfig := &cc.SecurityConfig{
+		CapAdd:         c.StringSlice("cap-add"),
+		CapDrop:        c.StringSlice("cap-drop"),
+		Privileged:     c.Bool("privileged"),
+		ReadOnlyRootfs: c.Bool("read-only"),
+		ReadOnlyTmpfs:  c.Bool("read-only-tmpfs"),
+		Sysctl:         sysctl,
+	}
+
+	if err := secConfig.SetLabelOpts(runtime, pid, ipc); err != nil {
+		return nil, err
+	}
+	if err := secConfig.SetSecurityOpts(runtime, c.StringArray("security-opt")); err != nil {
+		return nil, err
+	}
+
 	config := &cc.CreateConfig{
 		Annotations:       annotations,
 		BuiltinImgVolumes: ImageVolumes,
 		ConmonPidFile:     c.String("conmon-pidfile"),
 		ImageVolumeType:   c.String("image-volume"),
-		CapAdd:            c.StringSlice("cap-add"),
-		CapDrop:           c.StringSlice("cap-drop"),
 		CidFile:           c.String("cidfile"),
-		Cgroupns:          c.String("cgroupns"),
-		Cgroups:           c.String("cgroups"),
-		CgroupParent:      c.String("cgroup-parent"),
 		Command:           command,
 		UserCommand:       userCommand,
 		Detach:            c.Bool("detach"),
 		Devices:           c.StringSlice("device"),
-		DNSOpt:            c.StringSlice("dns-opt"),
-		DNSSearch:         c.StringSlice("dns-search"),
-		DNSServers:        c.StringSlice("dns"),
 		Entrypoint:        entrypoint,
 		Env:               env,
 		// ExposedPorts:   ports,
-		GroupAdd:    c.StringSlice("group-add"),
-		Hostname:    c.String("hostname"),
-		HostAdd:     c.StringSlice("add-host"),
-		HTTPProxy:   c.Bool("http-proxy"),
-		NoHosts:     c.Bool("no-hosts"),
-		IDMappings:  idmappings,
 		Init:        c.Bool("init"),
 		InitPath:    c.String("init-path"),
 		Image:       imageName,
 		ImageID:     imageID,
 		Interactive: c.Bool("interactive"),
 		// IP6Address:     c.String("ipv6"), // Not implemented yet - needs CNI support for static v6
-		IPAddress: c.String("ip"),
-		Labels:    labels,
+		Labels: labels,
 		// LinkLocalIP:    c.StringSlice("link-local-ip"), // Not implemented yet
 		LogDriver:    logDriver,
 		LogDriverOpt: c.StringSlice("log-opt"),
-		MacAddress:   c.String("mac-address"),
 		Name:         c.String("name"),
-		Network:      network,
 		// NetworkAlias:   c.StringSlice("network-alias"), // Not implemented - does this make sense in Podman?
-		IpcMode:        ipcMode,
-		NetMode:        netMode,
-		UtsMode:        utsMode,
-		PidMode:        pidMode,
-		CgroupMode:     cgroupMode,
-		Pod:            podName,
-		Privileged:     c.Bool("privileged"),
-		Publish:        c.StringSlice("publish"),
-		PublishAll:     c.Bool("publish-all"),
-		PortBindings:   portBindings,
-		Quiet:          c.Bool("quiet"),
-		ReadOnlyRootfs: c.Bool("read-only"),
-		ReadOnlyTmpfs:  c.Bool("read-only-tmpfs"),
+		Pod:   podName,
+		Quiet: c.Bool("quiet"),
 		Resources: cc.CreateResourceConfig{
 			BlkioWeight:       blkioWeight,
 			BlkioWeightDevice: c.StringSlice("blkio-weight-device"),
@@ -774,30 +737,27 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		},
 		RestartPolicy: c.String("restart"),
 		Rm:            c.Bool("rm"),
+		Security:      *secConfig,
 		StopSignal:    stopSignal,
 		StopTimeout:   c.Uint("stop-timeout"),
-		Sysctl:        sysctl,
 		Systemd:       systemd,
 		Tmpfs:         c.StringArray("tmpfs"),
 		Tty:           tty,
-		User:          user,
-		UsernsMode:    usernsMode,
 		MountsFlag:    c.StringArray("mount"),
 		Volumes:       c.StringArray("volume"),
 		WorkDir:       workDir,
 		Rootfs:        rootfs,
 		VolumesFrom:   c.StringSlice("volumes-from"),
 		Syslog:        c.Bool("syslog"),
-	}
 
-	if config.Privileged {
-		config.LabelOpts = label.DisableSecOpt()
-	} else {
-		if err := parseSecurityOpt(config, c.StringArray("security-opt"), runtime); err != nil {
-			return nil, err
-		}
+		Pid:     *pid,
+		Ipc:     *ipc,
+		Cgroup:  *cgroup,
+		User:    *userns,
+		Uts:     *uts,
+		Network: *net,
 	}
-	config.SecurityOpts = c.StringArray("security-opt")
+
 	warnings, err := verifyContainerResources(config, false)
 	if err != nil {
 		return nil, err