From 906acb18b603b7ed0ef0807e2955c5b18af2e893 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 7 Sep 2018 17:05:33 -0400 Subject: [PATCH] Don't provide IO when it's not set This makes sure that runc does not get any valid IO for the pipe. Some builds and other containers will be stuck if they inspect stdin expecially and its a pipe but not connected to any user input. Signed-off-by: Michael Crosby --- cio/io.go | 9 +++ container_linux_test.go | 50 ++++++++++++++++ runtime/v1/linux/proc/exec.go | 2 +- runtime/v1/linux/proc/init.go | 10 +++- runtime/v1/linux/proc/io.go | 1 - vendor.conf | 2 +- vendor/github.com/containerd/go-runc/io.go | 55 ++++++++++++++--- .../github.com/containerd/go-runc/io_unix.go | 59 +++++++++++-------- .../containerd/go-runc/io_windows.go | 41 +++++++------ vendor/github.com/containerd/go-runc/runc.go | 3 +- 10 files changed, 174 insertions(+), 58 deletions(-) diff --git a/cio/io.go b/cio/io.go index 10ad36ba8..a9c6d2b15 100644 --- a/cio/io.go +++ b/cio/io.go @@ -141,6 +141,15 @@ func NewCreator(opts ...Opt) Creator { if err != nil { return nil, err } + if streams.Stdin == nil { + fifos.Stdin = "" + } + if streams.Stdout == nil { + fifos.Stdout = "" + } + if streams.Stderr == nil { + fifos.Stderr = "" + } return copyIO(fifos, streams) } } diff --git a/container_linux_test.go b/container_linux_test.go index e7cd5fc8b..2026b1928 100644 --- a/container_linux_test.go +++ b/container_linux_test.go @@ -23,6 +23,7 @@ import ( "context" "fmt" "io" + "io/ioutil" "os/exec" "runtime" "strings" @@ -1481,3 +1482,52 @@ func TestBindLowPortNonOpt(t *testing.T) { t.Fatal(err) } } + +func TestContainerNoSTDIN(t *testing.T) { + t.Parallel() + + client, err := newClient(t, address) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + var ( + image Image + ctx, cancel = testContext() + id = t.Name() + ) + defer cancel() + + image, err = client.GetImage(ctx, testImage) + if err != nil { + t.Fatal(err) + } + container, err := client.NewContainer(ctx, id, WithNewSpec(oci.WithImageConfig(image), withExitStatus(0)), WithNewSnapshot(id, image)) + if err != nil { + t.Fatal(err) + } + defer container.Delete(ctx, WithSnapshotCleanup) + + task, err := container.NewTask(ctx, cio.NewCreator(cio.WithStreams(nil, ioutil.Discard, ioutil.Discard))) + if err != nil { + t.Fatal(err) + } + defer task.Delete(ctx) + + statusC, err := task.Wait(ctx) + if err != nil { + t.Fatal(err) + } + if err := task.Start(ctx); err != nil { + t.Fatal(err) + } + status := <-statusC + code, _, err := status.Result() + if err != nil { + t.Fatal(err) + } + if code != 0 { + t.Errorf("expected status 0 from wait but received %d", code) + } +} diff --git a/runtime/v1/linux/proc/exec.go b/runtime/v1/linux/proc/exec.go index 9e5261adb..96c425dd9 100644 --- a/runtime/v1/linux/proc/exec.go +++ b/runtime/v1/linux/proc/exec.go @@ -147,7 +147,7 @@ func (e *execProcess) start(ctx context.Context) (err error) { return errors.Wrap(err, "creating new NULL IO") } } else { - if e.io, err = runc.NewPipeIO(e.parent.IoUID, e.parent.IoGID); err != nil { + if e.io, err = runc.NewPipeIO(e.parent.IoUID, e.parent.IoGID, withConditionalIO(e.stdio)); err != nil { return errors.Wrap(err, "failed to create runc io pipes") } } diff --git a/runtime/v1/linux/proc/init.go b/runtime/v1/linux/proc/init.go index c08ec98be..5bf5f8344 100644 --- a/runtime/v1/linux/proc/init.go +++ b/runtime/v1/linux/proc/init.go @@ -123,7 +123,7 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error { return errors.Wrap(err, "creating new NULL IO") } } else { - if p.io, err = runc.NewPipeIO(p.IoUID, p.IoGID); err != nil { + if p.io, err = runc.NewPipeIO(p.IoUID, p.IoGID, withConditionalIO(p.stdio)); err != nil { return errors.Wrap(err, "failed to create OCI runtime io pipes") } } @@ -399,3 +399,11 @@ func (p *Init) runtimeError(rErr error, msg string) error { return errors.Errorf("%s: %s", msg, rMsg) } } + +func withConditionalIO(c proc.Stdio) runc.IOOpt { + return func(o *runc.IOOption) { + o.OpenStdin = c.Stdin != "" + o.OpenStdout = c.Stdout != "" + o.OpenStderr = c.Stderr != "" + } +} diff --git a/runtime/v1/linux/proc/io.go b/runtime/v1/linux/proc/io.go index c9b7145c8..71f6ee1bb 100644 --- a/runtime/v1/linux/proc/io.go +++ b/runtime/v1/linux/proc/io.go @@ -109,7 +109,6 @@ func copyPipes(ctx context.Context, rio runc.IO, stdin, stdout, stderr string, w i.dest(fw, fr) } if stdin == "" { - rio.Stdin().Close() return nil } f, err := fifo.OpenFifo(ctx, stdin, syscall.O_RDONLY|syscall.O_NONBLOCK, 0) diff --git a/vendor.conf b/vendor.conf index 24fbb458c..216122309 100644 --- a/vendor.conf +++ b/vendor.conf @@ -1,4 +1,4 @@ -github.com/containerd/go-runc acb7c88cac264acca9b5eae187a117f4d77a1292 +github.com/containerd/go-runc 5a6d9f37cfa36b15efba46dc7ea349fa9b7143c3 github.com/containerd/console c12b1e7919c14469339a5d38f2f8ed9b64a9de23 github.com/containerd/cgroups 5e610833b72089b37d0e615de9a92dfc043757c2 github.com/containerd/typeurl a93fcdb778cd272c6e9b3028b2f42d813e785d40 diff --git a/vendor/github.com/containerd/go-runc/io.go b/vendor/github.com/containerd/go-runc/io.go index 24a419d18..6cf0410c9 100644 --- a/vendor/github.com/containerd/go-runc/io.go +++ b/vendor/github.com/containerd/go-runc/io.go @@ -34,6 +34,24 @@ type StartCloser interface { CloseAfterStart() error } +// IOOpt sets I/O creation options +type IOOpt func(*IOOption) + +// IOOption holds I/O creation options +type IOOption struct { + OpenStdin bool + OpenStdout bool + OpenStderr bool +} + +func defaultIOOption() *IOOption { + return &IOOption{ + OpenStdin: true, + OpenStdout: true, + OpenStderr: true, + } +} + func newPipe() (*pipe, error) { r, w, err := os.Pipe() if err != nil { @@ -65,14 +83,23 @@ type pipeIO struct { } func (i *pipeIO) Stdin() io.WriteCloser { + if i.in == nil { + return nil + } return i.in.w } func (i *pipeIO) Stdout() io.ReadCloser { + if i.out == nil { + return nil + } return i.out.r } func (i *pipeIO) Stderr() io.ReadCloser { + if i.err == nil { + return nil + } return i.err.r } @@ -83,28 +110,38 @@ func (i *pipeIO) Close() error { i.out, i.err, } { - if cerr := v.Close(); err == nil { - err = cerr + if v != nil { + if cerr := v.Close(); err == nil { + err = cerr + } } } return err } func (i *pipeIO) CloseAfterStart() error { - for _, f := range []*os.File{ - i.out.w, - i.err.w, + for _, f := range []*pipe{ + i.out, + i.err, } { - f.Close() + if f != nil { + f.w.Close() + } } return nil } // Set sets the io to the exec.Cmd func (i *pipeIO) Set(cmd *exec.Cmd) { - cmd.Stdin = i.in.r - cmd.Stdout = i.out.w - cmd.Stderr = i.err.w + if i.in != nil { + cmd.Stdin = i.in.r + } + if i.out != nil { + cmd.Stdout = i.out.w + } + if i.err != nil { + cmd.Stderr = i.err.w + } } func NewSTDIO() (IO, error) { diff --git a/vendor/github.com/containerd/go-runc/io_unix.go b/vendor/github.com/containerd/go-runc/io_unix.go index e0420ead6..567cd072e 100644 --- a/vendor/github.com/containerd/go-runc/io_unix.go +++ b/vendor/github.com/containerd/go-runc/io_unix.go @@ -24,8 +24,15 @@ import ( ) // NewPipeIO creates pipe pairs to be used with runc -func NewPipeIO(uid, gid int) (i IO, err error) { - var pipes []*pipe +func NewPipeIO(uid, gid int, opts ...IOOpt) (i IO, err error) { + option := defaultIOOption() + for _, o := range opts { + o(option) + } + var ( + pipes []*pipe + stdin, stdout, stderr *pipe + ) // cleanup in case of an error defer func() { if err != nil { @@ -34,33 +41,33 @@ func NewPipeIO(uid, gid int) (i IO, err error) { } } }() - stdin, err := newPipe() - if err != nil { - return nil, err + if option.OpenStdin { + if stdin, err = newPipe(); err != nil { + return nil, err + } + pipes = append(pipes, stdin) + if err = unix.Fchown(int(stdin.r.Fd()), uid, gid); err != nil { + return nil, errors.Wrap(err, "failed to chown stdin") + } } - pipes = append(pipes, stdin) - if err = unix.Fchown(int(stdin.r.Fd()), uid, gid); err != nil { - return nil, errors.Wrap(err, "failed to chown stdin") + if option.OpenStdout { + if stdout, err = newPipe(); err != nil { + return nil, err + } + pipes = append(pipes, stdout) + if err = unix.Fchown(int(stdout.w.Fd()), uid, gid); err != nil { + return nil, errors.Wrap(err, "failed to chown stdout") + } } - - stdout, err := newPipe() - if err != nil { - return nil, err + if option.OpenStderr { + if stderr, err = newPipe(); err != nil { + return nil, err + } + pipes = append(pipes, stderr) + if err = unix.Fchown(int(stderr.w.Fd()), uid, gid); err != nil { + return nil, errors.Wrap(err, "failed to chown stderr") + } } - pipes = append(pipes, stdout) - if err = unix.Fchown(int(stdout.w.Fd()), uid, gid); err != nil { - return nil, errors.Wrap(err, "failed to chown stdout") - } - - stderr, err := newPipe() - if err != nil { - return nil, err - } - pipes = append(pipes, stderr) - if err = unix.Fchown(int(stderr.w.Fd()), uid, gid); err != nil { - return nil, errors.Wrap(err, "failed to chown stderr") - } - return &pipeIO{ in: stdin, out: stdout, diff --git a/vendor/github.com/containerd/go-runc/io_windows.go b/vendor/github.com/containerd/go-runc/io_windows.go index 4ac557ef8..fc56ac4f3 100644 --- a/vendor/github.com/containerd/go-runc/io_windows.go +++ b/vendor/github.com/containerd/go-runc/io_windows.go @@ -19,8 +19,15 @@ package runc // NewPipeIO creates pipe pairs to be used with runc -func NewPipeIO() (i IO, err error) { - var pipes []*pipe +func NewPipeIO(opts ...IOOpt) (i IO, err error) { + option := defaultIOOption() + for _, o := range opts { + o(option) + } + var ( + pipes []*pipe + stdin, stdout, stderr *pipe + ) // cleanup in case of an error defer func() { if err != nil { @@ -29,24 +36,24 @@ func NewPipeIO() (i IO, err error) { } } }() - stdin, err := newPipe() - if err != nil { - return nil, err + if option.OpenStdin { + if stdin, err = newPipe(); err != nil { + return nil, err + } + pipes = append(pipes, stdin) } - pipes = append(pipes, stdin) - - stdout, err := newPipe() - if err != nil { - return nil, err + if option.OpenStdout { + if stdout, err = newPipe(); err != nil { + return nil, err + } + pipes = append(pipes, stdout) } - pipes = append(pipes, stdout) - - stderr, err := newPipe() - if err != nil { - return nil, err + if option.OpenStderr { + if stderr, err = newPipe(); err != nil { + return nil, err + } + pipes = append(pipes, stderr) } - pipes = append(pipes, stderr) - return &pipeIO{ in: stdin, out: stdout, diff --git a/vendor/github.com/containerd/go-runc/runc.go b/vendor/github.com/containerd/go-runc/runc.go index ac9c89d80..96262afab 100644 --- a/vendor/github.com/containerd/go-runc/runc.go +++ b/vendor/github.com/containerd/go-runc/runc.go @@ -608,9 +608,8 @@ func parseVersion(data []byte) (Version, error) { var v Version parts := strings.Split(strings.TrimSpace(string(data)), "\n") if len(parts) != 3 { - return v, ErrParseRuncVersion + return v, nil } - for i, p := range []struct { dest *string split string