From 45838b9561977f3e79cf2e61c7ed0dfd9badb303 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 11 May 2018 13:03:28 -0700 Subject: 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 Closes: #758 Approved by: rhatdan --- pkg/hooks/1.0.0/hook.go | 6 +++- pkg/hooks/1.0.0/hook_test.go | 34 ++++++++++++++++------- pkg/hooks/README.md | 7 ++--- pkg/hooks/hooks.go | 41 ++++++++++++++++++---------- pkg/hooks/hooks_test.go | 65 ++++++++++++++++++++++++++++++++++++++++---- pkg/hooks/monitor_test.go | 10 +++---- pkg/hooks/read.go | 8 +++--- pkg/hooks/read_test.go | 14 +++++----- 8 files changed, 134 insertions(+), 51 deletions(-) (limited to 'pkg') 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") } -- cgit v1.2.3-54-g00ecf