Nit: Do on one line
if err := ...

Nit: Do on one line
if err := ...

Is there a way to do this automatically (e.g. with gofmt)? I usually use separate lines, and there are already many instances of the separate-line approach in the hooks package:

$ git describe --tags
v0.5.2-3-g07253fc
$ git grep -B1 'if err != nil' pkg/hooks
pkg/hooks/0.1.0/hook_test.go-   hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"stages\": [\"prestart\"], \"cmds\": [\"sh\"]}"))
pkg/hooks/0.1.0/hook_test.go:   if err != nil {
--
pkg/hooks/0.1.0/hook_test.go-   hook, err := read([]byte("{\"hook\": \"/a/b/c\", \"arguments\": [\"d\", \"e\"], \"stages\": [\"prestart\"], \
"cmds\": [\"sh\"]}"))
pkg/hooks/0.1.0/hook_test.go:   if err != nil {
--
...
--
pkg/hooks/1.0.0/hook_test.go-   err := hook.Validate()
pkg/hooks/1.0.0/hook_test.go:   if err != nil {
--
...
--
pkg/hooks/hooks.go-             err = ReadDir(dir, manager.hooks)
pkg/hooks/hooks.go:             if err != nil {
--
...
--
pkg/hooks/hooks_test.go-                err = ioutil.WriteFile(jsonPath, []byte(fmt.Sprintf("{\"version\": \"1.0.0\", \"hook\": {\"path\": \"%s\", \"timeout\": %d}, \"when\": {\"always\": true}, \"stages\": [\"prestart\"%s]}", path, i+1, extraStages)), 0644)
pkg/hooks/hooks_test.go:                if err != nil {
--
...
--
--
pkg/hooks/hooks_test.go-        err = manager.Hooks(config, map[string]string{}, false)
pkg/hooks/hooks_test.go:        if err != nil {
--
...
--
pkg/hooks/monitor.go-//   err := <-sync // block until writers are established
pkg/hooks/monitor.go://   if err != nil {
--
...
--
pkg/hooks/monitor_test.go-      err = <-sync
pkg/hooks/monitor_test.go:      if err != nil {
--
...

It looks like I've only gone with the one-line approach twice:

$ git grep 'if .*:=' pkg/hooks
pkg/hooks/1.0.0/hook.go:        if _, err := os.Stat(hook.Hook.Path); err != nil {
pkg/hooks/read.go:      if err := json.Unmarshal(content, &ver); err != nil {

I can go whichever way you like for any lines you like. Changing this one line manually is easy, but I'm not sure how you feel about the other cases.

Don't know of any gofmt call to do it. But when I look at the go code in hooks directory I see all instances where err is created by itself to use the single line.

grep "err.*:=" -r pkg/hooks/
pkg/hooks/hooks.go:	raw, err := ioutil.ReadFile(hookPath)
pkg/hooks/hooks.go:	if err := json.Unmarshal(raw, &hook); err != nil {
pkg/hooks/hooks.go:	if _, err := os.Stat(hook.Hook); err != nil {
pkg/hooks/hooks.go:	if _, err := os.Stat(hooksPath); err != nil {
pkg/hooks/hooks.go:	files, err := ioutil.ReadDir(hooksPath)
pkg/hooks/hooks.go:		hook, err := readHook(filepath.Join(hooksPath, file.Name()))
pkg/hooks/hooks.go:	if err := readHooks(hooksPath, hooksMap); err != nil {
pkg/hooks/hooks.go:		if err := readHooks(OverrideHooksDir, hooksMap); err != nil {

But when I look at the go code in hooks directory I see all instances where err is created by itself to use the single line.

grep "err.*:=" -r pkg/hooks/
...
pkg/hooks/hooks.go:	if err := json.Unmarshal(raw, &hook); err != nil {
...

It looks like you're looking at a version from before 68eb128 (#686), which moved the Unmarshal call from pkg/hooks/hooks.go into other places:

$ git show 68eb128fb0a | grep '^diff\|json\.Unmarshal' | grep -B1 Unmarshal
diff --git a/pkg/hooks/0.1.0/hook.go b/pkg/hooks/0.1.0/hook.go
+       if err = json.Unmarshal(content, &raw); err != nil {
--
diff --git a/pkg/hooks/1.0.0/hook.go b/pkg/hooks/1.0.0/hook.go
+       if err = json.Unmarshal(content, &hook); err != nil {
--
diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go
-       if err := json.Unmarshal(raw, &hook); err != nil {
--
diff --git a/pkg/hooks/read.go b/pkg/hooks/read.go
+       if err := json.Unmarshal(content, &ver); err != nil {

My grep above is still what I get with the current master (69a6cb2). And 68eb128 added a bunch of examples with the two-line approach. Most of those additions were *_test.go files, but there were some others which show up in my earlier grep.

Anyhow, I'm fine translating any of these to the one-line approach but identifying them is hard. Do you also want me to adjust master entries like this to:

if _, err := Read(filepath.Join("does", "not", "exist.json")); err == nil {
	t.Fatal("unexpected success")
} else {
	assert.Regexp(t, "^open does/not/exist.json: no such file or directory$", err.Error())
	if !os.IsNotExist(err) {
		t.Fatal("opaque wrapping for not-exist errors")
	}
}

I'm not sure where you see the tighter-scoping benefit of the one-line approach balancing out the readability drop from the more compact approach. For that TestUnknownPath example, the above if/else would be the entire test function, so gains from tighter scoping would be very small.

Or should I just be adjusting entries already being touched by this PR?

Here is an example of a line touched in this PR that could be one-lined if you like.

Yes I guess I was looking at an older pool. I guess we can allow this in and then I will open a PR to fix the non single like ones.

Being strict here helps catch typos like pre-start (where the user likely intended prestart). But it's going to make sharing a single /usr/share/containers/oci/hooks.d difficult. Do we expect all tools consuming that directory to always agree on the set of extension stages? Do we expect tools to add additional directories for extension stages not yet adopted across the whole consumer community? Do we want to provide a way for tools to allow unrecognized extension stages?

I'm content to say that all users of a single hooks.d should share the same extension stages - simplest solution for now. We can make further changes later if this proves inadequate.

I'm content to say that all users of a single hooks.d should share the same extension stages - simplest solution for now.

So what directory should I change to when I add postexit support for #730? Does the extensionStages config need to be per-directory to avoid breaking users who don't need postexit but have existing content in /usr/share/containers/oci/hooks.d?