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]: #730
[2]: opencontainers/runc#1797

Signed-off-by: W. Trevor King <[email protected]>

Closes: #758
Approved by: rhatdan

Author: wking

libpod/container_internal.go

@@ -1333,7 +1333,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) er
 		}
 	}
 
-	manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}, lang)
+	manager, err := hooks.New(ctx, []string{c.runtime.config.HooksDir}, []string{}, lang)
 	if err != nil {
 		if c.runtime.config.HooksDirNotExistFatal || !os.IsNotExist(err) {
 			return err
@@ -1342,5 +1342,6 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) er
 		return nil
 	}
 
-	return manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0)
+	_, err = manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0)
+	return err
 }

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)

pkg/hooks/1.0.0/hook_test.go

@@ -51,15 +51,15 @@ func TestGoodValidate(t *testing.T) {
 		},
 		Stages: []string{"prestart"},
 	}
-	err := hook.Validate()
+	err := hook.Validate([]string{})
 	if err != nil {
 		t.Fatal(err)
 	}
 }
 
 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")
 	}

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].

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
 	}

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() {