Skip to content
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

fix(userspace/libsinsp): properly set successful lookup state when parsing old container json events #1811

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Apr 24, 2024

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area libsinsp

Does this PR require a change in the driver versions?

What this PR does / why we need it:

This PR fixups a small bug indirectly caused by the changes in #1707.
Default at successful lookup state when lookup_state is not part of the container json event.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Falco CI uses very very old container events in scap files (prior to 58a546a changes) thus one event was expected but not received becase it had no lookup_state field.
See https://github.com/falcosecurity/falco/actions/runs/8802240019?pr=3177:

TestFalco_Legacy_ContainerPrivileged
{"deadline":180000000000,"level":"info","msg":"running falco with runner","time":"2024-04-24T08:33:14Z"}
{"cmd":"/usr/bin/falco -c /etc/falco/falco.yaml -o json_output=true -r falco_rules.yaml -o engine.kind=replay -o engine.replay.capture_file=container-privileged.scap -A -o json_include_output_property=false -o json_include_tags_property=false -o log_level=debug -o log_stderr=true -o log_syslog=false -o stdout_output.enabled=true","level":"debug","msg":"executing command","time":"2024-04-24T08:33:14Z"}
legacy_test.go:2526:
Error Trace: /home/runner/work/_actions/falcosecurity/testing/main/legacy_test.go:2526
Error: Not equal:
expected: 3
actual : 2
Test: TestFalco_Legacy_ContainerPrivileged

Does this PR introduce a user-facing change?:

NONE

…rsing old container json events.

Signed-off-by: Federico Di Pierro <[email protected]>
@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 24, 2024

/milestone 0.16.0
/cc @incertum

@poiana poiana requested a review from incertum April 24, 2024 10:04
@poiana poiana added this to the 0.16.0 milestone Apr 24, 2024
@poiana poiana added the size/XS label Apr 24, 2024
@poiana poiana requested review from hbrueckner and leogr April 24, 2024 10:04
@FedeDP
Copy link
Contributor Author

FedeDP commented Apr 24, 2024

Without the change:

Events detected: 2
Rule counts by severity:
   INFO: 2
Triggered rules by rule name:
   Launch Sensitive Mount Container: 2

With the change:

Events detected: 3
Rule counts by severity:
   INFO: 3
Triggered rules by rule name:
   Launch Sensitive Mount Container: 3

@@ -5125,6 +5125,11 @@ void sinsp_parser::parse_container_json_evt(sinsp_evt *evt)
container_info->set_lookup_status(sinsp_container_lookup::state::FAILED);
}
}
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is useful only for old scap-files

Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@poiana
Copy link
Contributor

poiana commented Apr 24, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, leogr, LucaGuerra

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:
  • OWNERS [Andreagit97,FedeDP,LucaGuerra,leogr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit b373489 into master Apr 24, 2024
34 of 35 checks passed
@poiana poiana deleted the fix/container_engine_lookup_default branch April 24, 2024 15:28
@FedeDP FedeDP mentioned this pull request Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants