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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] Implement number diff algorithm #180160

Closed
8 tasks done
Tracked by #174168
jpdjere opened this issue Apr 5, 2024 · 8 comments
Closed
8 tasks done
Tracked by #174168

[Security Solution] Implement number diff algorithm #180160

jpdjere opened this issue Apr 5, 2024 · 8 comments
Assignees
@dplumlee
Labels
8.15 candidate enhancement New value added to drive a business result Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0

Comments

@jpdjere
Copy link
Contributor

jpdjere commented Apr 5, 2024 •
edited by banderror
Loading

Epics: https://github.com/elastic/security-team/issues/1974 (internal), #174168

Summary

Implement an algorithm for diffing and merging changes in number type of fields of detection rules.

Context from the Rule Customization RFC:

To do
@jpdjere jpdjere added triage_needed Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area labels Apr 5, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@jpdjere jpdjere mentioned this issue Apr 5, 2024
Open
@banderror banderror changed the title [Security Solution] Implement number fields diff algorithm [Security Solution] Implement number fields diff algorithm (DRAFT) Apr 17, 2024
@banderror banderror mentioned this issue May 24, 2024
Open
5 tasks
@banderror banderror changed the title [Security Solution] Implement number fields diff algorithm (DRAFT) [Security Solution] Implement number diff algorithm May 24, 2024
@dplumlee
Copy link
Contributor

Here are the proposed fields that would utilize this diff algorithm:

Common fields

  • version
  • risk_score
  • max_signals

ML fields

  • anomaly_threshold

Threat match fields
(Are we showing these as we decided not to expose them in the UI?)

  • concurrent_searches
  • items_per_search

@xcrzx
Copy link
Contributor

xcrzx commented May 30, 2024

(Are we showing these as we decided not to expose them in the UI?)

  • concurrent_searches
  • items_per_search

Since these are undocumented fields, I'd suggest excluding them from being shown in the UI. We can also skip calculating the diff for these fields and always use the target version if it doesn't match the current.

@dplumlee
Copy link
Contributor

dplumlee commented May 30, 2024 •
edited
Loading

@xcrzx Makes sense to me, I'll remove them in the proposal here but note both of them for edge cases

In which case the final proposed fields that would utilize this diff algorithm would be:

Common fields

  • version
  • risk_score
  • max_signals

ML fields

  • anomaly_threshold

@banderror
Copy link
Contributor

@dplumlee @xcrzx According to #180393:

  • version:
    • should be returned from the API in the diff object
    • should always be updated to the target version of the field under the hood in the upgrade/_perform endpoint
    • should be displayed in the diff UI, but read-only
  • concurrent_searches and items_per_search:
    • should NOT be returned from the API in the diff object
    • should always be updated to the current version of the field under the hood in the upgrade/_perform endpoint
    • should NOT be displayed in the diff UI

The final proposal LGTM ✅

@dplumlee dplumlee mentioned this issue Jun 6, 2024
Merged
3 tasks
dplumlee added a commit that referenced this issue Jun 13, 2024
@dplumlee @kibanamachine
[Security Solution] Integration tests for simple diff algorithm (#184889
0082f45 
)

## Summary

Completes related tickets:
#180160 and
#180158

Switches fields to use the diff algorithms assigned to them in the
related tickets

Adds integration tests in accordance to
#184484 for the `upgrade/_review`
API endpoint for the simple diff algorithm.

Also changes logic in the `upgrade/_review` API endpoint to return user
customized fields in the diffs even if there was not an update for that
field. This new logic is described in
#180154. We filter out the
fields that fall under this new logic so that they are only returned
from the API but not displayed in the per-field rule diff flyout as the
current UI cannot support them. They are utilized in testing logic and
will be implemented in the UI at a later date

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@banderror
Copy link
Contributor

Resolved by:

@banderror banderror added the enhancement New value added to drive a business result label Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dplumlee dplumlee

Labels
8.15 candidate enhancement New value added to drive a business result Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@xcrzx @jpdjere @banderror @elasticmachine @dplumlee