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

Stop IM rule execution if there are no events #123811

Merged
merged 3 commits into from
Jan 28, 2022

Conversation

nkhristinin
Copy link
Contributor

@nkhristinin nkhristinin commented Jan 26, 2022

Summary

Currently, it can be the situation, that the IM rule can be executed, and parse all threat indicators even it has no events.
This optimisation will stop the rule execution if there no events to process.

Checklist

Delete any items that are not applicable to this PR.

@nkhristinin nkhristinin requested review from rylnd and ecezalp January 26, 2022 16:06
@nkhristinin nkhristinin added v8.0.0 Feature:Indicator Match Rule Security Solution Indicator Match rule type Team: CTI release_note:skip Skip the PR/issue when compiling release notes labels Jan 26, 2022
@nkhristinin nkhristinin changed the title Add event count check Stop IM rule execution if there are no events Jan 26, 2022
@nkhristinin nkhristinin marked this pull request as ready for review January 26, 2022 16:09
@nkhristinin nkhristinin requested a review from a team as a code owner January 26, 2022 16:09
@ecezalp
Copy link
Contributor

ecezalp commented Jan 26, 2022

@elasticmachine merge upstream

Copy link
Contributor

@ecezalp ecezalp left a comment

Choose a reason for hiding this comment

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

conceptually LGTM, but for obtaining the count for source events we will always need the tuple, so we should make that default and remove the if statement from the function

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💚 Build #19895 succeeded cf353a2faba2c7e20942664c53e6b32bcb5a3109
  • 💔 Build #19794 failed b0ad9901f5df4b34ee115c8651cdff30ff5b0a0b
  • 💔 Build #19671 failed f5e1630e255585c773cec6a0ab347484c171215a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@ecezalp ecezalp left a comment

Choose a reason for hiding this comment

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

LGTM

@nkhristinin nkhristinin merged commit e774ab4 into elastic:main Jan 28, 2022
@nkhristinin nkhristinin added the auto-backport Deprecated - use backport:version if exact versions are needed label Jan 28, 2022
@kibanamachine
Copy link
Contributor

The following labels were identified as gaps in your version labels and will be added automatically:

  • v8.1.0

If any of these should not be on your pull request, please manually remove them.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 28, 2022
* Add event count check

* Fix linter

* Make tuple required

(cherry picked from commit e774ab4)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@nkhristinin nkhristinin added backport:skip This commit does not require backporting and removed v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jan 28, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jan 28, 2022
…fix-potential-race-condition-when-screenshotting

* 'main' of github.com:elastic/kibana: (75 commits)
  [Reporting] Logging improvements while generating reports (elastic#123802)
  [Uptime] Default alert connectors email settings (elastic#123244)
  Update comparison series styles to match the main series (elastic#123858)
  [RAC][Uptime] remove extra dot from the uptime alert connector message (elastic#124000)
  [Exploratory view] Allow ability add extra actions in lens embeddable (elastic#123713)
  [SecuritySolution][Investigations] Add message about missing index in data view in analyzer (elastic#122859)
  [TSVB] Formatting in the left axis is not respected when I have two separate axis (elastic#123903)
  [Discover] Remove services from component dependencies (elastic#121691)
  Stop IM rule execution if there are no events (elastic#123811)
  [Security Solution][Endpoint] Update Fleet Trusted Apps and Host Isolation Exception cards to use exception list summary API (elastic#123900)
  [Security Solution][Exceptions] Switches modal to flyout component (elastic#123408)
  [Workplace Search] Fix bug where modal visible after deleting a group (elastic#123976)
  [Alerting] Remove state variables from action variable menu (elastic#123702)
  replace deprecated api usage (elastic#123970)
  Fix package policy merge logic for boolean values (elastic#123974)
  [Security Solution][Endpoint][Policy] Remove GET policy list api route (elastic#123873)
  Reenable alert_add test suite (elastic#123862)
  [Fleet] Remove usage of IFieldType in Fleet (elastic#123960)
  [Lists] Add an instance of `ExceptionListClient` with server extension points turned off to context object provided to callbacks (elastic#123885)
  [Maps] Add execution context (elastic#123651)
  ...

# Conflicts:
#	x-pack/plugins/screenshotting/server/browsers/chromium/driver_factory/index.ts
awahab07 pushed a commit to awahab07/kibana that referenced this pull request Jan 31, 2022
* Add event count check

* Fix linter

* Make tuple required
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Indicator Match Rule Security Solution Indicator Match rule type release_note:skip Skip the PR/issue when compiling release notes Team: CTI v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants