From 5e0d793801aefe1be5740a26771c9d30dac46f90 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Wed, 19 Jun 2019 11:15:17 -0700 Subject: [PATCH 1/2] Fix bugs in BinaryIO creator Signed-off-by: Maksym Pavlenko --- cio/io.go | 13 +++++++++++-- cio/io_test.go | 24 ++++++++++++++++++++++++ runtime/v1/linux/proc/io.go | 2 +- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/cio/io.go b/cio/io.go index 133bfcdbe..0460efef6 100644 --- a/cio/io.go +++ b/cio/io.go @@ -18,10 +18,13 @@ package cio import ( "context" + "errors" "fmt" "io" "net/url" "os" + "path/filepath" + "strings" "sync" "github.com/containerd/containerd/defaults" @@ -242,13 +245,19 @@ func LogURI(uri *url.URL) Creator { // BinaryIO forwards container STDOUT|STDERR directly to a logging binary func BinaryIO(binary string, args map[string]string) Creator { return func(_ string) (IO, error) { + binary = filepath.Clean(binary) + if !strings.HasPrefix(binary, "/") { + return nil, errors.New("absolute path needed") + } uri := &url.URL{ Scheme: "binary", - Host: binary, + Path: binary, } + q := uri.Query() for k, v := range args { - uri.Query().Set(k, v) + q.Set(k, v) } + uri.RawQuery = q.Encode() return &logURI{ config: Config{ Stdout: uri.String(), diff --git a/cio/io_test.go b/cio/io_test.go index 6a654eef9..000310129 100644 --- a/cio/io_test.go +++ b/cio/io_test.go @@ -23,6 +23,7 @@ import ( "context" "io" "io/ioutil" + "net/url" "os" "path/filepath" "runtime" @@ -153,3 +154,26 @@ func initProducers(t *testing.T, producers producers, stdout, stderr string) { assert.NilError(t, err) assert.NilError(t, producers.Stderr.Close()) } + +func TestBinaryIOArgs(t *testing.T) { + res, err := BinaryIO("/file.bin", map[string]string{"id": "1"})("") + assert.NilError(t, err) + assert.Equal(t, "binary:///file.bin?id=1", res.Config().Stdout) + assert.Equal(t, "binary:///file.bin?id=1", res.Config().Stderr) +} + +func TestBinaryIOAbsolutePath(t *testing.T) { + res, err := BinaryIO("/full/path/bin", nil)("!") + assert.NilError(t, err) + + // Test parse back + parsed, err := url.Parse(res.Config().Stdout) + assert.NilError(t, err) + assert.Equal(t, "binary", parsed.Scheme) + assert.Equal(t, "/full/path/bin", parsed.Path) +} + +func TestBinaryIOFailOnRelativePath(t *testing.T) { + _, err := BinaryIO("./bin", nil)("!") + assert.Error(t, err, "absolute path needed") +} diff --git a/runtime/v1/linux/proc/io.go b/runtime/v1/linux/proc/io.go index 5d2d348b5..9c4317574 100644 --- a/runtime/v1/linux/proc/io.go +++ b/runtime/v1/linux/proc/io.go @@ -265,7 +265,7 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (runc.IO, error) } } ctx, cancel := context.WithCancel(ctx) - cmd := exec.CommandContext(ctx, uri.Host, args...) + cmd := exec.CommandContext(ctx, uri.Path, args...) cmd.Env = append(cmd.Env, "CONTAINER_ID="+id, "CONTAINER_NAMESPACE="+ns, From fbf96d302aff95f8c52ab0161e7ff8ae7d33b9b9 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Wed, 19 Jun 2019 16:53:33 -0700 Subject: [PATCH 2/2] Fix path in LogFile creator Signed-off-by: Maksym Pavlenko --- cio/io.go | 16 +++++++++++----- cio/io_test.go | 18 ++++++++++++++++++ runtime/v1/linux/proc/io.go | 4 ++-- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/cio/io.go b/cio/io.go index 0460efef6..c7cf4f0bc 100644 --- a/cio/io.go +++ b/cio/io.go @@ -258,10 +258,11 @@ func BinaryIO(binary string, args map[string]string) Creator { q.Set(k, v) } uri.RawQuery = q.Encode() + res := uri.String() return &logURI{ config: Config{ - Stdout: uri.String(), - Stderr: uri.String(), + Stdout: res, + Stderr: res, }, }, nil } @@ -271,14 +272,19 @@ func BinaryIO(binary string, args map[string]string) Creator { // If the log file already exists, the logs will be appended to the file. func LogFile(path string) Creator { return func(_ string) (IO, error) { + path = filepath.Clean(path) + if !strings.HasPrefix(path, "/") { + return nil, errors.New("absolute path needed") + } uri := &url.URL{ Scheme: "file", - Host: path, + Path: path, } + res := uri.String() return &logURI{ config: Config{ - Stdout: uri.String(), - Stderr: uri.String(), + Stdout: res, + Stderr: res, }, }, nil } diff --git a/cio/io_test.go b/cio/io_test.go index 000310129..e42836f76 100644 --- a/cio/io_test.go +++ b/cio/io_test.go @@ -177,3 +177,21 @@ func TestBinaryIOFailOnRelativePath(t *testing.T) { _, err := BinaryIO("./bin", nil)("!") assert.Error(t, err, "absolute path needed") } + +func TestLogFileAbsolutePath(t *testing.T) { + res, err := LogFile("/full/path/file.txt")("!") + assert.NilError(t, err) + assert.Equal(t, "file:///full/path/file.txt", res.Config().Stdout) + assert.Equal(t, "file:///full/path/file.txt", res.Config().Stderr) + + // Test parse back + parsed, err := url.Parse(res.Config().Stdout) + assert.NilError(t, err) + assert.Equal(t, "file", parsed.Scheme) + assert.Equal(t, "/full/path/file.txt", parsed.Path) +} + +func TestLogFileFailOnRelativePath(t *testing.T) { + _, err := LogFile("./file.txt")("!") + assert.Error(t, err, "absolute path needed") +} diff --git a/runtime/v1/linux/proc/io.go b/runtime/v1/linux/proc/io.go index 9c4317574..e1bc64623 100644 --- a/runtime/v1/linux/proc/io.go +++ b/runtime/v1/linux/proc/io.go @@ -103,11 +103,11 @@ func createIO(ctx context.Context, id string, ioUID, ioGID int, stdio proc.Stdio case "binary": pio.io, err = NewBinaryIO(ctx, id, u) case "file": - if err := os.MkdirAll(filepath.Dir(u.Host), 0755); err != nil { + if err := os.MkdirAll(filepath.Dir(u.Path), 0755); err != nil { return nil, err } var f *os.File - f, err = os.OpenFile(u.Host, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + f, err = os.OpenFile(u.Path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { return nil, err }