From 0ee2f35e43b2d78f1331513f3897b748b5d64aeb Mon Sep 17 00:00:00 2001 From: Justin Cormack Date: Tue, 3 Apr 2018 14:30:47 +0100 Subject: [PATCH] Consistently add empty types where they are nil in spec In a few places we check for nil types when modifying a spec, but in many cases we do not so we could get a panic if the passed in type was not filled. Because the generated spec is filled we will not notice this but users may get unexpected panics. Signed-off-by: Justin Cormack --- oci/spec_opts.go | 10 +++++++++ oci/spec_opts_unix.go | 48 +++++++++++++++++++++++++++++++--------- oci/spec_opts_windows.go | 3 +++ 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/oci/spec_opts.go b/oci/spec_opts.go index 53741ea80..675be3970 100644 --- a/oci/spec_opts.go +++ b/oci/spec_opts.go @@ -27,9 +27,17 @@ import ( // SpecOpts sets spec specific information to a newly generated OCI spec type SpecOpts func(context.Context, Client, *containers.Container, *specs.Spec) error +// setProcess sets Process to empty if unset +func setProcess(s *specs.Spec) { + if s.Process == nil { + s.Process = &specs.Process{} + } +} + // WithProcessArgs replaces the args on the generated spec func WithProcessArgs(args ...string) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setProcess(s) s.Process.Args = args return nil } @@ -38,6 +46,7 @@ func WithProcessArgs(args ...string) SpecOpts { // WithProcessCwd replaces the current working directory on the generated spec func WithProcessCwd(cwd string) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setProcess(s) s.Process.Cwd = cwd return nil } @@ -55,6 +64,7 @@ func WithHostname(name string) SpecOpts { func WithEnv(environmentVariables []string) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { if len(environmentVariables) > 0 { + setProcess(s) s.Process.Env = replaceOrAppendEnvValues(s.Process.Env, environmentVariables) } return nil diff --git a/oci/spec_opts_unix.go b/oci/spec_opts_unix.go index 514b357dd..27d78178b 100644 --- a/oci/spec_opts_unix.go +++ b/oci/spec_opts_unix.go @@ -43,14 +43,38 @@ import ( // WithTTY sets the information on the spec as well as the environment variables for // using a TTY func WithTTY(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setProcess(s) s.Process.Terminal = true s.Process.Env = append(s.Process.Env, "TERM=xterm") return nil } +// setRoot sets Root to empty if unset +func setRoot(s *specs.Spec) { + if s.Root == nil { + s.Root = &specs.Root{} + } +} + +// setLinux sets Linux to empty if unset +func setLinux(s *specs.Spec) { + if s.Linux == nil { + s.Linux = &specs.Linux{} + } +} + +// setCapabilities sets Linux Capabilities to empty if unset +func setCapabilities(s *specs.Spec) { + setProcess(s) + if s.Process.Capabilities == nil { + s.Process.Capabilities = &specs.LinuxCapabilities{} + } +} + // WithHostNamespace allows a task to run inside the host's linux namespace func WithHostNamespace(ns specs.LinuxNamespaceType) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setLinux(s) for i, n := range s.Linux.Namespaces { if n.Type == ns { s.Linux.Namespaces = append(s.Linux.Namespaces[:i], s.Linux.Namespaces[i+1:]...) @@ -65,6 +89,7 @@ func WithHostNamespace(ns specs.LinuxNamespaceType) SpecOpts { // spec, the existing namespace is replaced by the one provided. func WithLinuxNamespace(ns specs.LinuxNamespace) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setLinux(s) for i, n := range s.Linux.Namespaces { if n.Type == ns.Type { before := s.Linux.Namespaces[:i] @@ -105,10 +130,7 @@ func WithImageConfig(image Image) SpecOpts { return fmt.Errorf("unknown image config media type %s", ic.MediaType) } - if s.Process == nil { - s.Process = &specs.Process{} - } - + setProcess(s) s.Process.Env = append(s.Process.Env, config.Env...) cmd := config.Cmd s.Process.Args = append(config.Entrypoint, cmd...) @@ -127,9 +149,7 @@ func WithImageConfig(image Image) SpecOpts { // WithRootFSPath specifies unmanaged rootfs path. func WithRootFSPath(path string) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { - if s.Root == nil { - s.Root = &specs.Root{} - } + setRoot(s) s.Root.Path = path // Entrypoint is not set here (it's up to caller) return nil @@ -139,9 +159,7 @@ func WithRootFSPath(path string) SpecOpts { // WithRootFSReadonly sets specs.Root.Readonly to true func WithRootFSReadonly() SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { - if s.Root == nil { - s.Root = &specs.Root{} - } + setRoot(s) s.Root.Readonly = true return nil } @@ -149,6 +167,7 @@ func WithRootFSReadonly() SpecOpts { // WithNoNewPrivileges sets no_new_privileges on the process for the container func WithNoNewPrivileges(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setProcess(s) s.Process.NoNewPrivileges = true return nil } @@ -191,6 +210,7 @@ func WithHostLocaltime(_ context.Context, _ Client, _ *containers.Container, s * func WithUserNamespace(container, host, size uint32) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { var hasUserns bool + setLinux(s) for _, ns := range s.Linux.Namespaces { if ns.Type == specs.UserNamespace { hasUserns = true @@ -216,6 +236,7 @@ func WithUserNamespace(container, host, size uint32) SpecOpts { // WithCgroup sets the container's cgroup path func WithCgroup(path string) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setLinux(s) s.Linux.CgroupsPath = path return nil } @@ -229,6 +250,7 @@ func WithNamespacedCgroup() SpecOpts { if err != nil { return err } + setLinux(s) s.Linux.CgroupsPath = filepath.Join("/", namespace, c.ID) return nil } @@ -239,6 +261,7 @@ func WithNamespacedCgroup() SpecOpts { // user, uid, user:group, uid:gid, uid:group, user:gid func WithUser(userstr string) SpecOpts { return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) error { + setProcess(s) parts := strings.Split(userstr, ":") switch len(parts) { case 1: @@ -316,6 +339,7 @@ func WithUser(userstr string) SpecOpts { // WithUIDGID allows the UID and GID for the Process to be set func WithUIDGID(uid, gid uint32) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setProcess(s) s.Process.User.UID = uid s.Process.User.GID = gid return nil @@ -328,6 +352,7 @@ func WithUIDGID(uid, gid uint32) SpecOpts { // uid, and not returns error. func WithUserID(uid uint32) SpecOpts { return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) (err error) { + setProcess(s) if c.Snapshotter == "" && c.SnapshotKey == "" { if !isRootfsAbs(s.Root.Path) { return errors.Errorf("rootfs absolute path is required") @@ -380,6 +405,7 @@ func WithUserID(uid uint32) SpecOpts { // it returns error. func WithUsername(username string) SpecOpts { return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) (err error) { + setProcess(s) if c.Snapshotter == "" && c.SnapshotKey == "" { if !isRootfsAbs(s.Root.Path) { return errors.Errorf("rootfs absolute path is required") @@ -419,6 +445,8 @@ func WithUsername(username string) SpecOpts { // WithAllCapabilities set all linux capabilities for the process func WithAllCapabilities(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setCapabilities(s) + caps := getAllCapabilities() s.Process.Capabilities.Bounding = caps diff --git a/oci/spec_opts_windows.go b/oci/spec_opts_windows.go index 5b8ebba13..4da0802dd 100644 --- a/oci/spec_opts_windows.go +++ b/oci/spec_opts_windows.go @@ -33,6 +33,7 @@ import ( // WithImageConfig configures the spec to from the configuration of an Image func WithImageConfig(image Image) SpecOpts { return func(ctx context.Context, client Client, _ *containers.Container, s *specs.Spec) error { + setProcess(s) ic, err := image.Config(ctx) if err != nil { return err @@ -67,6 +68,7 @@ func WithImageConfig(image Image) SpecOpts { // using a TTY func WithTTY(width, height int) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { + setProcess(s) s.Process.Terminal = true if s.Process.ConsoleSize == nil { s.Process.ConsoleSize = &specs.Box{} @@ -80,6 +82,7 @@ func WithTTY(width, height int) SpecOpts { // WithUsername sets the username on the process func WithUsername(username string) SpecOpts { return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) error { + setProcess(s) s.Process.User.Username = username return nil }