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

[Security Solution] Implement single-line string diff algorithm #180158

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

[Security Solution] Implement single-line string diff algorithm #180158

jpdjere opened this issue Apr 5, 2024 · 6 comments
Assignees
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

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

Summary

Implement an algorithm for diffing and merging changes in single-line string 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-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@banderror banderror changed the title [Security Solution] Implement single-line string fields diff algorithm [Security Solution] Implement single-line string fields diff algorithm (DRAFT) Apr 17, 2024
@dplumlee
Copy link
Contributor

dplumlee commented May 22, 2024

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

Common fields

  • name
  • severity (Enum treated as string field for diff logic purposes)
  • license

Non-ML rule type fields

  • type
  • language
  • query

EQL fields

  • event_category_override
  • timestamp_field
  • tiebreaker_field

New terms fields

  • history_window_start

@banderror banderror changed the title [Security Solution] Implement single-line string fields diff algorithm (DRAFT) [Security Solution] Implement single-line string diff algorithm May 24, 2024
dplumlee added a commit that referenced this issue May 31, 2024
## Summary

Adds unit tests in accordance to
#180158

Abstracts the `simpleDiffAlgorithm` function used in the prebuilt rule
upgrade workflow into `singleLineStringDiffAlgorithm` and
`numberDiffAlgorithm` and adds unit tests for both cases

Addresses the following test cases defined in the [related RFC
section](https://github.com/elastic/kibana/blob/4c9ab711b2a59ebec60ce5f1de18122d7405f9a0/x-pack/plugins/security_solution/docs/rfcs/detection_response/prebuilt_rules_customization.md#single-line-string-fields)
table

- [x] AAA
- [x] ABA
- [x] AAB
- [x] ABB
- [x] ABC
- [x] -AA
- [x] -AB


### 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
- [ ] [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)
@banderror
Copy link
Contributor

@dplumlee Thanks, the proposal makes sense to me, except:

  • We've already discussed that query fields grouped into a kql_query or eql_query diffable objects will require their own specialized diff algorithms. Internally, these algorithms can use the single-line string diff algo in certain branches of their logic.
  • The rule type field might need its own, stricter diff algorithm, that would produce a non-solvable conflict if there's a type update from Elastic. We'll need dedicated UX for that case: [Security Solution] Implement UI for updating prebuilt rule to a new rule type (MVP) #180395

banderror pushed a commit that referenced this issue Jun 10, 2024
## Summary

Adds test plan in accordance to
#180158

Adds test cases for simple diff algorithm to prebuilt rules'
`Installation and upgrade` test plan

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
dplumlee added a commit that referenced this issue Jun 13, 2024
)

## 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 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
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
Development

No branches or pull requests

4 participants