From 83d4c05d6e39b854b1b7fb0f76be95884164aa33 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 3 Jun 2018 11:38:46 -0700 Subject: [PATCH 1/5] hooks: Fail ReadDir if a configured hook executable is missing The continue here is from 5676597f (hooks/read: Ignore IsNotExist for JSON files in ReadDir, 2018-04-27, #686), where it was intended to silently ignore missing JSON files. However, the old logic was also silently ignoring not-exist errors from the os.Stat(hook.Hook.Path) from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). This commit adjusts the check so JSON not-exist errors continue to be silently ignored while hook executable not-exist errors become fatal. Signed-off-by: W. Trevor King --- pkg/hooks/read.go | 7 +++++-- pkg/hooks/read_test.go | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/pkg/hooks/read.go b/pkg/hooks/read.go index ae34913b612..a8c9a7adcd0 100644 --- a/pkg/hooks/read.go +++ b/pkg/hooks/read.go @@ -67,13 +67,16 @@ func ReadDir(path string, extensionStages []string, hooks map[string]*current.Ho } for _, file := range files { - hook, err := Read(filepath.Join(path, file.Name()), extensionStages) + filePath := filepath.Join(path, file.Name()) + hook, err := Read(filePath, extensionStages) if err != nil { if err == ErrNoJSONSuffix { continue } if os.IsNotExist(err) { - continue + if err2, ok := err.(*os.PathError); ok && err2.Path == filePath { + continue + } } return err } diff --git a/pkg/hooks/read_test.go b/pkg/hooks/read_test.go index 69e7aff4402..811cace2357 100644 --- a/pkg/hooks/read_test.go +++ b/pkg/hooks/read_test.go @@ -191,3 +191,24 @@ func TestBadDir(t *testing.T) { } assert.Regexp(t, "^parsing hook \"[^\"]*a.json\": unrecognized hook version: \"-1\"$", err.Error()) } + +func TestHookExecutableDoesNotExit(t *testing.T) { + dir, err := ioutil.TempDir("", "hooks-test-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + jsonPath := filepath.Join(dir, "hook.json") + err = ioutil.WriteFile(jsonPath, []byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/does/not/exist\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}"), 0644) + if err != nil { + t.Fatal(err) + } + + hooks := map[string]*current.Hook{} + err = ReadDir(dir, []string{}, hooks) + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^stat /does/not/exist: no such file or directory$", err.Error()) +} From bcc78b875b4ad0a38f31791a5b471a5f059207db Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 3 Jun 2018 11:49:18 -0700 Subject: [PATCH 2/5] hooks/1.0.0: Fix 'annotation' -> 'annotations' in JSON This typo from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686) was causing any 'annotations' entries in hook JSON to be silently ignored. Signed-off-by: W. Trevor King --- pkg/hooks/1.0.0/when.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/hooks/1.0.0/when.go b/pkg/hooks/1.0.0/when.go index c23223ec0ce..a4870d0fad4 100644 --- a/pkg/hooks/1.0.0/when.go +++ b/pkg/hooks/1.0.0/when.go @@ -10,7 +10,7 @@ import ( // When holds hook-injection conditions. type When struct { Always *bool `json:"always,omitempty"` - Annotations map[string]string `json:"annotation,omitempty"` + Annotations map[string]string `json:"annotations,omitempty"` Commands []string `json:"commands,omitempty"` HasBindMounts *bool `json:"hasBindMounts,omitempty"` From 094e8398a5a4c3bc59ee5b0605107e756d53029e Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 3 Jun 2018 12:22:14 -0700 Subject: [PATCH 3/5] hooks/1.0.0/when_test: Fix "both, and" -> "both, or" name typo The typo is a copy/paste error from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). Signed-off-by: W. Trevor King --- pkg/hooks/1.0.0/when_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/hooks/1.0.0/when_test.go b/pkg/hooks/1.0.0/when_test.go index 5a73270ac62..61e725c8cd5 100644 --- a/pkg/hooks/1.0.0/when_test.go +++ b/pkg/hooks/1.0.0/when_test.go @@ -218,7 +218,7 @@ func TestHasBindMountsAndCommands(t *testing.T) { match: true, }, { - name: "both, and", + name: "both, or", command: "/bin/sh", hasBindMounts: true, or: true, From 6e6411262b7e712c3f7c98b1a6c0738721af2aaa Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 3 Jun 2018 11:50:45 -0700 Subject: [PATCH 4/5] hooks/docs: Fix 1.0.0 Nvidia example (adding version, etc.) Reported by Gary Edwards [1]. Both typos are originally from 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). [1]: https://github.com/projectatomic/libpod/issues/884#issuecomment-394174571 Signed-off-by: W. Trevor King --- pkg/hooks/docs/oci-hooks.5.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/hooks/docs/oci-hooks.5.md b/pkg/hooks/docs/oci-hooks.5.md index cdc1376416c..493e4f15769 100644 --- a/pkg/hooks/docs/oci-hooks.5.md +++ b/pkg/hooks/docs/oci-hooks.5.md @@ -119,6 +119,7 @@ The following example injects `nvidia-container-runtime-hook prestart` with part ```console $ cat /etc/containers/oci/hooks.d/nvidia.json { + "version": "1.0.0", "hook": { "path": "/usr/sbin/nvidia-container-runtime-hook", "args": ["nvidia-container-runtime-hook", "prestart"], @@ -129,7 +130,7 @@ $ cat /etc/containers/oci/hooks.d/nvidia.json }, "when": { "annotations": { - "^com\.example\.department$": ".*fluid-dynamics$" + "^com\\.example\\.department$": ".*fluid-dynamics$" } }, "stages": ["prestart"] From 6ded615177e879633c360e92ed061a7c7cbeb2a3 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 3 Jun 2018 12:34:42 -0700 Subject: [PATCH 5/5] hooks: Add debug logging for initial hook loading We've had logrus logging in the monitor code since it landed in 68eb128f (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). This commit adds similar logging to the initial hook.New() and Manager.Hooks() calls to make it easier to see if those are working as expected. Signed-off-by: W. Trevor King --- pkg/hooks/hooks.go | 4 ++++ pkg/hooks/read.go | 3 +++ 2 files changed, 7 insertions(+) diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go index d18cc119509..fdc8a6c46b2 100644 --- a/pkg/hooks/hooks.go +++ b/pkg/hooks/hooks.go @@ -10,6 +10,7 @@ import ( rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" current "github.com/projectatomic/libpod/pkg/hooks/1.0.0" + "github.com/sirupsen/logrus" "golang.org/x/text/collate" "golang.org/x/text/language" ) @@ -112,6 +113,7 @@ func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBi return extensionStageHooks, errors.Wrapf(err, "matching hook %q", namedHook.name) } if match { + logrus.Debugf("hook %s matched; adding to stages %v", namedHook.name, namedHook.hook.Stages) if config.Hooks == nil { config.Hooks = &rspec.Hooks{} } @@ -134,6 +136,8 @@ func (m *Manager) Hooks(config *rspec.Spec, annotations map[string]string, hasBi } } } + } else { + logrus.Debugf("hook %s did not match", namedHook.name) } } diff --git a/pkg/hooks/read.go b/pkg/hooks/read.go index a8c9a7adcd0..20431bdf1f3 100644 --- a/pkg/hooks/read.go +++ b/pkg/hooks/read.go @@ -11,6 +11,7 @@ import ( "github.com/pkg/errors" current "github.com/projectatomic/libpod/pkg/hooks/1.0.0" + "github.com/sirupsen/logrus" ) type reader func(content []byte) (*current.Hook, error) @@ -61,6 +62,7 @@ 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, extensionStages []string, hooks map[string]*current.Hook) error { + logrus.Debugf("reading hooks from %s", path) files, err := ioutil.ReadDir(path) if err != nil { return err @@ -81,6 +83,7 @@ func ReadDir(path string, extensionStages []string, hooks map[string]*current.Ho return err } hooks[file.Name()] = hook + logrus.Debugf("added hook %s", filePath) } return nil }