From 68eb128fb0ad261f48f960547742542584ff1f11 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 27 Apr 2018 10:35:32 -0700 Subject: 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 Closes: #686 Approved by: mheon --- pkg/hooks/1.0.0/hook_test.go | 200 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 pkg/hooks/1.0.0/hook_test.go (limited to 'pkg/hooks/1.0.0/hook_test.go') diff --git a/pkg/hooks/1.0.0/hook_test.go b/pkg/hooks/1.0.0/hook_test.go new file mode 100644 index 000000000..003be34bb --- /dev/null +++ b/pkg/hooks/1.0.0/hook_test.go @@ -0,0 +1,200 @@ +package hook + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + rspec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" +) + +// path is the path to an example hook executable. +var path string + +func TestGoodRead(t *testing.T) { + hook, err := Read([]byte("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"/a/b/c\"}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"]}")) + if err != nil { + t.Fatal(err) + } + always := true + assert.Equal(t, &Hook{ + Version: Version, + Hook: rspec.Hook{ + Path: "/a/b/c", + }, + When: When{ + Always: &always, + }, + Stages: []string{"prestart"}, + }, hook) +} + +func TestInvalidJSON(t *testing.T) { + _, err := Read([]byte("{")) + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^unexpected end of JSON input$", err.Error()) +} + +func TestGoodValidate(t *testing.T) { + always := true + hook := &Hook{ + Version: Version, + Hook: rspec.Hook{ + Path: path, + }, + When: When{ + Always: &always, + }, + Stages: []string{"prestart"}, + } + err := hook.Validate() + if err != nil { + t.Fatal(err) + } +} + +func TestNilValidation(t *testing.T) { + var hook *Hook + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^nil hook$", err.Error()) +} + +func TestWrongVersion(t *testing.T) { + hook := Hook{Version: "0.1.0"} + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^unexpected hook version \"0.1.0\" \\(expecting 1.0.0\\)$", err.Error()) +} + +func TestNoHookPath(t *testing.T) { + hook := Hook{ + Version: "1.0.0", + Hook: rspec.Hook{}, + } + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^missing required property: hook.path$", err.Error()) +} + +func TestUnknownHookPath(t *testing.T) { + hook := Hook{ + Version: "1.0.0", + Hook: rspec.Hook{ + Path: filepath.Join("does", "not", "exist"), + }, + } + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^stat does/not/exist: no such file or directory$", err.Error()) + if !os.IsNotExist(err) { + t.Fatal("opaque wrapping for not-exist errors") + } +} + +func TestNoStages(t *testing.T) { + hook := Hook{ + Version: "1.0.0", + Hook: rspec.Hook{ + Path: path, + }, + } + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^missing required property: stages$", err.Error()) +} + +func TestInvalidStage(t *testing.T) { + hook := Hook{ + Version: "1.0.0", + Hook: rspec.Hook{ + Path: path, + }, + Stages: []string{"does-not-exist"}, + } + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^unknown stage \"does-not-exist\"$", err.Error()) +} + +func TestInvalidAnnotationKey(t *testing.T) { + hook := Hook{ + Version: "1.0.0", + Hook: rspec.Hook{ + Path: path, + }, + When: When{ + Annotations: map[string]string{ + "[": "a", + }, + }, + Stages: []string{"prestart"}, + } + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^invalid annotation key \"\\[\": error parsing regexp: .*", err.Error()) +} + +func TestInvalidAnnotationValue(t *testing.T) { + hook := Hook{ + Version: "1.0.0", + Hook: rspec.Hook{ + Path: path, + }, + When: When{ + Annotations: map[string]string{ + "a": "[", + }, + }, + Stages: []string{"prestart"}, + } + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^invalid annotation value \"\\[\": error parsing regexp: .*", err.Error()) +} + +func TestInvalidCommand(t *testing.T) { + hook := Hook{ + Version: "1.0.0", + Hook: rspec.Hook{ + Path: path, + }, + When: When{ + Commands: []string{"["}, + }, + Stages: []string{"prestart"}, + } + err := hook.Validate() + if err == nil { + t.Fatal("unexpected success") + } + assert.Regexp(t, "^invalid command \"\\[\": error parsing regexp: .*", err.Error()) +} + +func init() { + if runtime.GOOS != "windows" { + path = "/bin/sh" + } else { + panic("we need a reliable executable path on Windows") + } +} -- cgit v1.2.3-54-g00ecf