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 concurrent_searches and items_per_search fields diff algorithms #188061

Closed
1 task done
Tracked by #174168
jpdjere opened this issue Jul 11, 2024 · 7 comments
Closed
1 task done
Tracked by #174168
Assignees
Labels
8.16 candidate enhancement New value added to drive a business result Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules 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.

Comments

@jpdjere
Copy link
Contributor

jpdjere commented Jul 11, 2024

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

Summary

Implement algorithms for diffing and merging changes in concurrent_searches and items_per_search fields

These two fields require a specialized algorithm because of the following reasons:

  • These fields can be modified via the API, although we've made a decision not to enable modifications via the UI.
  • This can cause conflicts when rule updates that include changes for these fields are released.
  • We want the response of the /upgrade/_review endpoint to include the diff calculation for these fields, but they shouldn't show up in the UI, since that would allow the user to customize it via the UI, during the upgrade workflow.
  • Instead, we will make this algorithm specific so that in cases of an ABC conflict scenario, the value for the conflict prop for these fields will be NO. This way we can ensure that these two fields are not displayed in the upgrade workflow UI. The value for the merged_version should be the current_version.
  • The field diff for an ABC scenario for these two fields should be:
{
  base_version: baseVersion,
  current_version: currentVersion,
  target_version: targetVersion,
  merged_version: mergedVersion, // which equals the `current_version`

  diff_outcome: ThreeWayDiffOutcome.CustomizedValueCanUpdate,
  merge_outcome: ThreeWayMergeOutcome.Merged,
  has_update: true,
  conflict: ThreeWayDiffConflictResolutionResult.NO
};

Context from the Rule Customization RFC:

To do

  • Remove fields from upgrade review workflow
@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 labels Jul 11, 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)

@banderror
Copy link
Contributor

@jpdjere Thanks for creating this ticket. One small reminder, please: every ticket in the backlog should have custom fields filled in, because that's what powers all our tabs in the GH project. I moved it to Inbox for now, but feel free to fill them in and move back to Backlog

Screenshot 2024-07-11 at 11 22 29

@yctercero
Copy link
Contributor

@jpdjere is this work still required if we feel these fields should be deprecated? I guess even if they are we'll need to support past versions having it.

@dplumlee
Copy link
Contributor

I think this work would be needed no matter what, but what might be a better way forward is to delete the fields from the DiffableRule type and instead just always use the current version as we don't seem to push updates to these fields anyways since they're performance based. This will forgo any elastic specific updates but will just automatically keep the user customized fields if they have changed them.

Right now, I'm not sure we'd need a diff algorithm for it as we will never return the fields in the eyes of the users anyways

dplumlee added a commit that referenced this issue Sep 4, 2024
…es` from `upgrade/_review` API endpoint logic (#190440)

## Summary

Addresses #188061

Removes the threat match fields `items_per_search` and
`concurrent_searches` from the `DiffableRule` type we utilize in the
`upgrade/_review` endpoint logic. This omits these fields from the
upgrade review workflow as we will never have incoming updates for the
fields.



### 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: Elastic Machine <[email protected]>
@dplumlee
Copy link
Contributor

dplumlee commented Sep 9, 2024

Closed by #190440

@dplumlee dplumlee closed this as completed Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.16 candidate enhancement New value added to drive a business result Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules 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.
Projects
None yet
Development

No branches or pull requests

5 participants