From d70194d907dc945c7bcc5cb5510884a3b4dbc361 Mon Sep 17 00:00:00 2001 From: Roberto Scolaro Date: Wed, 15 Jan 2025 11:08:38 +0000 Subject: [PATCH 1/7] feat(containers): add host-containerd socket Signed-off-by: Roberto Scolaro --- userspace/libsinsp/container.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/userspace/libsinsp/container.cpp b/userspace/libsinsp/container.cpp index 6ad3a53d2c..58d67bfa9b 100644 --- a/userspace/libsinsp/container.cpp +++ b/userspace/libsinsp/container.cpp @@ -577,6 +577,7 @@ void sinsp_container_manager::create_engines() { cri_settings.add_cri_unix_socket_path("/run/containerd/containerd.sock"); cri_settings.add_cri_unix_socket_path("/run/crio/crio.sock"); cri_settings.add_cri_unix_socket_path("/run/k3s/containerd/containerd.sock"); + cri_settings.add_cri_unix_socket_path("/run/host-containerd/containerd.sock"); } const auto& cri_socket_paths = cri_settings.get_cri_unix_socket_paths(); From e78cd94ff0a0f28097bfea922bee4112d720d981 Mon Sep 17 00:00:00 2001 From: Roberto Scolaro Date: Wed, 15 Jan 2025 17:42:02 +0000 Subject: [PATCH 2/7] fix(libsinsp/runc): use old logic and fallback for containerd Signed-off-by: Roberto Scolaro --- userspace/libsinsp/runc.cpp | 37 ++++++++----------- .../test/container_engine/cri_settings.ut.cpp | 3 +- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/userspace/libsinsp/runc.cpp b/userspace/libsinsp/runc.cpp index a61b083aef..2fcad374b8 100644 --- a/userspace/libsinsp/runc.cpp +++ b/userspace/libsinsp/runc.cpp @@ -27,6 +27,7 @@ namespace { const size_t CONTAINER_ID_LENGTH = 64; const size_t REPORTED_CONTAINER_ID_LENGTH = 12; +const char *CONTAINER_ID_VALID_CHARACTERS = "0123456789abcdefABCDEF"; static_assert(REPORTED_CONTAINER_ID_LENGTH <= CONTAINER_ID_LENGTH, "Reported container ID length cannot be longer than actual length"); @@ -40,21 +41,6 @@ inline static bool endswith(const std::string &s, const std::string &suffix) { return s.rfind(suffix) == (s.size() - suffix.size()); } -inline static bool is_host(const std::string &cgroup) { - // A good approximation to minize false-positives is to exclude systemd suffixes. - if(endswith(cgroup, ".slice") || endswith(cgroup, ".service")) { - return true; - } else if(endswith(cgroup, ".scope")) { - if(cgroup.find("crio-") != std::string::npos || - cgroup.find("docker-") != std::string::npos) { - return false; - } - return true; - } - - return false; -} - // check if cgroup ends with // If true, set to a truncated version of the id and return true. // Otherwise return false and leave container_id unchanged @@ -73,6 +59,12 @@ bool match_one_container_id(const std::string &cgroup, return false; } + if(end_pos - start_pos == CONTAINER_ID_LENGTH && + cgroup.find_first_not_of(CONTAINER_ID_VALID_CHARACTERS, start_pos) >= CONTAINER_ID_LENGTH) { + container_id = cgroup.substr(start_pos, REPORTED_CONTAINER_ID_LENGTH); + return true; + } + // In some container runtimes the container the container id is not // necessarly CONTAINER_ID_LENGTH long and can be arbitrarly defined. // To keep it simple we only discard the container id > of CONTAINER_ID_LENGTH. @@ -80,15 +72,16 @@ bool match_one_container_id(const std::string &cgroup, return false; } - if(is_host(cgroup)) { - return false; + if(cgroup.rfind("/default/") == 0 && !endswith(cgroup, ".service") && + !endswith(cgroup, ".slice")) { + size_t reported_len = end_pos - start_pos >= REPORTED_CONTAINER_ID_LENGTH + ? REPORTED_CONTAINER_ID_LENGTH + : end_pos; + container_id = cgroup.substr(start_pos, reported_len); + return true; } - size_t reported_len = end_pos - start_pos >= REPORTED_CONTAINER_ID_LENGTH - ? REPORTED_CONTAINER_ID_LENGTH - : end_pos; - container_id = cgroup.substr(start_pos, reported_len); - return true; + return false; } bool match_container_id(const std::string &cgroup, diff --git a/userspace/libsinsp/test/container_engine/cri_settings.ut.cpp b/userspace/libsinsp/test/container_engine/cri_settings.ut.cpp index 1480bd3013..74ca9e69cc 100644 --- a/userspace/libsinsp/test/container_engine/cri_settings.ut.cpp +++ b/userspace/libsinsp/test/container_engine/cri_settings.ut.cpp @@ -35,9 +35,10 @@ TEST_F(sinsp_with_test_input, default_cri_socket_paths) { auto socket_paths = cri_settings.get_cri_unix_socket_paths(); - ASSERT_EQ(socket_paths.size(), 3); + ASSERT_EQ(socket_paths.size(), 4); ASSERT_TRUE("/run/containerd/containerd.sock" == socket_paths[0]); ASSERT_TRUE("/run/crio/crio.sock" == socket_paths[1]); ASSERT_TRUE("/run/k3s/containerd/containerd.sock" == socket_paths[2]); + ASSERT_TRUE("/run/host-containerd/containerd.sock" == socket_paths[3]); } #endif From 03ab1c3d7cbb7612b50c98aa474220f412250acf Mon Sep 17 00:00:00 2001 From: Roberto Scolaro Date: Wed, 15 Jan 2025 21:15:17 +0000 Subject: [PATCH 3/7] fix(libsinsp/container_engine/containerd): avoid cache confusion between containerd sockets Signed-off-by: Roberto Scolaro --- userspace/libsinsp/container.cpp | 6 ++-- .../libsinsp/container_engine/containerd.cpp | 29 +++++++++++-------- .../libsinsp/container_engine/containerd.h | 3 +- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/userspace/libsinsp/container.cpp b/userspace/libsinsp/container.cpp index 58d67bfa9b..11880899c8 100644 --- a/userspace/libsinsp/container.cpp +++ b/userspace/libsinsp/container.cpp @@ -569,6 +569,7 @@ void sinsp_container_manager::create_engines() { m_container_engine_by_type[CT_DOCKER].push_back(docker_engine); } + size_t engine_index = 0; if(m_container_engine_mask & ((1 << CT_CRI) | (1 << CT_CRIO) | (1 << CT_CONTAINERD))) { // Get CRI socket paths from settings libsinsp::cri::cri_settings& cri_settings = libsinsp::cri::cri_settings::get(); @@ -582,13 +583,13 @@ void sinsp_container_manager::create_engines() { const auto& cri_socket_paths = cri_settings.get_cri_unix_socket_paths(); - size_t engine_index = 0; for(auto socket_path : cri_socket_paths) { auto cri_engine = std::make_shared(*this, socket_path, engine_index); m_container_engines.push_back(cri_engine); m_container_engine_by_type[CT_CRI].push_back(cri_engine); m_container_engine_by_type[CT_CRIO].push_back(cri_engine); + m_container_engine_by_type[CT_CONTAINERD].push_back(cri_engine); engine_index++; } } @@ -618,7 +619,8 @@ void sinsp_container_manager::create_engines() { m_container_engine_by_type[CT_BPM].push_back(bpm_engine); } if(m_container_engine_mask & (1 << CT_CONTAINERD)) { - auto containerd_engine = std::make_shared(*this); + auto containerd_engine = + std::make_shared(*this, engine_index); m_container_engines.push_back(containerd_engine); m_container_engine_by_type[CT_CONTAINERD].push_back(containerd_engine); } diff --git a/userspace/libsinsp/container_engine/containerd.cpp b/userspace/libsinsp/container_engine/containerd.cpp index 32df3e470f..2a4d04862d 100644 --- a/userspace/libsinsp/container_engine/containerd.cpp +++ b/userspace/libsinsp/container_engine/containerd.cpp @@ -37,7 +37,7 @@ constexpr const std::string_view CONTAINERD_SOCKETS[] = { }; bool containerd_async_source::is_ok() { - return m_container_stub != nullptr && m_image_stub != nullptr; + return m_container_stub && m_image_stub; } static inline void setup_grpc_client_context(grpc::ClientContext &context) { @@ -144,8 +144,11 @@ grpc::Status containerd_async_source::get_image_resp( return m_image_stub->Get(&context, req, &resp); } -libsinsp::container_engine::containerd::containerd(container_cache_interface &cache): +libsinsp::container_engine::containerd::containerd(container_cache_interface &cache, + size_t engine_index): container_engine_base(cache) { + m_engine_index = engine_index; + for(const auto &p : CONTAINERD_SOCKETS) { if(p.empty()) { continue; @@ -158,11 +161,8 @@ libsinsp::container_engine::containerd::containerd(container_cache_interface &ca } container_cache_interface *cache_interface = &container_cache(); - auto src = new containerd_async_source(socket_path, - containerd_async_source::NO_WAIT_LOOKUP, - 10000, - cache_interface); - m_containerd_info_source.reset(src); + m_containerd_info_source = + std::make_unique(socket_path, 0, 10000, cache_interface); if(!m_containerd_info_source->is_ok()) { m_containerd_info_source.reset(nullptr); continue; @@ -172,6 +172,10 @@ libsinsp::container_engine::containerd::containerd(container_cache_interface &ca bool containerd_async_source::parse(const containerd_lookup_request &request, sinsp_container_info &container) { + if(!is_ok()) { + return false; + } + auto container_id = request.container_id; libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, @@ -294,12 +298,13 @@ void libsinsp::container_engine::containerd::parse_containerd( libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, "containerd_async (%s): Starting asynchronous lookup", request.container_id.c_str()); - done = m_containerd_info_source->lookup(request, result); + done = m_containerd_info_source && m_containerd_info_source->lookup(request, result); } else { libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, "containerd_async (%s): Starting synchronous lookup", request.container_id.c_str()); - done = m_containerd_info_source->lookup_sync(request, result); + + done = m_containerd_info_source && m_containerd_info_source->lookup_sync(request, result); } if(done) { // if a previous lookup call already found the metadata, process it now @@ -339,7 +344,7 @@ bool libsinsp::container_engine::containerd::resolve(sinsp_threadinfo *tinfo, return true; } - if(cache->should_lookup(request.container_id, request.container_type)) { + if(cache->should_lookup(request.container_id, request.container_type, m_engine_index)) { libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, "containerd_async (%s): No existing container info", request.container_id.c_str()); @@ -348,7 +353,7 @@ bool libsinsp::container_engine::containerd::resolve(sinsp_threadinfo *tinfo, cache->set_lookup_status(request.container_id, request.container_type, sinsp_container_lookup::state::STARTED, - 0); + m_engine_index); parse_containerd(request, cache); } return false; @@ -375,7 +380,7 @@ bool libsinsp::container_engine::containerd::resolve(sinsp_threadinfo *tinfo, container.m_cpu_period = limits.m_cpu_period; container.m_cpuset_cpu_count = limits.m_cpuset_cpu_count; - if(container_cache().should_lookup(container.m_id, CT_CONTAINERD)) { + if(container_cache().should_lookup(container.m_id, CT_CONTAINERD, m_engine_index)) { container.m_name = container.m_id; container.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL); container_cache().add_container(std::make_shared(container), tinfo); diff --git a/userspace/libsinsp/container_engine/containerd.h b/userspace/libsinsp/container_engine/containerd.h index 30bcaeccd5..12a2a4b172 100644 --- a/userspace/libsinsp/container_engine/containerd.h +++ b/userspace/libsinsp/container_engine/containerd.h @@ -100,7 +100,7 @@ class containerd_async_source : public container_async_source m_containerd_info_source; + size_t m_engine_index; }; } // namespace container_engine From 33267d6ac8edbfed560351119bc9371f0434e2d4 Mon Sep 17 00:00:00 2001 From: Leonardo Grasso Date: Thu, 16 Jan 2025 11:04:28 +0100 Subject: [PATCH 4/7] docs(userspace/libsinsp/filter/parser): update grammar doc Signed-off-by: Leonardo Grasso --- userspace/libsinsp/filter/parser.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/userspace/libsinsp/filter/parser.h b/userspace/libsinsp/filter/parser.h index a502afa47c..94b051897f 100644 --- a/userspace/libsinsp/filter/parser.h +++ b/userspace/libsinsp/filter/parser.h @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 /* -Copyright (C) 2023 The Falco Authors. +Copyright (C) 2025 The Falco Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -64,10 +64,14 @@ class RE2; // UnaryOperator ::= 'exists' // NumOperator ::= '<=' | '<' | '>=' | '>' // StrOperator ::= '==' | '=' | '!=' -// | 'glob ' | 'iglob ' -// | 'contains ' | 'icontains ' | 'bcontains ' -// | 'startswith ' | 'bstartswith ' | 'endswith ' -// ListOperator ::= 'intersects' | 'in' | 'pmatch' +// | 'bcontains' | 'bstartswith' +// | 'contains' +// | 'endswith' +// | 'glob' +// | 'icontains' +// | 'startswith' +// | 'regex' +// ListOperator ::= 'in' | 'intersects' | 'pmatch' // FieldTransformerVal ::= 'val(' // FieldTransformerType ::= 'tolower(' | 'toupper(' | 'b64(' | 'basename(' | 'len(' // From f00ed0c1ac3b2b170465662549cdc764421ff038 Mon Sep 17 00:00:00 2001 From: Roberto Scolaro Date: Thu, 16 Jan 2025 11:54:49 +0000 Subject: [PATCH 5/7] chore(libsinsp/runc): report correct container id with short cid Signed-off-by: Roberto Scolaro --- userspace/libsinsp/runc.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/userspace/libsinsp/runc.cpp b/userspace/libsinsp/runc.cpp index 2fcad374b8..07239146d2 100644 --- a/userspace/libsinsp/runc.cpp +++ b/userspace/libsinsp/runc.cpp @@ -72,11 +72,12 @@ bool match_one_container_id(const std::string &cgroup, return false; } + // Avoid system host cgroups. if(cgroup.rfind("/default/") == 0 && !endswith(cgroup, ".service") && !endswith(cgroup, ".slice")) { size_t reported_len = end_pos - start_pos >= REPORTED_CONTAINER_ID_LENGTH ? REPORTED_CONTAINER_ID_LENGTH - : end_pos; + : end_pos - start_pos; container_id = cgroup.substr(start_pos, reported_len); return true; } From fb0255c9d1245d70818058899881b9d7b54d297a Mon Sep 17 00:00:00 2001 From: Leonardo Grasso Date: Thu, 16 Jan 2025 13:37:44 +0100 Subject: [PATCH 6/7] docs(userspace/libsinsp/filter/parser): fix grammar doc The spaces after the operator tokens were intended to indicate operators that mandate a whitespace character to be followed. Co-authored-by: Jason Dellaluce Signed-off-by: Leonardo Grasso --- userspace/libsinsp/filter/parser.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/userspace/libsinsp/filter/parser.h b/userspace/libsinsp/filter/parser.h index 94b051897f..bf2c6d68ac 100644 --- a/userspace/libsinsp/filter/parser.h +++ b/userspace/libsinsp/filter/parser.h @@ -64,13 +64,14 @@ class RE2; // UnaryOperator ::= 'exists' // NumOperator ::= '<=' | '<' | '>=' | '>' // StrOperator ::= '==' | '=' | '!=' -// | 'bcontains' | 'bstartswith' -// | 'contains' -// | 'endswith' -// | 'glob' -// | 'icontains' -// | 'startswith' -// | 'regex' +// | 'bcontains ' | 'bstartswith ' +// | 'contains ' +// | 'endswith ' +// | 'glob ' +// | 'icontains ' +// | 'iglob ' +// | 'startswith ' +// | 'regex ' // ListOperator ::= 'in' | 'intersects' | 'pmatch' // FieldTransformerVal ::= 'val(' // FieldTransformerType ::= 'tolower(' | 'toupper(' | 'b64(' | 'basename(' | 'len(' From a67053f7386bc7b67c684e1b3ba1498c70ae1474 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Thu, 16 Jan 2025 10:11:34 +0100 Subject: [PATCH 7/7] chore(ci): switch to github-provided arm64 runners. Signed-off-by: Federico Di Pierro --- .github/workflows/ci.yml | 2 +- .github/workflows/drivers_ci.yml | 4 ++-- .github/workflows/e2e_ci.yml | 4 ++-- .github/workflows/latest-kernel.yml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9f790bc5d..7655539316 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ concurrency: jobs: build-libs-linux: name: build-libs-linux-${{ matrix.arch }} 😁 (${{ matrix.name }}) - runs-on: ${{ (matrix.arch == 'arm64' && 'github-arm64-2c-8gb') || 'ubuntu-22.04' }} + runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-22.04-arm') || 'ubuntu-22.04' }} strategy: fail-fast: false matrix: diff --git a/.github/workflows/drivers_ci.yml b/.github/workflows/drivers_ci.yml index 2a3e7a9b0b..8e2d7ea848 100644 --- a/.github/workflows/drivers_ci.yml +++ b/.github/workflows/drivers_ci.yml @@ -39,7 +39,7 @@ jobs: # This job run all engine tests and scap-open test-scap: name: test-scap-${{ matrix.arch }} 😆 (bundled_deps) - runs-on: ${{ (matrix.arch == 'arm64' && 'github-arm64-2c-8gb') || 'ubuntu-22.04' }} + runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-22.04-arm') || 'ubuntu-22.04' }} needs: paths-filter strategy: matrix: @@ -104,7 +104,7 @@ jobs: test-drivers: name: test-drivers-${{ matrix.arch }} 😇 (bundled_deps) - runs-on: ${{ (matrix.arch == 'arm64' && 'github-arm64-2c-8gb') || 'ubuntu-22.04' }} + runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-22.04-arm') || 'ubuntu-22.04' }} needs: paths-filter strategy: matrix: diff --git a/.github/workflows/e2e_ci.yml b/.github/workflows/e2e_ci.yml index 72cfa7fd45..bc5a585090 100644 --- a/.github/workflows/e2e_ci.yml +++ b/.github/workflows/e2e_ci.yml @@ -15,7 +15,7 @@ concurrency: jobs: build-test-e2e: name: build-test-e2e-${{ matrix.arch }} 😇 (bundled_deps) - runs-on: ${{ (matrix.arch == 'arm64' && 'github-arm64-2c-8gb') || 'ubuntu-22.04' }} + runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-22.04-arm') || 'ubuntu-22.04' }} strategy: matrix: arch: [amd64, arm64] @@ -105,7 +105,7 @@ jobs: test-e2e: name: test-e2e-${{ matrix.arch }}-${{ matrix.driver.name }} 😇 (bundled_deps) needs: [build-test-e2e] - runs-on: ${{ (matrix.arch == 'arm64' && 'github-arm64-2c-8gb') || 'ubuntu-22.04' }} + runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-22.04-arm') || 'ubuntu-22.04' }} strategy: matrix: arch: [amd64, arm64] diff --git a/.github/workflows/latest-kernel.yml b/.github/workflows/latest-kernel.yml index 73a92ef63f..ffddfce55a 100644 --- a/.github/workflows/latest-kernel.yml +++ b/.github/workflows/latest-kernel.yml @@ -78,7 +78,7 @@ jobs: needs: 'compute-latest-version' outputs: build: ${{ steps.build.outcome }} - runs-on: 'github-arm64-2c-8gb' + runs-on: 'ubuntu-22.04-arm' steps: - name: Download driverkit config uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8