-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rthooks: support NRI #2608
rthooks: support NRI #2608
Conversation
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
15c26f3
to
56443a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did one pass, left a few minor comments. I would need a second pass to fully follow the code logic, but looks good overall :)
contrib/rthooks/tetragon-rthooks/cmd/setup/patch-containerd-conf.go
Outdated
Show resolved
Hide resolved
Thanks Anna! I addressed the comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice patches! I just skipped this commit 4360fbf to be honest but the rest looks good, here are some comments :)
contrib/rthooks/tetragon-rthooks/cmd/setup/patch-containerd-conf.go
Outdated
Show resolved
Hide resolved
contrib/rthooks/tetragon-rthooks/cmd/setup/patch-containerd-conf.go
Outdated
Show resolved
Hide resolved
Thanks for the review! Pushed a new version to address the comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this PR contains a lot of changes and it may be difficult to follow all of these during the review. But at a high level this LGTM!
Added also some minor comments.
Replace use of logrus with slog in tetragon-oci-hook setup. Signed-off-by: Kornilios Kourtis <[email protected]>
This patch moves the containerd configuration patch code to tetragon-oci-hook. While doing so, we also change the code to use slog instead of logrus and kong instead of cobra. One of the benefits of doing this is that many go modules that were needed for patching containerd conf, can now leave under tetragon-oci-hook, and not in the main tetragon module. Signed-off-by: Kornilios Kourtis <[email protected]>
Rename tetragon-oci-hook to tetragon-rthooks. Subsequent patches will add code for hookinig into NRI. Signed-off-by: Kornilios Kourtis <[email protected]>
One more rename, since we do not need the intermediate folder any more. Signed-off-by: Kornilios Kourtis <[email protected]>
Add a dockerfile for a new tetragon-rthooks container. Signed-off-by: Kornilios Kourtis <[email protected]>
Add the rthooks dockerfile so that the tetragon-rthooks image is build in CI. Signed-off-by: Kornilios Kourtis <[email protected]>
Some initial documentation for runtime hooks. More to follow, once we are done with helm integration. Signed-off-by: Kornilios Kourtis <[email protected]>
The flag --fail-allow-namespaces is not specific to the OCI interface so move it to the setup command so that it can be used by other interfaces. Specifically, it is meant to be used by the NRI interface, to be added in upcoming patches. Signed-off-by: Kornilios Kourtis <[email protected]>
Add a --daemonize flag which allow sto run the OCI hook setup as a daemon. This serves two main puproses: 1. It allows to do cleanup once we receive a termination signal. 2. It allows to implement the NRI interface that requires a daemon. For now, the implementation for the oci-hooks is very simple: it just installs the required files and exits. Example: $ mkdir -p /tmp/deleteme/{hook,bin} $ ./tetragon-oci-hook-setup install \ --host-install-dir=/tmp/deleteme/bin \ --local-binary=$(pwd)/tetragon-oci-hook \ --local-install-dir=/tmp/deleteme/bin \ --oci-hooks.local-dir=/tmp/deleteme/hook \ --daemonize time=2024-06-28T12:04:44.873+02:00 level=INFO msg="written binary" hook-dst-path=/tmp/deleteme/bin/tetragon-oci-hook time=2024-06-28T12:04:44.873+02:00 level=INFO msg="written conf" conf-dst-path=/tmp/deleteme/hook/tetragon-oci-hook.json ^C time=2024-06-28T12:07:50.644+02:00 level=INFO msg="removed conf" conf-dst-path=/tmp/deleteme/hook/tetragon-oci-hook.json time=2024-06-28T12:07:50.650+02:00 level=INFO msg="binary removed" bin-dst-path=/tmp/deleteme/bin/tetragon-oci-hook Signed-off-by: Kornilios Kourtis <[email protected]>
This is a target for building the rthooks image. Signed-off-by: Kornilios Kourtis <[email protected]>
The current solution for installing runtime hooks is defined under ociHookSetup, and it uses an init container, which has two issues: 1. For the "oci-hooks" interface, where we just add a file under a directory, there is no easy solution to remove the file so that the hook will no longer take effect. 2. The next interface we want to implement, NRI, requires a running daemon to talk to the NRI socket. To address these issues, this patch introduces a new way to install tetragon runtime hook. First, we add the "oci-hooks" interface, but subseuent patches will also add support for "nri". Signed-off-by: Kornilios Kourtis <[email protected]>
When setting up the tetragon hook (tetragon-oci-hook), we typically need to pass arguments to it for when it is invocated by the runtime system. One way of achieving that is to add a flag for the flags we want to pass to the hook. This is what we currently do with --fail-allow-namespace. The problem with this approach is that we need to keep adding arguments to the setup tool for every argument we want to pass to the hook. This patch addresses this issue by introducing a positional argument (called "hook-args"), where all flags after this argument will be passed to the hook. So instead of doing: tetragon-oci-setup ... --fail-allow-namespaces ... We can now do tetragon-oci-setup ... hook-args --fail-allow-namespaces ... Hence, we remove the previous --fail-allow-namespaces flag, and replace all its uses with the new approach. Signed-off-by: Kornilios Kourtis <[email protected]>
Add a --grpc-address argument for the tetragon-oci-hook binary, in both the old (now deprecated) init container and the new container that runs in the tetragon-rthooks daemonset. This allows the default gRPC setting to work, since tetragon (by default) runs in the host network namespace. For a UNIX socket to work, it has to be in a location that is accessible from both the pod and the host hook. Host's /var/run/cilium is mounted in the tetragon pod, so change the example comment to use that. Signed-off-by: Kornilios Kourtis <[email protected]>
Add a first version of documentation on how to configure rthooks. Signed-off-by: Kornilios Kourtis <[email protected]>
Refactor code so that we can implement the NRI interface in tetragon-oci-hook-setup. Specifically, move everything under the ociHooksInstall function. Signed-off-by: Kornilios Kourtis <[email protected]>
Add an NRI interface for tetragon-oci-hook-setup. Signed-off-by: Kornilios Kourtis <[email protected]>
Signed-off-by: Kornilios Kourtis <[email protected]>
Add a log message if the hook was not able to find config.json. Signed-off-by: Kornilios Kourtis <[email protected]>
Use the createRuntime both for containerd and cri-o. The intitial version of the hook was using the createContainer. The problem with createContainer, is that the hook is executed in the namespace of the container, which means that it cannot access host networking. the createRuntime hook, however, is executed in the runtime namespace, which typicall is the host namespace. This means, that the hook can connect via TCP to the tetragon agent. (This assumes that the tetragon agent is executed using hostNamespace: true, which is the default.) Signed-off-by: Kornilios Kourtis <[email protected]>
This patch adds supoprt for the nri-hook interface in the helm charts. The patch uses required to ensure that if rthooks are enabled, the interface would be set to either "oci-hooks" or "nri-hook". The chart is configured to: - mount the hooks directoy, only when "oci-hooks" is set - moutn the NRI socket, only when "nri-hook" is set Signed-off-by: Kornilios Kourtis <[email protected]>
Add instructions for contianerd (NRI) when using minikube. Signed-off-by: Kornilios Kourtis <[email protected]>
Add a server-version command, which is intended for testing. Signed-off-by: Kornilios Kourtis <[email protected]>
Signed-off-by: Kornilios Kourtis <[email protected]>
replace uses of DialContext with NewClient, since the former is deprecated. Signed-off-by: Kornilios Kourtis <[email protected]>
Merging this, thanks everyone for the helpful reviews! |
This commit adds a worfklow for building rthooks images. rthooks images (tetragon-rthooks) are used by the tetragon-rthooks daemonset to setup the tetragon container runtime hook. See: #2608 for more details. The release images are build using rthooks/v* tags. So tagging `rthooks/v0.1`, will end up generating a tetragon-rthooks:v0.1 image. Signed-off-by: Kornilios Kourtis <[email protected]>
This commit adds a worfklow for building rthooks images. rthooks images (tetragon-rthooks) are used by the tetragon-rthooks daemonset to setup the tetragon container runtime hook. See: #2608 for more details. The release images are build using rthooks/v* tags. So tagging `rthooks/v0.1`, will end up generating a tetragon-rthooks:v0.1 image. Signed-off-by: Kornilios Kourtis <[email protected]>
This is a first PR for supporting NRI (https://github.com/containerd/nri), and having an easy way to install the tetragon runtime hook in containerd. Please see patches.
The main idea is to:
see: #1328