hooks: Add pre-create hooks for runtime-config manipulation

There's been a lot of discussion over in [1] about how to support the
NVIDIA folks and others who want to be able to create devices
(possibly after having loaded kernel modules) and bind userspace
libraries into the container.  Currently that's happening in the
middle of runc's create-time mount handling before the container
pivots to its new root directory with runc's incorrectly-timed
prestart hook trigger [2].  With this commit, we extend hooks with a
'precreate' stage to allow trusted parties to manipulate the config
JSON before calling the runtime's 'create'.

I'm recycling the existing Hook schema from pkg/hooks for this,
because we'll want Timeout for reliability and When to avoid the
expense of fork/exec when a given hook does not need to make config
changes [3].

[1]: https://github.com/opencontainers/runc/pull/1811
[2]: https://github.com/opencontainers/runc/issues/1710
[3]: https://github.com/containers/libpod/issues/1828#issuecomment-439888059

Signed-off-by: W. Trevor King <wking@tremily.us>

2 files changed, 302 insertions, 0 deletions

diff --git a/pkg/hooks/exec/runtimeconfigfilter.go b/pkg/hooks/exec/runtimeconfigfilter.go
new file mode 100644
index 000000000..b5018a4ad
--- /dev/null
+++ b/pkg/hooks/exec/runtimeconfigfilter.go
@@ -0,0 +1,36 @@
+package exec
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"time"
+
+	spec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+// RuntimeConfigFilter calls a series of hooks.  But instead of
+// passing container state on their standard input,
+// RuntimeConfigFilter passes the proposed runtime configuration (and
+// reads back a possibly-altered form from their standard output).
+func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Spec, postKillTimeout time.Duration) (hookErr, err error) {
+	data, err := json.Marshal(config)
+	for _, hook := range hooks {
+		var stdout bytes.Buffer
+		hookErr, err = Run(ctx, &hook, data, &stdout, nil, postKillTimeout)
+		if err != nil {
+			return hookErr, err
+		}
+
+		data = stdout.Bytes()
+	}
+	err = json.Unmarshal(data, config)
+	if err != nil {
+		logrus.Debugf("invalid JSON from config-filter hooks:\n%s", string(data))
+		return nil, errors.Wrap(err, "unmarshal output from config-filter hooks")
+	}
+
+	return nil, nil
+}
diff --git a/pkg/hooks/exec/runtimeconfigfilter_test.go b/pkg/hooks/exec/runtimeconfigfilter_test.go
new file mode 100644
index 000000000..b50b1b156
--- /dev/null
+++ b/pkg/hooks/exec/runtimeconfigfilter_test.go
@@ -0,0 +1,266 @@
+package exec
+
+import (
+	"context"
+	"encoding/json"
+	"os"
+	"testing"
+	"time"
+
+	spec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+)
+
+func pointerInt(value int) *int {
+	return &value
+}
+
+func pointerUInt32(value uint32) *uint32 {
+	return &value
+}
+
+func pointerFileMode(value os.FileMode) *os.FileMode {
+	return &value
+}
+
+func TestRuntimeConfigFilter(t *testing.T) {
+	unexpectedEndOfJSONInput := json.Unmarshal([]byte("{\n"), nil)
+
+	for _, test := range []struct {
+		name              string
+		contextTimeout    time.Duration
+		hooks             []spec.Hook
+		input             *spec.Spec
+		expected          *spec.Spec
+		expectedHookError string
+		expectedRunError  error
+	}{
+		{
+			name: "no-op",
+			hooks: []spec.Hook{
+				{
+					Path: path,
+					Args: []string{"sh", "-c", "cat"},
+				},
+			},
+			input: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+			expected: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+		},
+		{
+			name: "device injection",
+			hooks: []spec.Hook{
+				{
+					Path: path,
+					Args: []string{"sh", "-c", `sed 's|\("gid":0}\)|\1,{"path": "/dev/sda","type":"b","major":8,"minor":0,"fileMode":384,"uid":0,"gid":0}|'`},
+				},
+			},
+			input: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+				Linux: &spec.Linux{
+					Devices: []spec.LinuxDevice{
+						{
+							Path:     "/dev/fuse",
+							Type:     "c",
+							Major:    10,
+							Minor:    229,
+							FileMode: pointerFileMode(0600),
+							UID:      pointerUInt32(0),
+							GID:      pointerUInt32(0),
+						},
+					},
+				},
+			},
+			expected: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+				Linux: &spec.Linux{
+					Devices: []spec.LinuxDevice{
+						{
+							Path:     "/dev/fuse",
+							Type:     "c",
+							Major:    10,
+							Minor:    229,
+							FileMode: pointerFileMode(0600),
+							UID:      pointerUInt32(0),
+							GID:      pointerUInt32(0),
+						},
+						{
+							Path:     "/dev/sda",
+							Type:     "b",
+							Major:    8,
+							Minor:    0,
+							FileMode: pointerFileMode(0600),
+							UID:      pointerUInt32(0),
+							GID:      pointerUInt32(0),
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "chaining",
+			hooks: []spec.Hook{
+				{
+					Path: path,
+					Args: []string{"sh", "-c", `sed 's|\("gid":0}\)|\1,{"path": "/dev/sda","type":"b","major":8,"minor":0,"fileMode":384,"uid":0,"gid":0}|'`},
+				},
+				{
+					Path: path,
+					Args: []string{"sh", "-c", `sed 's|/dev/sda|/dev/sdb|'`},
+				},
+			},
+			input: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+				Linux: &spec.Linux{
+					Devices: []spec.LinuxDevice{
+						{
+							Path:     "/dev/fuse",
+							Type:     "c",
+							Major:    10,
+							Minor:    229,
+							FileMode: pointerFileMode(0600),
+							UID:      pointerUInt32(0),
+							GID:      pointerUInt32(0),
+						},
+					},
+				},
+			},
+			expected: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+				Linux: &spec.Linux{
+					Devices: []spec.LinuxDevice{
+						{
+							Path:     "/dev/fuse",
+							Type:     "c",
+							Major:    10,
+							Minor:    229,
+							FileMode: pointerFileMode(0600),
+							UID:      pointerUInt32(0),
+							GID:      pointerUInt32(0),
+						},
+						{
+							Path:     "/dev/sdb",
+							Type:     "b",
+							Major:    8,
+							Minor:    0,
+							FileMode: pointerFileMode(0600),
+							UID:      pointerUInt32(0),
+							GID:      pointerUInt32(0),
+						},
+					},
+				},
+			},
+		},
+		{
+			name:           "context timeout",
+			contextTimeout: time.Duration(1) * time.Second,
+			hooks: []spec.Hook{
+				{
+					Path: path,
+					Args: []string{"sh", "-c", "sleep 2"},
+				},
+			},
+			input: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+			expected: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+			expectedHookError: "^signal: killed$",
+			expectedRunError:  context.DeadlineExceeded,
+		},
+		{
+			name: "hook timeout",
+			hooks: []spec.Hook{
+				{
+					Path:    path,
+					Args:    []string{"sh", "-c", "sleep 2"},
+					Timeout: pointerInt(1),
+				},
+			},
+			input: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+			expected: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+			expectedHookError: "^signal: killed$",
+			expectedRunError:  context.DeadlineExceeded,
+		},
+		{
+			name: "invalid JSON",
+			hooks: []spec.Hook{
+				{
+					Path: path,
+					Args: []string{"sh", "-c", "echo '{'"},
+				},
+			},
+			input: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+			expected: &spec.Spec{
+				Version: "1.0.0",
+				Root: &spec.Root{
+					Path: "rootfs",
+				},
+			},
+			expectedRunError: unexpectedEndOfJSONInput,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := context.Background()
+			if test.contextTimeout > 0 {
+				var cancel context.CancelFunc
+				ctx, cancel = context.WithTimeout(ctx, test.contextTimeout)
+				defer cancel()
+			}
+			hookErr, err := RuntimeConfigFilter(ctx, test.hooks, test.input, DefaultPostKillTimeout)
+			assert.Equal(t, test.expectedRunError, errors.Cause(err))
+			if test.expectedHookError == "" {
+				if hookErr != nil {
+					t.Fatal(hookErr)
+				}
+			} else {
+				assert.Regexp(t, test.expectedHookError, hookErr.Error())
+			}
+			assert.Equal(t, test.expected, test.input)
+		})
+	}
+}