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>
This commit is contained in:
W. Trevor King 2018-11-19 09:22:32 -08:00
parent c9d63fe89d
commit f6a2b6bf2b
6 changed files with 337 additions and 13 deletions

View file

@ -37,7 +37,9 @@ libpod to manage containers.
For the bind-mount conditions, only mounts explicitly requested by the caller via `--volume` are considered. Bind mounts that libpod inserts by default (e.g. `/dev/shm`) are not considered.
If `hooks_dir` is unset for root callers, Podman and libpod will currently default to `/usr/share/containers/oci/hooks.d` and `/etc/containers/oci/hooks.d` in order of increasing precedence. Using these defaults is deprecated, and callers should migrate to explicitly setting `hooks_dir`.
Podman and libpod currently support an additional `precreate` state which is called before the runtime's `create` operation. Unlike the other stages, which receive the container state on their standard input, `precreate` hooks receive the proposed runtime configuration on their standard input. They may alter that configuration as they see fit, and write the altered form to their standard output.
**WARNING**: the `precreate` hook lets you do powerful things, such as adding additional mounts to the runtime configuration. That power also makes it easy to break things. Before reporting libpod errors, try running your container with `precreate` hooks disabled to see if the problem is due to one of your hooks.
**static_dir**=""
Directory for persistent libpod files (database, etc)

View file

@ -43,6 +43,10 @@ For the bind-mount conditions, only mounts explicitly requested by the caller vi
If `--hooks-dir` is unset for root callers, Podman and libpod will currently default to `/usr/share/containers/oci/hooks.d` and `/etc/containers/oci/hooks.d` in order of increasing precedence. Using these defaults is deprecated, and callers should migrate to explicitly setting `--hooks-dir`.
Podman and libpod currently support an additional `precreate` state which is called before the runtime's `create` operation. Unlike the other stages, which receive the container state on their standard input, `precreate` hooks receive the proposed runtime configuration on their standard input. They may alter that configuration as they see fit, and write the altered form to their standard output.
**WARNING**: the `precreate` hook lets you do powerful things, such as adding additional mounts to the runtime configuration. That power also makes it easy to break things. Before reporting libpod errors, try running your container with `precreate` hooks disabled to see if the problem is due to one of your hooks.
**--log-level**
Log messages above specified level: debug, info, warn, error (default), fatal or panic

View file

@ -1181,6 +1181,7 @@ func (c *Container) saveSpec(spec *spec.Spec) error {
return nil
}
// Warning: precreate hooks may alter 'config' in place.
func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (extensionStageHooks map[string][]spec.Hook, err error) {
var locale string
var ok bool
@ -1209,13 +1210,13 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (exten
}
}
allHooks := make(map[string][]spec.Hook)
if c.runtime.config.HooksDir == nil {
if rootless.IsRootless() {
return nil, nil
}
allHooks := make(map[string][]spec.Hook)
for _, hDir := range []string{hooks.DefaultDir, hooks.OverrideDir} {
manager, err := hooks.New(ctx, []string{hDir}, []string{"poststop"}, lang)
manager, err := hooks.New(ctx, []string{hDir}, []string{"precreate", "poststop"}, lang)
if err != nil {
if os.IsNotExist(err) {
continue
@ -1233,19 +1234,32 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (exten
allHooks[i] = hook
}
}
return allHooks, nil
} else {
manager, err := hooks.New(ctx, c.runtime.config.HooksDir, []string{"precreate", "poststop"}, lang)
if err != nil {
if os.IsNotExist(err) {
logrus.Warnf("Requested OCI hooks directory %q does not exist", c.runtime.config.HooksDir)
return nil, nil
}
return nil, err
}
allHooks, err = manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0)
if err != nil {
return nil, err
}
}
manager, err := hooks.New(ctx, c.runtime.config.HooksDir, []string{"poststop"}, lang)
hookErr, err := exec.RuntimeConfigFilter(ctx, allHooks["precreate"], config, exec.DefaultPostKillTimeout)
if err != nil {
if os.IsNotExist(err) {
logrus.Warnf("Requested OCI hooks directory %q does not exist", c.runtime.config.HooksDir)
return nil, nil
logrus.Warnf("container %s: precreate hook: %v", c.ID(), err)
if hookErr != nil && hookErr != err {
logrus.Debugf("container %s: precreate hook (hook error): %v", c.ID(), hookErr)
}
return nil, err
}
return manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0)
return allHooks, nil
}
// mount mounts the container's root filesystem

View file

@ -228,10 +228,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
}
}
if c.state.ExtensionStageHooks, err = c.setupOCIHooks(ctx, g.Config); err != nil {
return nil, errors.Wrapf(err, "error setting up OCI Hooks")
}
// Bind builtin image volumes
if c.config.Rootfs == "" && c.config.ImageVolumes {
if err := c.addLocalVolumes(ctx, &g, execUser); err != nil {
@ -384,6 +380,12 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
logrus.Debugf("set root propagation to %q", rootPropagation)
g.SetLinuxRootPropagation(rootPropagation)
}
// Warning: precreate hooks may alter g.Config in place.
if c.state.ExtensionStageHooks, err = c.setupOCIHooks(ctx, g.Config); err != nil {
return nil, errors.Wrapf(err, "error setting up OCI Hooks")
}
return g.Config, nil
}

View file

@ -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
}

View file

@ -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)
})
}
}