pkg/hooks: Version the hook structure and add 1.0.0 hooks

This shifts the matching logic out of libpod/container_internal and
into the hook package, where we can reuse it after vendoring into
CRI-O.  It also adds unit tests with almost-complete coverage.  Now
libpod is even more isolated from the hook internals, which makes it
fairly straightforward to bump the hook config file to 1.0.0.  I've
dubbed the old format 0.1.0, although it doesn't specify an explicit
version.  Motivation for some of my changes with 1.0.0:

* Add an explicit version field.  This will make any future JSON
  structure migrations more straightforward by avoiding the need for
  version-guessing heuristics.

* Collect the matching properties in a new When sub-structure.  This
  makes the root Hook structure easier to understand, because you
  don't have to read over all the matching properties when wrapping
  your head around Hook.

* Replace the old 'hook' and 'arguments' with a direct embedding of
  the runtime-spec's hook structure.  This provides access to
  additional upstream properties (args[0], env, and timeout) and
  avoids the complication of a CRI-O-specific analog structure.

* Add a 'when.always' property.  You can usually accomplish this
  effect in another way (e.g. when.commands = [".*"]), but having a
  boolean explicitly for this use-case makes for easier reading and
  writing.

* Replace the previous annotations array with an annotations map.  The
  0.1.0 approach matched only the values regardless of key, and that
  seems unreliable.

* Replace 'cmds' with 'when.commands', because while there are a few
  ways to abbreviate "commands", there's only one way to write it out
  in full ;).  This gives folks one less thing to remember when
  writing hook JSON.

* Replace the old "inject if any specified condition matches" with
  "inject if all specified conditions match".  This allows for more
  precise targeting.  Users that need more generous targeting can
  recover the previous behavior by creating a separate 1.0.0 hook file
  for each specified 0.1.0 condition.

I've added doc-compat support for the various pluralizations of the
0.1.0 properties.  Previously, the docs and code were not in
agreement.  More on this particular facet in [1].

I've updated the docs to point out that the annotations being matched
are the OCI config annotations.  This differs from CRI-O, where the
annotations used are the Kubernetes-supplied annotations [2,3].  For
example, io.kubernetes.cri-o.Volumes [4] is part of CRI-O's runtime
config annotations [5], but not part of the Kubernetes-supplied
annotations CRI-O uses for matching hooks.

The Monitor method supports the CRI-O use-case [6].  podman doesn't
need it directly, but CRI-O will need it when we vendor this package
there.

I've used nvidia-container-runtime-hook for the annotation examples
because Dan mentioned the Nvidia folks as the motivation behind
annotation matching.  The environment variables are documented in [7].
The 0.1.0 hook config, which does not allow for environment variables,
only works because runc currently leaks the host environment into the
hooks [8].  I haven't been able to find documentation for their usual
annotation trigger or hook-install path, so I'm just guessing there.

[1]: https://github.com/kubernetes-incubator/cri-o/pull/1235
[2]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/server/container_create.go#L760
[3]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/server/container_create.go#L772
[4]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/pkg/annotations/annotations.go#L97-L98
[5]: https://github.com/kubernetes-incubator/cri-o/blob/v1.10.0/server/container_create.go#L830-L834
[6]: https://github.com/kubernetes-incubator/cri-o/pull/1345/
[7]: https://github.com/NVIDIA/nvidia-container-runtime/tree/v1.3.0-1#environment-variables-oci-spec
[8]: https://github.com/opencontainers/runc/pull/1738

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

Closes: #686
Approved by: mheon

1 files changed, 70 insertions, 107 deletions

diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go
index dbcd7b773..f079dd0f7 100644
--- a/pkg/hooks/hooks.go
+++ b/pkg/hooks/hooks.go
@@ -1,141 +1,104 @@
+// Package hooks implements CRI-O's hook handling.
 package hooks
 
 import (
-	"encoding/json"
+	"context"
 	"fmt"
-	"io/ioutil"
-	"os"
 	"path/filepath"
-	"regexp"
-	"strings"
-	"syscall"
+	"sync"
 
-	spec "github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/opencontainers/runtime-tools/generate"
+	rspec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
+	current "github.com/projectatomic/libpod/pkg/hooks/1.0.0"
 )
 
+// Version is the current hook configuration version.
+const Version = current.Version
+
 const (
-	// DefaultHooksDir Default directory containing hooks config files
-	DefaultHooksDir = "/usr/share/containers/oci/hooks.d"
-	// OverrideHooksDir Directory where admin can override the default configuration
-	OverrideHooksDir = "/etc/containers/oci/hooks.d"
+	// DefaultDir is the default directory containing system hook configuration files.
+	DefaultDir = "/usr/share/containers/oci/hooks.d"
+
+	// OverrideDir is the directory for hook configuration files overriding the default entries.
+	OverrideDir = "/etc/containers/oci/hooks.d"
 )
 
-// HookParams is the structure returned from read the hooks configuration
-type HookParams struct {
-	Hook          string   `json:"hook"`
-	Stage         []string `json:"stage"`
-	Cmds          []string `json:"cmd"`
-	Annotations   []string `json:"annotation"`
-	HasBindMounts bool     `json:"hasbindmounts"`
-	Arguments     []string `json:"arguments"`
+// Manager provides an opaque interface for managing CRI-O hooks.
+type Manager struct {
+	hooks       map[string]*current.Hook
+	directories []string
+	lock        sync.Mutex
 }
 
-// readHook reads hooks json files, verifies it and returns the json config
-func readHook(hookPath string) (HookParams, error) {
-	var hook HookParams
-	raw, err := ioutil.ReadFile(hookPath)
-	if err != nil {
-		return hook, errors.Wrapf(err, "error Reading hook %q", hookPath)
-	}
-	if err := json.Unmarshal(raw, &hook); err != nil {
-		return hook, errors.Wrapf(err, "error Unmarshalling JSON for %q", hookPath)
+// New creates a new hook manager.  Directories are ordered by
+// increasing preference (hook configurations in later directories
+// override configurations with the same filename from earlier
+// directories).
+func New(ctx context.Context, directories []string) (manager *Manager, err error) {
+	manager = &Manager{
+		hooks:       map[string]*current.Hook{},
+		directories: directories,
 	}
-	if _, err := os.Stat(hook.Hook); err != nil {
-		return hook, errors.Wrapf(err, "unable to stat hook %q in hook config %q", hook.Hook, hookPath)
-	}
-	validStage := map[string]bool{"prestart": true, "poststart": true, "poststop": true}
-	for _, cmd := range hook.Cmds {
-		if _, err = regexp.Compile(cmd); err != nil {
-			return hook, errors.Wrapf(err, "invalid cmd regular expression %q defined in hook config %q", cmd, hookPath)
-		}
-	}
-	for _, cmd := range hook.Annotations {
-		if _, err = regexp.Compile(cmd); err != nil {
-			return hook, errors.Wrapf(err, "invalid cmd regular expression %q defined in hook config %q", cmd, hookPath)
-		}
-	}
-	for _, stage := range hook.Stage {
-		if !validStage[stage] {
-			return hook, errors.Wrapf(err, "unknown stage %q defined in hook config %q", stage, hookPath)
-		}
-	}
-	return hook, nil
-}
 
-// readHooks reads hooks json files in directory to setup OCI Hooks
-// adding hooks to the passedin hooks map.
-func readHooks(hooksPath string, hooks map[string]HookParams) error {
-	if _, err := os.Stat(hooksPath); err != nil {
-		if os.IsNotExist(err) {
-			logrus.Warnf("hooks path: %q does not exist", hooksPath)
-			return nil
+	for _, dir := range directories {
+		err = ReadDir(dir, manager.hooks)
+		if err != nil {
+			return nil, err
 		}
-		return errors.Wrapf(err, "unable to stat hooks path %q", hooksPath)
 	}
 
-	files, err := ioutil.ReadDir(hooksPath)
-	if err != nil {
-		return err
-	}
+	return manager, nil
+}
 
-	for _, file := range files {
-		if !strings.HasSuffix(file.Name(), ".json") {
-			continue
-		}
-		hook, err := readHook(filepath.Join(hooksPath, file.Name()))
+// Hooks injects OCI runtime hooks for a given container configuration.
+func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (err error) {
+	m.lock.Lock()
+	defer m.lock.Unlock()
+	for name, hook := range m.hooks {
+		match, err := hook.When.Match(config, annotations, hasBindMounts)
 		if err != nil {
-			return err
+			return errors.Wrapf(err, "matching hook %q", name)
 		}
-		for key, h := range hooks {
-			// hook.Hook can only be defined in one hook file, unless it has the
-			// same name in the override path.
-			if hook.Hook == h.Hook && key != file.Name() {
-				return errors.Wrapf(syscall.EINVAL, "duplicate path,  hook %q from %q already defined in %q", hook.Hook, hooksPath, key)
+		if match {
+			if config.Hooks == nil {
+				config.Hooks = &rspec.Hooks{}
+			}
+			for _, stage := range hook.Stages {
+				switch stage {
+				case "prestart":
+					config.Hooks.Prestart = append(config.Hooks.Prestart, hook.Hook)
+				case "poststart":
+					config.Hooks.Poststart = append(config.Hooks.Poststart, hook.Hook)
+				case "poststop":
+					config.Hooks.Poststop = append(config.Hooks.Poststop, hook.Hook)
+				default:
+					return fmt.Errorf("hook %q: unknown stage %q", name, stage)
+				}
 			}
 		}
-		hooks[file.Name()] = hook
 	}
 	return nil
 }
 
-// SetupHooks takes a hookspath and reads all of the hooks in that directory.
-// returning a map of the configured hooks
-func SetupHooks(hooksPath string) (map[string]HookParams, error) {
-	hooksMap := make(map[string]HookParams)
-	if err := readHooks(hooksPath, hooksMap); err != nil {
-		return nil, err
-	}
-	if hooksPath == DefaultHooksDir {
-		if err := readHooks(OverrideHooksDir, hooksMap); err != nil {
-			return nil, err
-		}
+// remove remove a hook by name.
+func (m *Manager) remove(hook string) (ok bool) {
+	m.lock.Lock()
+	defer m.lock.Unlock()
+	_, ok = m.hooks[hook]
+	if ok {
+		delete(m.hooks, hook)
 	}
-
-	return hooksMap, nil
+	return ok
 }
 
-// AddOCIHook generates OCI specification using the included hook
-func AddOCIHook(g *generate.Generator, hook HookParams) error {
-	for _, stage := range hook.Stage {
-		h := spec.Hook{
-			Path: hook.Hook,
-			Args: append([]string{hook.Hook}, hook.Arguments...),
-			Env:  []string{fmt.Sprintf("stage=%s", stage)},
-		}
-		logrus.Debugf("AddOCIHook", h)
-		switch stage {
-		case "prestart":
-			g.AddPreStartHook(h)
-
-		case "poststart":
-			g.AddPostStartHook(h)
-
-		case "poststop":
-			g.AddPostStopHook(h)
-		}
+// add adds a hook by path
+func (m *Manager) add(path string) (err error) {
+	m.lock.Lock()
+	defer m.lock.Unlock()
+	hook, err := Read(path)
+	if err != nil {
+		return err
 	}
+	m.hooks[filepath.Base(path)] = hook
 	return nil
 }