pkg/hooks/exec: Add a new package for local hook execution

This wraps os/exec to:

* Clear the environment when the hook doesn't set 'env'.  The runtime
  spec has [1]:

  > * env (array of strings, OPTIONAL) with the same semantics as IEEE
  >   Std 1003.1-2008's environ.

  And running execle or similar with NULL env results in an empty
  environment:

    $ cat test.c
    #include <unistd.h>

    int main()
    {
      return execle("/usr/bin/env", "env", NULL, NULL);
    }
    $ cc -o test test.c
    $ ./test
    ...no output...

  Go's Cmd.Env, on the other hand, has [2]:

  > If Env is nil, the new process uses the current process's environment.

  This commit works around that by setting []string{} in those cases
  to avoid leaking the runtime environment into the hooks.

* Roll the 'timeout' value (if set) into the passed context.  There's
  no need for two separate ways to cancel hook execution.

* Add a configurable timeout on abandoning a post-kill wait.  The
  waiting goroutine will continue and eventually reap the process, but
  this avoids blocking the Run() call when that takes inordinately
  long (for example, if a GPU cleanup hook is stuck in I/O sleep [3]).

The 'env' output format is specified in POSIX [4].

[1]: https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#posix-platform-hooks
[2]: https://golang.org/pkg/os/exec/#Cmd
[3]: https://github.com/projectatomic/libpod/pull/857#discussion_r192191002
[4]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/env.html

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

Closes: #857
Approved by: mheon

2 files changed, 282 insertions, 0 deletions

diff --git a/pkg/hooks/exec/exec.go b/pkg/hooks/exec/exec.go
new file mode 100644
index 000000000..94469b1d2
--- /dev/null
+++ b/pkg/hooks/exec/exec.go
@@ -0,0 +1,62 @@
+// Package exec provides utilities for executing Open Container Initative runtime hooks.
+package exec
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"io"
+	osexec "os/exec"
+	"time"
+
+	rspec "github.com/opencontainers/runtime-spec/specs-go"
+)
+
+// DefaultPostKillTimeout is the recommended default post-kill timeout.
+const DefaultPostKillTimeout = time.Duration(10) * time.Second
+
+// Run executes the hook and waits for it to complete or for the
+// context or hook-specified timeout to expire.
+func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer, stderr io.Writer, postKillTimeout time.Duration) (hookErr, err error) {
+	cmd := osexec.Cmd{
+		Path:   hook.Path,
+		Args:   hook.Args,
+		Env:    hook.Env,
+		Stdin:  bytes.NewReader(state),
+		Stdout: stdout,
+		Stderr: stderr,
+	}
+	if cmd.Env == nil {
+		cmd.Env = []string{}
+	}
+
+	if hook.Timeout != nil {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, time.Duration(*hook.Timeout)*time.Second)
+		defer cancel()
+	}
+
+	err = cmd.Start()
+	if err != nil {
+		return err, err
+	}
+	exit := make(chan error, 1)
+	go func() {
+		exit <- cmd.Wait()
+	}()
+
+	select {
+	case err = <-exit:
+		return err, err
+	case <-ctx.Done():
+		cmd.Process.Kill()
+		timer := time.NewTimer(postKillTimeout)
+		defer timer.Stop()
+		select {
+		case <-timer.C:
+			err = fmt.Errorf("failed to reap process within %s of the kill signal", postKillTimeout)
+		case err = <-exit:
+		}
+		return err, ctx.Err()
+	}
+}
diff --git a/pkg/hooks/exec/exec_test.go b/pkg/hooks/exec/exec_test.go
new file mode 100644
index 000000000..433f953df
--- /dev/null
+++ b/pkg/hooks/exec/exec_test.go
@@ -0,0 +1,220 @@
+package exec
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"os"
+	"runtime"
+	"strings"
+	"testing"
+	"time"
+
+	rspec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
+)
+
+// path is the path to an example hook executable.
+var path string
+
+// unavoidableEnvironmentKeys may be injected even if the hook
+// executable is executed with a requested empty environment.
+var unavoidableEnvironmentKeys []string
+
+func TestRun(t *testing.T) {
+	ctx := context.Background()
+	hook := &rspec.Hook{
+		Path: path,
+		Args: []string{"sh", "-c", "cat"},
+	}
+	var stderr, stdout bytes.Buffer
+	hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if hookErr != nil {
+		t.Fatal(hookErr)
+	}
+	assert.Equal(t, "{}", stdout.String())
+	assert.Equal(t, "", stderr.String())
+}
+
+func TestRunIgnoreOutput(t *testing.T) {
+	ctx := context.Background()
+	hook := &rspec.Hook{
+		Path: path,
+		Args: []string{"sh", "-c", "cat"},
+	}
+	hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if hookErr != nil {
+		t.Fatal(hookErr)
+	}
+}
+
+func TestRunFailedStart(t *testing.T) {
+	ctx := context.Background()
+	hook := &rspec.Hook{
+		Path: "/does/not/exist",
+	}
+	hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, DefaultPostKillTimeout)
+	if err == nil {
+		t.Fatal("unexpected success")
+	}
+	if !os.IsNotExist(err) {
+		t.Fatal(err)
+	}
+	assert.Equal(t, err, hookErr)
+}
+
+func parseEnvironment(input string) (env map[string]string, err error) {
+	env = map[string]string{}
+	lines := strings.Split(input, "\n")
+	for i, line := range lines {
+		if line == "" && i == len(lines)-1 {
+			continue // no content after the terminal newline
+		}
+		keyValue := strings.SplitN(line, "=", 2)
+		if len(keyValue) < 2 {
+			return env, fmt.Errorf("no = in environment line: %q", line)
+		}
+		env[keyValue[0]] = keyValue[1]
+	}
+	for _, key := range unavoidableEnvironmentKeys {
+		delete(env, key)
+	}
+	return env, nil
+}
+
+func TestRunEnvironment(t *testing.T) {
+	ctx := context.Background()
+	hook := &rspec.Hook{
+		Path: path,
+		Args: []string{"sh", "-c", "env"},
+	}
+	for _, test := range []struct {
+		name     string
+		env      []string
+		expected map[string]string
+	}{
+		{
+			name:     "unset",
+			expected: map[string]string{},
+		},
+		{
+			name:     "set empty",
+			env:      []string{},
+			expected: map[string]string{},
+		},
+		{
+			name: "set",
+			env: []string{
+				"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
+				"TERM=xterm",
+			},
+			expected: map[string]string{
+				"PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
+				"TERM": "xterm",
+			},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			var stderr, stdout bytes.Buffer
+			hook.Env = test.env
+			hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if hookErr != nil {
+				t.Fatal(hookErr)
+			}
+			assert.Equal(t, "", stderr.String())
+
+			env, err := parseEnvironment(stdout.String())
+			if err != nil {
+				t.Fatal(err)
+			}
+			assert.Equal(t, test.expected, env)
+		})
+	}
+}
+
+func TestRunCancel(t *testing.T) {
+	hook := &rspec.Hook{
+		Path: path,
+		Args: []string{"sh", "-c", "echo waiting; sleep 2; echo done"},
+	}
+	one := 1
+	for _, test := range []struct {
+		name              string
+		contextTimeout    time.Duration
+		hookTimeout       *int
+		expectedHookError string
+		expectedRunError  error
+		expectedStdout    string
+	}{
+		{
+			name:           "no timeouts",
+			expectedStdout: "waiting\ndone\n",
+		},
+		{
+			name:              "context timeout",
+			contextTimeout:    time.Duration(1) * time.Second,
+			expectedStdout:    "waiting\n",
+			expectedHookError: "^signal: killed$",
+			expectedRunError:  context.DeadlineExceeded,
+		},
+		{
+			name:              "hook timeout",
+			hookTimeout:       &one,
+			expectedStdout:    "waiting\n",
+			expectedHookError: "^signal: killed$",
+			expectedRunError:  context.DeadlineExceeded,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := context.Background()
+			var stderr, stdout bytes.Buffer
+			if test.contextTimeout > 0 {
+				var cancel context.CancelFunc
+				ctx, cancel = context.WithTimeout(ctx, test.contextTimeout)
+				defer cancel()
+			}
+			hook.Timeout = test.hookTimeout
+			hookErr, err := Run(ctx, hook, []byte("{}"), &stdout, &stderr, DefaultPostKillTimeout)
+			assert.Equal(t, test.expectedRunError, err)
+			if test.expectedHookError == "" {
+				if hookErr != nil {
+					t.Fatal(hookErr)
+				}
+			} else {
+				assert.Regexp(t, test.expectedHookError, hookErr.Error())
+			}
+			assert.Equal(t, "", stderr.String())
+			assert.Equal(t, test.expectedStdout, stdout.String())
+		})
+	}
+}
+
+func TestRunKillTimeout(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(500)*time.Millisecond)
+	defer cancel()
+	hook := &rspec.Hook{
+		Path: path,
+		Args: []string{"sh", "-c", "sleep 1"},
+	}
+	hookErr, err := Run(ctx, hook, []byte("{}"), nil, nil, time.Duration(0))
+	assert.Equal(t, context.DeadlineExceeded, err)
+	assert.Regexp(t, "^failed to reap process within 0s of the kill signal$", hookErr)
+}
+
+func init() {
+	if runtime.GOOS != "windows" {
+		path = "/bin/sh"
+		unavoidableEnvironmentKeys = []string{"PWD", "SHLVL", "_"}
+	} else {
+		panic("we need a reliable executable path on Windows")
+	}
+}