summaryrefslogtreecommitdiff
path: root/cmd/podman/libpodruntime/runtime.go
diff options
context:
space:
mode:
authorW. Trevor King <wking@tremily.us>2018-12-02 21:22:08 -0800
committerW. Trevor King <wking@tremily.us>2018-12-03 12:54:30 -0800
commita4b483c8484bb6fb9ae487264bccc663f007e711 (patch)
treee7463e1cce3adf5ce09747f8442eee4b92210187 /cmd/podman/libpodruntime/runtime.go
parent41f250c4861b9950b63dc7501211d13b6e0bd5ed (diff)
downloadpodman-a4b483c8484bb6fb9ae487264bccc663f007e711.tar.gz
podman-a4b483c8484bb6fb9ae487264bccc663f007e711.tar.bz2
podman-a4b483c8484bb6fb9ae487264bccc663f007e711.zip
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>
Diffstat (limited to 'cmd/podman/libpodruntime/runtime.go')
-rw-r--r--cmd/podman/libpodruntime/runtime.go4
1 files changed, 2 insertions, 2 deletions
diff --git a/cmd/podman/libpodruntime/runtime.go b/cmd/podman/libpodruntime/runtime.go
index a4b3581be..f69eaf3a4 100644
--- a/cmd/podman/libpodruntime/runtime.go
+++ b/cmd/podman/libpodruntime/runtime.go
@@ -90,8 +90,8 @@ func GetRuntimeWithStorageOpts(c *cli.Context, storageOpts *storage.StoreOptions
if c.GlobalIsSet("default-mounts-file") {
options = append(options, libpod.WithDefaultMountsFile(c.GlobalString("default-mounts-file")))
}
- if c.GlobalIsSet("hooks-dir-path") {
- options = append(options, libpod.WithHooksDir(c.GlobalString("hooks-dir-path")))
+ if c.GlobalIsSet("hooks-dir") {
+ options = append(options, libpod.WithHooksDir(c.GlobalStringSlice("hooks-dir")...))
}
// TODO flag to set CNI plugins dir?