fix(container): crash at filter extract #1019
Conversation
Rules files suggestions |
gnosek
left a comment
gnosek
left a comment
There was a problem hiding this comment.
LGTM, not sure why CI is unhappy though
I found this in the CI output: I think this has something to do with #1005 / #1016. |
|
CI is still using an old |
How did the previous one pass then? |
|
What is the previous one? Do you mean the previous merged PR? It failed as well: https://github.com/falcosecurity/plugins/actions/runs/18351059430/job/52271109104 |
|
FYI the CI will be fixed once falcosecurity/falco#3689 gets merged. You can ignore it at the moment. |
deepskyblue86
force-pushed
the
fix/container-filter-extract
branch
from
31e48a4 to
45cfeff
Compare
Rules files suggestions |
FYI it works now 😎 |
irozzo-1A
left a comment
irozzo-1A
left a comment
There was a problem hiding this comment.
LGTM, just a nit on a comment
|
LGTM label has been added. DetailsGit tree hash: 3285c4b0d4228f891f1dcc4392292883522af8f0 |
leogr
left a comment
leogr
left a comment
There was a problem hiding this comment.
SGTM. Just left a minor comment, but not a blocker.
I let the CI run, then I will approve eventually,.
| // Deprecated fields don't extract anything, deal with them immediately | ||
| if(field_id == TYPE_K8S_RC_NAME || field_id == TYPE_K8S_RC_ID || | ||
| field_id == TYPE_K8S_RC_LABEL || field_id == TYPE_K8S_RC_LABELS || | ||
| field_id == TYPE_K8S_SVC_NAME || field_id == TYPE_K8S_SVC_ID || | ||
| field_id == TYPE_K8S_SVC_LABEL || field_id == TYPE_K8S_SVC_LABELS || | ||
| field_id == TYPE_K8S_NS_ID || field_id == TYPE_K8S_NS_LABEL || | ||
| field_id == TYPE_K8S_NS_LABELS || field_id == TYPE_K8S_RS_NAME || | ||
| field_id == TYPE_K8S_RS_ID || field_id == TYPE_K8S_RS_LABEL || | ||
| field_id == TYPE_K8S_RS_LABELS || field_id == TYPE_K8S_DEPLOYMENT_NAME || | ||
| field_id == TYPE_K8S_DEPLOYMENT_ID || | ||
| field_id == TYPE_K8S_DEPLOYMENT_LABEL || | ||
| field_id == TYPE_K8S_DEPLOYMENT_LABELS) | ||
| { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Just nit:
Why should we pay this little cost assuming TYPE_K8S_* would never pass here in most cases?
Do nothing may be better, IMO.
Rules files suggestions |
Signed-off-by: Angelo Puglisi <angelopuglisi86@gmail.com>
deepskyblue86
force-pushed
the
fix/container-filter-extract
branch
from
c7335b5 to
f424638
Compare
Rules files suggestions |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deepskyblue86, irozzo-1A, leogr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM label has been added. DetailsGit tree hash: 1a04486c22afcfa196023aa5a4e48ab3a4683b6b |
6 participants
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area plugins
What this PR does / why we need it:
I hit a segfault with sinsp-example, just specifying
-f 'container.name!=my_container'.After debugging it I realized that event filtering (using extract cap) happens before parse, so the assumption in the code didn't hold.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: