hooks: Add package support for extension stages

We aren't consuming this yet, but these pkg/hooks changes lay the
groundwork for future libpod changes to support post-exit hooks [1,2].

[1]: https://github.com/projectatomic/libpod/issues/730
[2]: https://github.com/opencontainers/runc/issues/1797

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

Closes: #758
Approved by: rhatdan

8 files changed, 134 insertions, 51 deletions

diff --git a/pkg/hooks/1.0.0/hook.go b/pkg/hooks/1.0.0/hook.go
index cf623ee42..77fbab5aa 100644
--- a/pkg/hooks/1.0.0/hook.go
+++ b/pkg/hooks/1.0.0/hook.go
@@ -31,7 +31,7 @@ func Read(content []byte) (hook *Hook, err error) {
 }
 
 // Validate performs load-time hook validation.
-func (hook *Hook) Validate() (err error) {
+func (hook *Hook) Validate(extensionStages []string) (err error) {
 	if hook == nil {
 		return errors.New("nil hook")
 	}
@@ -68,6 +68,10 @@ func (hook *Hook) Validate() (err error) {
 	}
 
 	validStages := map[string]bool{"prestart": true, "poststart": true, "poststop": true}
+	for _, stage := range extensionStages {
+		validStages[stage] = true
+	}
+
 	for _, stage := range hook.Stages {
 		if !validStages[stage] {
 			return fmt.Errorf("unknown stage %q", stage)
diff --git a/pkg/hooks/1.0.0/hook_test.go b/pkg/hooks/1.0.0/hook_test.go
index 003be34bb..bd6d6b654 100644
--- a/pkg/hooks/1.0.0/hook_test.go
+++ b/pkg/hooks/1.0.0/hook_test.go
@@ -51,7 +51,7 @@ func TestGoodValidate(t *testing.T) {
 		},
 		Stages: []string{"prestart"},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -59,7 +59,7 @@ func TestGoodValidate(t *testing.T) {
 
 func TestNilValidation(t *testing.T) {
 	var hook *Hook
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -68,7 +68,7 @@ func TestNilValidation(t *testing.T) {
 
 func TestWrongVersion(t *testing.T) {
 	hook := Hook{Version: "0.1.0"}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -80,7 +80,7 @@ func TestNoHookPath(t *testing.T) {
 		Version: "1.0.0",
 		Hook:    rspec.Hook{},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -94,7 +94,7 @@ func TestUnknownHookPath(t *testing.T) {
 			Path: filepath.Join("does", "not", "exist"),
 		},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -111,7 +111,7 @@ func TestNoStages(t *testing.T) {
 			Path: path,
 		},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -126,13 +126,27 @@ func TestInvalidStage(t *testing.T) {
 		},
 		Stages: []string{"does-not-exist"},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
 	assert.Regexp(t, "^unknown stage \"does-not-exist\"$", err.Error())
 }
 
+func TestExtensionStage(t *testing.T) {
+	hook := Hook{
+		Version: "1.0.0",
+		Hook: rspec.Hook{
+			Path: path,
+		},
+		Stages: []string{"prestart", "b"},
+	}
+	err := hook.Validate([]string{"a", "b", "c"})
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestInvalidAnnotationKey(t *testing.T) {
 	hook := Hook{
 		Version: "1.0.0",
@@ -146,7 +160,7 @@ func TestInvalidAnnotationKey(t *testing.T) {
 		},
 		Stages: []string{"prestart"},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -166,7 +180,7 @@ func TestInvalidAnnotationValue(t *testing.T) {
 		},
 		Stages: []string{"prestart"},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -184,7 +198,7 @@ func TestInvalidCommand(t *testing.T) {
 		},
 		Stages: []string{"prestart"},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
diff --git a/pkg/hooks/README.md b/pkg/hooks/README.md
index 0d7037222..86855e9f1 100644
--- a/pkg/hooks/README.md
+++ b/pkg/hooks/README.md
@@ -44,7 +44,7 @@ Each JSON file should contain an object with the following properties:
         Entries MUST be [POSIX extended regular expressions][POSIX-ERE].
     * **`hasBindMounts`** (OPTIONAL, boolean) If `hasBindMounts` is true and the caller requested host-to-container bind mounts (beyond those that CRI-O or libpod use by default), this condition matches.
 * **`stages`** (REQUIRED, array of strings) Stages when the hook MUST be injected.
-    Entries MUST be chosen from the 1.0.1 OCI Runtime Specification [hook stages][spec-hooks].
+    Entries MUST be chosen from the 1.0.1 OCI Runtime Specification [hook stages][spec-hooks] or from extention stages supported by the package consumer.
 
 If *all* of the conditions set in `when` match, then the `hook` MUST be injected for the stages set in `stages`.
 
@@ -114,10 +114,7 @@ Previous versions of CRI-O and libpod supported the 0.1.0 hook schema:
     The injected hook's [`args`][spec-hooks] is `hook` with `arguments` appended.
 * **`stages`** (REQUIRED, array of strings) Stages when the hook MUST be injected.
     `stage` is an allowed synonym for this property, but you MUST NOT set both `stages` and `stage`.
-    Entries MUST be chosen from:
-    * **`prestart`**, to inject [pre-start][].
-    * **`poststart`**, to inject [post-start][].
-    * **`poststop`**, to inject [post-stop][].
+    Entries MUST be chosen from the 1.0.1 OCI Runtime Specification [hook stages][spec-hooks] or from extention stages supported by the package consumer.
 * **`cmds`** (OPTIONAL, array of strings) The hook MUST be injected if the configured [`process.args[0]`][spec-process] matches an entry.
     `cmd` is an allowed synonym for this property, but you MUST NOT set both `cmds` and `cmd`.
     Entries MUST be [POSIX extended regular expressions][POSIX-ERE].
diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go
index 337d6e3e2..6413829ee 100644
--- a/pkg/hooks/hooks.go
+++ b/pkg/hooks/hooks.go
@@ -27,10 +27,11 @@ const (
 
 // Manager provides an opaque interface for managing CRI-O hooks.
 type Manager struct {
-	hooks       map[string]*current.Hook
-	language    language.Tag
-	directories []string
-	lock        sync.Mutex
+	hooks           map[string]*current.Hook
+	directories     []string
+	extensionStages []string
+	language        language.Tag
+	lock            sync.Mutex
 }
 
 type namedHook struct {
@@ -44,15 +45,16 @@ type namedHooks []*namedHook
 // increasing preference (hook configurations in later directories
 // override configurations with the same filename from earlier
 // directories).
-func New(ctx context.Context, directories []string, lang language.Tag) (manager *Manager, err error) {
+func New(ctx context.Context, directories []string, extensionStages []string, lang language.Tag) (manager *Manager, err error) {
 	manager = &Manager{
-		hooks:       map[string]*current.Hook{},
-		directories: directories,
-		language:    lang,
+		hooks:           map[string]*current.Hook{},
+		directories:     directories,
+		extensionStages: extensionStages,
+		language:        lang,
 	}
 
 	for _, dir := range directories {
-		err = ReadDir(dir, manager.hooks)
+		err = ReadDir(dir, manager.extensionStages, manager.hooks)
 		if err != nil {
 			return nil, err
 		}
@@ -80,14 +82,18 @@ func (m *Manager) namedHooks() (hooks []*namedHook) {
 }
 
 // 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) {
+func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBindMounts bool) (extensionStages map[string][]rspec.Hook, err error) {
 	hooks := m.namedHooks()
 	collator := collate.New(m.language, collate.IgnoreCase, collate.IgnoreWidth)
 	collator.Sort(namedHooks(hooks))
+	validStages := map[string]bool{} // beyond the OCI stages
+	for _, stage := range m.extensionStages {
+		validStages[stage] = true
+	}
 	for _, namedHook := range hooks {
 		match, err := namedHook.hook.When.Match(config, annotations, hasBindMounts)
 		if err != nil {
-			return errors.Wrapf(err, "matching hook %q", namedHook.name)
+			return extensionStages, errors.Wrapf(err, "matching hook %q", namedHook.name)
 		}
 		if match {
 			if config.Hooks == nil {
@@ -102,12 +108,19 @@ func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBi
 				case "poststop":
 					config.Hooks.Poststop = append(config.Hooks.Poststop, namedHook.hook.Hook)
 				default:
-					return fmt.Errorf("hook %q: unknown stage %q", namedHook.name, stage)
+					if !validStages[stage] {
+						return extensionStages, fmt.Errorf("hook %q: unknown stage %q", namedHook.name, stage)
+					}
+					if extensionStages == nil {
+						extensionStages = map[string][]rspec.Hook{}
+					}
+					extensionStages[stage] = append(extensionStages[stage], namedHook.hook.Hook)
 				}
 			}
 		}
 	}
-	return nil
+
+	return extensionStages, nil
 }
 
 // remove remove a hook by name.
@@ -125,7 +138,7 @@ func (m *Manager) remove(hook string) (ok bool) {
 func (m *Manager) add(path string) (err error) {
 	m.lock.Lock()
 	defer m.lock.Unlock()
-	hook, err := Read(path)
+	hook, err := Read(path, m.extensionStages)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/hooks/hooks_test.go b/pkg/hooks/hooks_test.go
index 9da2ec8b0..1b0703556 100644
--- a/pkg/hooks/hooks_test.go
+++ b/pkg/hooks/hooks_test.go
@@ -48,13 +48,13 @@ func TestGoodNew(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	manager, err := New(ctx, []string{dir}, lang)
+	manager, err := New(ctx, []string{dir}, []string{}, lang)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	config := &rspec.Spec{}
-	err = manager.Hooks(config, map[string]string{}, false)
+	extensionStages, err := manager.Hooks(config, map[string]string{}, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -90,6 +90,9 @@ func TestGoodNew(t *testing.T) {
 			},
 		},
 	}, config.Hooks)
+
+	var nilExtensionStages map[string][]rspec.Hook
+	assert.Equal(t, nilExtensionStages, extensionStages)
 }
 
 func TestBadNew(t *testing.T) {
@@ -112,7 +115,7 @@ func TestBadNew(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err = New(ctx, []string{dir}, lang)
+	_, err = New(ctx, []string{dir}, []string{}, lang)
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -139,11 +142,14 @@ func TestBrokenMatch(t *testing.T) {
 			Args: []string{"/bin/sh"},
 		},
 	}
-	err := manager.Hooks(config, map[string]string{}, false)
+	extensionStages, err := manager.Hooks(config, map[string]string{}, false)
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
 	assert.Regexp(t, "^matching hook \"a\\.json\": command: error parsing regexp: .*", err.Error())
+
+	var nilExtensionStages map[string][]rspec.Hook
+	assert.Equal(t, nilExtensionStages, extensionStages)
 }
 
 func TestInvalidStage(t *testing.T) {
@@ -162,11 +168,60 @@ func TestInvalidStage(t *testing.T) {
 			},
 		},
 	}
-	err := manager.Hooks(&rspec.Spec{}, map[string]string{}, false)
+	extensionStages, err := manager.Hooks(&rspec.Spec{}, map[string]string{}, false)
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
 	assert.Regexp(t, "^hook \"a\\.json\": unknown stage \"does-not-exist\"$", err.Error())
+
+	var nilExtensionStages map[string][]rspec.Hook
+	assert.Equal(t, nilExtensionStages, extensionStages)
+}
+
+func TestExtensionStage(t *testing.T) {
+	always := true
+	manager := Manager{
+		hooks: map[string]*current.Hook{
+			"a.json": {
+				Version: current.Version,
+				Hook: rspec.Hook{
+					Path: "/a/b/c",
+				},
+				When: current.When{
+					Always: &always,
+				},
+				Stages: []string{"prestart", "a", "b"},
+			},
+		},
+		extensionStages: []string{"a", "b", "c"},
+	}
+
+	config := &rspec.Spec{}
+	extensionStages, err := manager.Hooks(config, map[string]string{}, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, &rspec.Hooks{
+		Prestart: []rspec.Hook{
+			{
+				Path: "/a/b/c",
+			},
+		},
+	}, config.Hooks)
+
+	assert.Equal(t, map[string][]rspec.Hook{
+		"a": {
+			{
+				Path: "/a/b/c",
+			},
+		},
+		"b": {
+			{
+				Path: "/a/b/c",
+			},
+		},
+	}, extensionStages)
 }
 
 func init() {
diff --git a/pkg/hooks/monitor_test.go b/pkg/hooks/monitor_test.go
index 3887b2072..78e8d3fe5 100644
--- a/pkg/hooks/monitor_test.go
+++ b/pkg/hooks/monitor_test.go
@@ -27,7 +27,7 @@ func TestMonitorGood(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	manager, err := New(ctx, []string{dir}, lang)
+	manager, err := New(ctx, []string{dir}, []string{}, lang)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -50,7 +50,7 @@ func TestMonitorGood(t *testing.T) {
 		time.Sleep(100 * time.Millisecond) // wait for monitor to notice
 
 		config := &rspec.Spec{}
-		err = manager.Hooks(config, map[string]string{}, false)
+		_, err = manager.Hooks(config, map[string]string{}, false)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -84,7 +84,7 @@ func TestMonitorGood(t *testing.T) {
 
 		config := &rspec.Spec{}
 		expected := config.Hooks
-		err = manager.Hooks(config, map[string]string{}, false)
+		_, err = manager.Hooks(config, map[string]string{}, false)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -101,7 +101,7 @@ func TestMonitorGood(t *testing.T) {
 
 		config := &rspec.Spec{}
 		expected := config.Hooks
-		err = manager.Hooks(config, map[string]string{}, false)
+		_, err = manager.Hooks(config, map[string]string{}, false)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -126,7 +126,7 @@ func TestMonitorBadWatcher(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	manager, err := New(ctx, []string{}, lang)
+	manager, err := New(ctx, []string{}, []string{}, lang)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/hooks/read.go b/pkg/hooks/read.go
index 29953ac5d..ae34913b6 100644
--- a/pkg/hooks/read.go
+++ b/pkg/hooks/read.go
@@ -25,7 +25,7 @@ var (
 )
 
 // Read reads a hook JSON file, verifies it, and returns the hook configuration.
-func Read(path string) (*current.Hook, error) {
+func Read(path string, extensionStages []string) (*current.Hook, error) {
 	if !strings.HasSuffix(path, ".json") {
 		return nil, ErrNoJSONSuffix
 	}
@@ -37,7 +37,7 @@ func Read(path string) (*current.Hook, error) {
 	if err != nil {
 		return nil, errors.Wrapf(err, "parsing hook %q", path)
 	}
-	err = hook.Validate()
+	err = hook.Validate(extensionStages)
 	return hook, err
 }
 
@@ -60,14 +60,14 @@ func read(content []byte) (hook *current.Hook, err error) {
 
 // ReadDir reads hook JSON files from a directory into the given map,
 // clobbering any previous entries with the same filenames.
-func ReadDir(path string, hooks map[string]*current.Hook) error {
+func ReadDir(path string, extensionStages []string, hooks map[string]*current.Hook) error {
 	files, err := ioutil.ReadDir(path)
 	if err != nil {
 		return err
 	}
 
 	for _, file := range files {
-		hook, err := Read(filepath.Join(path, file.Name()))
+		hook, err := Read(filepath.Join(path, file.Name()), extensionStages)
 		if err != nil {
 			if err == ErrNoJSONSuffix {
 				continue
diff --git a/pkg/hooks/read_test.go b/pkg/hooks/read_test.go
index 5e77f5228..69e7aff44 100644
--- a/pkg/hooks/read_test.go
+++ b/pkg/hooks/read_test.go
@@ -13,12 +13,12 @@ import (
 )
 
 func TestNoJSONSuffix(t *testing.T) {
-	_, err := Read("abc")
+	_, err := Read("abc", []string{})
 	assert.Equal(t, err, ErrNoJSONSuffix)
 }
 
 func TestUnknownPath(t *testing.T) {
-	_, err := Read(filepath.Join("does", "not", "exist.json"))
+	_, err := Read(filepath.Join("does", "not", "exist.json"), []string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -41,7 +41,7 @@ func TestGoodFile(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	hook, err := Read(jsonPath)
+	hook, err := Read(jsonPath, []string{})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -71,7 +71,7 @@ func TestBadFile(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err = Read(path)
+	_, err = Read(path, []string{})
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -139,7 +139,7 @@ func TestGoodDir(t *testing.T) {
 	}
 
 	hooks := map[string]*current.Hook{}
-	err = ReadDir(dir, hooks)
+	err = ReadDir(dir, []string{}, hooks)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -161,7 +161,7 @@ func TestGoodDir(t *testing.T) {
 
 func TestUnknownDir(t *testing.T) {
 	hooks := map[string]*current.Hook{}
-	err := ReadDir(filepath.Join("does", "not", "exist"), hooks)
+	err := ReadDir(filepath.Join("does", "not", "exist"), []string{}, hooks)
 	if err == nil {
 		t.Fatal("unexpected success")
 	}
@@ -185,7 +185,7 @@ func TestBadDir(t *testing.T) {
 	}
 
 	hooks := map[string]*current.Hook{}
-	err = ReadDir(dir, hooks)
+	err = ReadDir(dir, []string{}, hooks)
 	if err == nil {
 		t.Fatal("unexpected success")
 	}