Replacing /tmp/gvisor.sock with /run/gvisor.sock #2163
Conversation
poiana
added
size/XS
release-note-none
release-note
and removed
do-not-merge/release-note-label-needed
release-note-none
labels
|
cc @LucaGuerra |
| @@ -165,7 +165,7 @@ void cmdline_options::define() | |||
| ("e", "Read the events from <events_file> in .scap format instead of tapping into live.", cxxopts::value(trace_filename), "<events_file>") | |||
| #ifdef HAS_GVISOR | |||
| ("g,gvisor-config", "Parse events from gVisor using the specified configuration file. A falco-compatible configuration file can be generated with --gvisor-generate-config and can be used for both runsc and Falco.", cxxopts::value(gvisor_config), "<gvisor_config>") | |||
| ("gvisor-generate-config", "Generate a configuration file that can be used for gVisor.", cxxopts::value<std::string>(gvisor_generate_config_with_socket)->implicit_value("/tmp/gvisor.sock"), "<socket_path>") | |||
| ("gvisor-generate-config", "Generate a configuration file that can be used for gVisor.", cxxopts::value<std::string>(gvisor_generate_config_with_socket)->implicit_value("/run/gvisor.sock"), "<socket_path>") | |||
There was a problem hiding this comment.
LGTM! Actually @vjjmiras , can we call it something like /run/falco_gvisor.sock so it's clear that it's the socket that gvisor can connect to when interacting with Falco?
There was a problem hiding this comment.
Also from the FHS 3.0: Programs may have a subdirectory of /run; this is encouraged for programs that use more than one run-time file.
So, in that case, and assuming we might want to follow this method for future integrations, I'd go for /run/falco/gvisor.sock.
The only issue I've seen so far is that Falco doesn't create the subdirectory (falco/ under /run), so it has to either be created manually or the code should take care of the whole path. If the subdirectory has to be created externally, I'd choose systemd-tmpfiles for that job, which can do that just by adding a configuration file.
There was a problem hiding this comment.
I believe Falco should create subdirectories. We faced the same issues with the gRPC Unix socket.
Although modifying those default paths would introduce a backward incompatible change, I think it is worth it.
If @falcosecurity/falco-maintainers agree we can fix that.
There was a problem hiding this comment.
I would accept introducing breaking changes in some default paths to have a more consistent path schema. Which paths will be affected?
There was a problem hiding this comment.
AFAIK, only the gRPC unix socket (/run/falco.sock 👉 /run/falco/falco.sock). Btw, we are already using /run/falco/falco.sock in the helm chart.
There was a problem hiding this comment.
I've rebased this PR and fixed both sock paths (now they are under /run/falco).
PTAL
|
/milestone 0.33.0 |
According to the FHS 3.0 (https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch03s15.html), transient UNIX-domain sockets should be placed under the directory /run, so this commit updates the implicit value generated by the application. Signed-off-by: Vicente J. Jiménez Miras <vjjmiras@gmail.com>
…or.sock` Co-authored-by: Vicente J. Jiménez Miras <vjjmiras@gmail.com> Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
leogr
force-pushed
the
vjjmiras-gvisor-sock-UDS
branch
from
7bfc411
to
cb33970
Compare
|
LGTM label has been added. Git tree hash: 9091928431aabc9960616c44fa9957cbc08491da
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, jasondellaluce, vjjmiras The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
According to the FHS 3.0 (https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch03s15.html), transient UNIX-domain sockets should be placed under the directory /run, so this commit updates the implicit value generated by the application.
Signed-off-by: Vicente J. Jiménez Miras vjjmiras@gmail.com
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area engine
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: