Skip to content

Commit

Permalink
Consistently add empty types where they are nil in spec
Browse files Browse the repository at this point in the history 
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 <justin.cormack@docker.com>
  • Loading branch information
@justincormack
justincormack committed Apr 3, 2018
1 parent 8c2acf4 commit 0ee2f35
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 10 deletions.
10 changes: 10 additions & 0 deletions oci/spec_opts.go
View file
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
Expand Down
48 changes: 38 additions & 10 deletions oci/spec_opts_unix.go
View file
Expand Up @@ -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:]...)
Expand All @@ -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]
Expand Down Expand Up @@ -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...)
Expand All @@ -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
Expand All @@ -139,16 +159,15 @@ 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
}
}

// 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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -229,6 +250,7 @@ func WithNamespacedCgroup() SpecOpts {
if err != nil {
return err
}
setLinux(s)
s.Linux.CgroupsPath = filepath.Join("/", namespace, c.ID)
return nil
}
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions oci/spec_opts_windows.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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{}
Expand All @@ -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
}
Expand Down

0 comments on commit 0ee2f35

Please sign in to comment.