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

crowdstrike: Return empty events array when no resources in alert, host. #10831

Merged
merged 4 commits into from
Aug 21, 2024

Conversation

kcreddy
Copy link
Contributor

@kcreddy kcreddy commented Aug 21, 2024

Proposed commit message

Return empty events array when no resources in alert and host data-streams.

When there are no resources in first API call, current CEL code returns state.
But this state doesn't have events inside it. As per CEL docs, the events field
is necessary. Without this, the following errors appear and lead to restarting of input.

{"log.level":"error","@timestamp":"2024-08-21T06:26:37.252Z","message":"unexpected missing events array from evaluation","component":{"binary":"filebeat","dataset":"elastic_agent.filebeat","id":"cel-default","type":"cel"},"log":{"source":"cel-default"},"input_source":"http://svc-crowdstrike-host:8090","input_url":"http://svc-crowdstrike-host:8090","ecs.version":"1.6.0","log.logger":"input.cel","log.origin":{"file.line":350,"file.name":"cel/input.go","function":"github.com/elastic/beats/v7/x-pack/filebeat/input/cel.input.run.func1"},"service.name":"filebeat","id":"cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2024-08-21T06:26:37.252Z","message":"unregistering","component":{"binary":"filebeat","dataset":"elastic_agent.filebeat","id":"cel-default","type":"cel"},"log":{"source":"cel-default"},"input_type":"cel","key":"cel-crowdstrike_host-20697e27-0353-4b36-8c17-18f5ea89ae90::http://svc-crowdstrike-host:8090","ecs.version":"1.6.0","log.logger":"metric_registry","log.origin":{"file.line":70,"file.name":"inputmon/input.go","function":"github.com/elastic/beats/v7/libbeat/monitoring/inputmon.NewInputRegistry.func1"},"service.name":"filebeat","id":"cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90::http://svc-crowdstrike-host:8090","uuid":"4ee4a37e-1dde-445d-9f17-2429c435d848","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2024-08-21T06:26:37.252Z","message":"Input 'cel' failed with: input.go:130: input cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90 failed (id=cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90)\n\tunexpected type returned for evaluation events: <nil>","component":{"binary":"filebeat","dataset":"elastic_agent.filebeat","id":"cel-default","type":"cel"},"log":{"source":"cel-default"},"log.logger":"input.cel","log.origin":{"file.line":132,"file.name":"compat/compat.go","function":"github.com/elastic/beats/v7/filebeat/input/v2/compat.(*runner).Start.func1"},"service.name":"filebeat","id":"cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90","ecs.version":"1.6.0","ecs.version":"1.6.0"}

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

By modifying "resources": [] in the config-alert.yml and config-host.yml:

Before: (example with host)
eval "$(elastic-package stack shellinit)" && elastic-package test system --generate -v --data-streams=host

{"log.level":"error","@timestamp":"2024-08-21T06:26:37.252Z","message":"unexpected missing events array from evaluation","component":{"binary":"filebeat","dataset":"elastic_agent.filebeat","id":"cel-default","type":"cel"},"log":{"source":"cel-default"},"input_source":"http://svc-crowdstrike-host:8090","input_url":"http://svc-crowdstrike-host:8090","ecs.version":"1.6.0","log.logger":"input.cel","log.origin":{"file.line":350,"file.name":"cel/input.go","function":"github.com/elastic/beats/v7/x-pack/filebeat/input/cel.input.run.func1"},"service.name":"filebeat","id":"cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2024-08-21T06:26:37.252Z","message":"unregistering","component":{"binary":"filebeat","dataset":"elastic_agent.filebeat","id":"cel-default","type":"cel"},"log":{"source":"cel-default"},"input_type":"cel","key":"cel-crowdstrike_host-20697e27-0353-4b36-8c17-18f5ea89ae90::http://svc-crowdstrike-host:8090","ecs.version":"1.6.0","log.logger":"metric_registry","log.origin":{"file.line":70,"file.name":"inputmon/input.go","function":"github.com/elastic/beats/v7/libbeat/monitoring/inputmon.NewInputRegistry.func1"},"service.name":"filebeat","id":"cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90::http://svc-crowdstrike-host:8090","uuid":"4ee4a37e-1dde-445d-9f17-2429c435d848","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2024-08-21T06:26:37.252Z","message":"Input 'cel' failed with: input.go:130: input cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90 failed (id=cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90)\n\tunexpected type returned for evaluation events: <nil>","component":{"binary":"filebeat","dataset":"elastic_agent.filebeat","id":"cel-default","type":"cel"},"log":{"source":"cel-default"},"log.logger":"input.cel","log.origin":{"file.line":132,"file.name":"compat/compat.go","function":"github.com/elastic/beats/v7/filebeat/input/v2/compat.(*runner).Start.func1"},"service.name":"filebeat","id":"cel-crowdstrike.host-20697e27-0353-4b36-8c17-18f5ea89ae90","ecs.version":"1.6.0","ecs.version":"1.6.0"}

After:
No error messages in agent logs.

eval "$(elastic-package stack shellinit)" && elastic-package test system --generate -v --data-streams=host,alert

--- Test results for package: crowdstrike - START ---
╭─────────────┬─────────────┬───────────┬───────────┬────────┬───────────────╮
│ PACKAGE     │ DATA STREAM │ TEST TYPE │ TEST NAME │ RESULT │  TIME ELAPSED │
├─────────────┼─────────────┼───────────┼───────────┼────────┼───────────────┤
│ crowdstrike │ alert       │ system    │ common    │ PASS   │ 37.831208584s │
│ crowdstrike │ host        │ system    │ common    │ PASS   │ 36.294177167s │
╰─────────────┴─────────────┴───────────┴───────────┴────────┴───────────────╯
--- Test results for package: crowdstrike - END   ---
Done

@kcreddy kcreddy added Integration:crowdstrike CrowdStrike bugfix Pull request that fixes a bug issue Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations] labels Aug 21, 2024
@kcreddy kcreddy marked this pull request as ready for review August 21, 2024 06:47
@kcreddy kcreddy requested a review from a team as a code owner August 21, 2024 06:47
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@elasticmachine
Copy link

elasticmachine commented Aug 21, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@@ -75,7 +75,7 @@ program: |
}
)
).as(state, state.with(
!has(state.resources) ? state : // Exit early due to GET failure or no resources to collect.
!has(state.resources) ? {"events": []} : // Exit early due to GET failure or no resources to collect.
Copy link
Contributor

Choose a reason for hiding this comment

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

That changes a couple of other things:

  • it doesn't return the calculated want_more and offset (maybe not important)
  • it doesn't return the error event in the case of a GET failure (important)

I think the change should instead be to insert "events": [], below the ?"resources" line.

Same for both data streams.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. I updated the CEL code with your suggestion and re-ran the system tests. No error messages in the logs, so its working.

@kcreddy kcreddy requested a review from chrisberkhout August 21, 2024 10:39
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

When there are no resources in first API call, current CEL code returns state

Can we mock this in our system test scenario? Like have it return no resources in a response before it returns resources.

@kcreddy
Copy link
Contributor Author

kcreddy commented Aug 21, 2024

Can we mock this in our system test scenario? Like have it return no resources in a response before it returns resources.

I was unable to mock the no resources scenario at the beginning, but at the end. i.e., first it returns resources and subsequent calls returns no resources. Commit for this change: eb72037.

The issue I had with making it at the beginning was that I am unable to increment offset because resources.size() is 0. Whatever offset that I start with, I end up with same value, thus unable to move to next page. https://github.com/elastic/integrations/blob/main/packages/crowdstrike/data_stream/alert/agent/stream/cel.yml.hbs#L55-L58

@elasticmachine
Copy link

💚 Build Succeeded

History

@kcreddy kcreddy requested a review from andrewkroh August 21, 2024 13:30
@kcreddy kcreddy merged commit 07bf7ed into elastic:main Aug 21, 2024
5 checks passed
@elasticmachine
Copy link

Package crowdstrike - 1.39.1 containing this change is available at https://epr.elastic.co/search?package=crowdstrike

jvalente-salemstate pushed a commit to jvalente-salemstate/integrations that referenced this pull request Aug 21, 2024
…st. (elastic#10831)

Return empty events array when no resources in alert and host data-streams.

When there are no resources in first API call, current CEL code returns state.
But this state doesn't have events inside it. As per CEL input docs, the events field
is necessary. Without this, the errors occur and lead to restarting of input.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull request that fixes a bug issue Integration:crowdstrike CrowdStrike Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants