Skip to content

Commit

Permalink
cleanup: revert any changes to parse_container_json_evt + ensure exis…
Browse files Browse the repository at this point in the history
…ting state updates in lookup_sync are preserved

Signed-off-by: Melissa Kilby <[email protected]>
  • Loading branch information
incertum authored and poiana committed Feb 28, 2024
1 parent ecfa917 commit 3aad411
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ bool container_async_source<key_type>::lookup(const key_type& key,
template<typename key_type>
bool container_async_source<key_type>::lookup_sync(const key_type& key, sinsp_container_info& value)
{
value.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL);
value.m_type = container_type(key);
value.m_id = container_id(key);

Expand Down
4 changes: 4 additions & 0 deletions userspace/libsinsp/container_engine/cri.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,11 @@ bool cri::resolve(sinsp_threadinfo *tinfo, bool query_os_for_missing_info)
* disabling async lookups.
*/
result.set_lookup_status(sinsp_container_lookup::state::STARTED);
// note: The cache should not have SUCCESSFUL as lookup status at this point, else `parse_container_json_evt` would wrongly exit early.
cache->replace_container(std::make_shared<sinsp_container_info>(result));
// note: On the other hand `parse_container_json_evt` expects SUCCESSFUL as lookup state for the incoming container event /
// the not yet cached container, exactly how it was done within `lookup_sync`.
result.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL);
}
}

Expand Down
45 changes: 14 additions & 31 deletions userspace/libsinsp/parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5080,11 +5080,22 @@ void sinsp_parser::parse_container_json_evt(sinsp_evt *evt)
const Json::Value& lookup_state = container["lookup_state"];
if(check_json_val_is_convertible(lookup_state, Json::uintValue, "lookup_state"))
{
container_info->set_lookup_status(static_cast<sinsp_container_lookup::state>(lookup_state.asUInt()));
switch(container_info->get_lookup_status())
{
case sinsp_container_lookup::state::STARTED:
case sinsp_container_lookup::state::SUCCESSFUL:
case sinsp_container_lookup::state::FAILED:
break;
default:
container_info->set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL);
}

// state == STARTED doesn't make sense in a scap file
// as there's no actual lookup that would ever finish
if(!evt->get_tinfo_ref() && container_info->get_lookup_status() == sinsp_container_lookup::state::STARTED)
{
SINSP_DEBUG("Rewriting lookup_state == STARTED from scap file to FAILED for container %s",
SINSP_DEBUG("Rewriting lookup_state = STARTED from scap file to FAILED for container %s",
container_info->m_id.c_str());
container_info->set_lookup_status(sinsp_container_lookup::state::FAILED);
}
Expand Down Expand Up @@ -5223,41 +5234,13 @@ void sinsp_parser::parse_container_json_evt(sinsp_evt *evt)
}
}

evt->set_tinfo_ref(container_info->get_tinfo(m_inspector));
evt->set_tinfo(evt->get_tinfo_ref().get());
container_info->set_lookup_status(static_cast<sinsp_container_lookup::state>(lookup_state.asUInt()));
switch(container_info->get_lookup_status())
{
// Preserve case where lookup_state == STARTED from scap file was set to FAILED
// or otherwise set to false.
case sinsp_container_lookup::state::FAILED:
break;
default:
{
// note: The container image is the most crucial field from a security incident response perspective.
// We aim to raise the bar for successful container lookups. Conversely, pod sandboxes do not include
// a container image in the API response.
if (!container_info->m_image.empty() || container_info->is_pod_sandbox())
{
// Marked as SUCCESSFUL if and only if the container image field present or
// if it's a pod sandbox container
container_info->set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL);
} else
{
container_info->set_lookup_status(sinsp_container_lookup::state::FAILED);
}
}
}

// note: Important, only set event filtering flag after all final state updates are done
if(!container_info->is_successful())
{
SINSP_DEBUG("Filtering container event for failed lookup of %s (but calling callbacks anyway)", container_info->m_id.c_str());
evt->set_filtered_out(true);
}

// `add_container` also triggers the callbacks, therefore always continue even if the
// lookup_status was set to FAILED
evt->set_tinfo_ref(container_info->get_tinfo(m_inspector));
evt->set_tinfo(evt->get_tinfo_ref().get());
m_inspector->m_container_manager.add_container(container_info, evt->get_thread_info(true));
/*
SINSP_STR_DEBUG("Container\n-------\nID:" + container_info.m_id +
Expand Down

0 comments on commit 3aad411

Please sign in to comment.