libpod/container_internal: Deprecate implicit hook directories

Part of the motivation for 800eb863 (Hooks supports two directories,
process default and override, 2018-09-17, #1487) was [1]:

> We only use this for override. The reason this was caught is people
> are trying to get hooks to work with CoreOS. You are not allowed to
> write to /usr/share... on CoreOS, so they wanted podman to also look
> at /etc, where users and third parties can write.

But we'd also been disabling hooks completely for rootless users.  And
even for root users, the override logic was tricky when folks actually
had content in both directories.  For example, if you wanted to
disable a hook from the default directory, you'd have to add a no-op
hook to the override directory.

Also, the previous implementation failed to handle the case where
there hooks defined in the override directory but the default
directory did not exist:

  $ podman version
  Version:       0.11.2-dev
  Go Version:    go1.10.3
  Git Commit:    "6df7409cb5a41c710164c42ed35e33b28f3f7214"
  Built:         Sun Dec  2 21:30:06 2018
  OS/Arch:       linux/amd64
  $ ls -l /etc/containers/oci/hooks.d/test.json
  -rw-r--r--. 1 root root 184 Dec  2 16:27 /etc/containers/oci/hooks.d/test.json
  $ podman --log-level=debug run --rm docker.io/library/alpine echo 'successful container' 2>&1 | grep -i hook
  time="2018-12-02T21:31:19-08:00" level=debug msg="reading hooks from /usr/share/containers/oci/hooks.d"
  time="2018-12-02T21:31:19-08:00" level=warning msg="failed to load hooks: {}%!(EXTRA *os.PathError=open /usr/share/containers/oci/hooks.d: no such file or directory)"

With this commit:

  $ podman --log-level=debug run --rm docker.io/library/alpine echo 'successful container' 2>&1 | grep -i hook
  time="2018-12-02T21:33:07-08:00" level=debug msg="reading hooks from /usr/share/containers/oci/hooks.d"
  time="2018-12-02T21:33:07-08:00" level=debug msg="reading hooks from /etc/containers/oci/hooks.d"
  time="2018-12-02T21:33:07-08:00" level=debug msg="added hook /etc/containers/oci/hooks.d/test.json"
  time="2018-12-02T21:33:07-08:00" level=debug msg="hook test.json matched; adding to stages [prestart]"
  time="2018-12-02T21:33:07-08:00" level=warning msg="implicit hook directories are deprecated; set --hooks-dir="/etc/containers/oci/hooks.d" explicitly to continue to load hooks from this directory"
  time="2018-12-02T21:33:07-08:00" level=error msg="container create failed: container_linux.go:336: starting container process caused "process_linux.go:399: container init caused \"process_linux.go:382: running prestart hook 0 caused \\\"error running hook: exit status 1, stdout: , stderr: oh, noes!\\\\n\\\"\""

(I'd setup the hook to error out).  You can see that it's silenly
ignoring the ENOENT for /usr/share/containers/oci/hooks.d and
continuing on to load hooks from /etc/containers/oci/hooks.d.

When it loads the hook, it also logs a warning-level message
suggesting that callers explicitly configure their hook directories.
That will help consumers migrate, so we can drop the implicit hook
directories in some future release.  When folks *do* explicitly
configure hook directories (via the newly-public --hooks-dir and
hooks_dir options), we error out if they're missing:

  $ podman --hooks-dir /does/not/exist run --rm docker.io/library/alpine echo 'successful container'
  error setting up OCI Hooks: open /does/not/exist: no such file or directory

I've dropped the trailing "path" from the old, hidden --hooks-dir-path
and hooks_dir_path because I think "dir(ectory)" is already enough
context for "we expect a path argument".  I consider this name change
non-breaking because the old forms were undocumented.

Coming back to rootless users, I've enabled hooks now.  I expect they
were previously disabled because users had no way to avoid
/usr/share/containers/oci/hooks.d which might contain hooks that
required root permissions.  But now rootless users will have to
explicitly configure hook directories, and since their default config
is from ~/.config/containers/libpod.conf, it's a misconfiguration if
it contains hooks_dir entries which point at directories with hooks
that require root access.  We error out so they can fix their
libpod.conf.

[1]: https://github.com/containers/libpod/pull/1487#discussion_r218149355

Signed-off-by: W. Trevor King <wking@tremily.us>

1 files changed, 4 insertions, 6 deletions

diff --git a/libpod/runtime.go b/libpod/runtime.go
index 9feae03fc..e043715b1 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -12,7 +12,6 @@ import (
 	"github.com/containers/image/types"
 	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/firewall"
-	"github.com/containers/libpod/pkg/hooks"
 	sysreg "github.com/containers/libpod/pkg/registries"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/libpod/pkg/util"
@@ -141,11 +140,11 @@ type RuntimeConfig struct {
 	// CNIDefaultNetwork is the network name of the default CNI network
 	// to attach pods to
 	CNIDefaultNetwork string `toml:"cni_default_network,omitempty"`
-	// HooksDir Path to the directory containing hooks configuration files
+	// HooksDir holds paths to the directories containing hooks
+	// configuration files. When the same filename is present in in
+	// multiple directories, the file in the directory listed last in
+	// this slice takes precedence.
 	HooksDir []string `toml:"hooks_dir"`
-	// HooksDirNotExistFatal switches between fatal errors and non-fatal
-	// warnings if the configured HooksDir does not exist.
-	HooksDirNotExistFatal bool `toml:"hooks_dir_not_exist_fatal"`
 	// DefaultMountsFile is the path to the default mounts file for testing
 	// purposes only
 	DefaultMountsFile string `toml:"-"`
@@ -203,7 +202,6 @@ var (
 			"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
 		},
 		CgroupManager:         SystemdCgroupsManager,
-		HooksDir:              []string{hooks.DefaultDir, hooks.OverrideDir},
 		StaticDir:             filepath.Join(storage.DefaultStoreOptions.GraphRoot, "libpod"),
 		TmpDir:                "",
 		MaxLogSize:            -1,