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][Detections] Stop updating rule status on enable/disable #119596

Closed
Tracked by #118324
banderror opened this issue Nov 24, 2021 · 3 comments · Fixed by #120088, #120161 or #120162
Closed
Tracked by #118324

[Security Solution][Detections] Stop updating rule status on enable/disable #119596

banderror opened this issue Nov 24, 2021 · 3 comments · Fixed by #120088, #120161 or #120162
Assignees
@banderror
Labels
Feature:Rule Monitoring Security Solution Detection Rule Monitoring 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. technical debt Improvement of the software architecture and operational architecture v7.16.1

Comments

@banderror
Copy link
Contributor

banderror commented Nov 24, 2021 •
edited
Loading

Parent ticket: #118324

Summary

Currently, when a user enables a rule, we update its status to going to run immediately from the endpoint handler (and the user is able to see the rule switching its status in the Rule Management table or on the Details page). We should not do this update, because that causes a race condition between the endpoint and the rule executor that updates the status to going to run as soon as the rule starts. This can happen almost immediately after calling the endpoint and we've already seen issues caused by this race condition (#119334).

Stop updating rule status SO in the endpoint handler that implements enabling/disabling a rule.

If this is not possible for any reason, handle the race condition using Optimistic Concurrency Control of Elasticsearch with the version field that every SO has.
@botelastic botelastic bot added the needs-team Issues missing a team label label Nov 24, 2021
@banderror banderror added technical debt Improvement of the software architecture and operational architecture Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. resilience Issues related to Platform resilience in terms of scale, performance & backwards compatibility v8.1.0 Team:Detection Rule Management Security Detection Rule Management Team labels Nov 24, 2021
@botelastic botelastic bot removed the needs-team Issues missing a team label label Nov 24, 2021
@banderror banderror mentioned this issue Nov 24, 2021
Open
19 tasks
@banderror banderror added Feature:Rule Monitoring Security Solution Detection Rule Monitoring area and removed resilience Issues related to Platform resilience in terms of scale, performance & backwards compatibility labels Nov 24, 2021
@banderror banderror self-assigned this Nov 24, 2021
@banderror
Copy link
Contributor Author

@jethr0null just want to double-check that there's no objections against that from the product standpoint.

@spong spong mentioned this issue Nov 24, 2021
Merged
@rylnd
Copy link
Contributor

rylnd commented Nov 29, 2021

@banderror I confirmed with product that the change makes sense; one less status update to cause a conflict 👍

@banderror
Copy link
Contributor Author

Thank you @rylnd 🙌

@banderror banderror mentioned this issue Dec 1, 2021
Merged
1 task
@banderror banderror added v7.16.1 and removed v8.1.0 labels Dec 1, 2021
banderror added a commit that referenced this issue Dec 1, 2021
@banderror
[Security Solution][Detections] Fix 409 conflict error happening when…
571adbb 
… user enables a rule (#120088)

**Tickets:** #119334, #119596

## Summary

With this PR, we do not update rule execution status to `going to run` for a rule immediately when the user enables this rule. Instead, the rule executor now does this as soon as the rule starts.

This should fix the 409 conflict error that has been happening due to a race condition between the route handler and the executor both changing the status to `going to run` at almost the same time.

### Checklist

- [ ] [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
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Dec 1, 2021
@banderror @kibanamachine
[Security Solution][Detections] Fix 409 conflict error happening when…
5b58f1f 
… user enables a rule (elastic#120088)

**Tickets:** elastic#119334, elastic#119596

## Summary

With this PR, we do not update rule execution status to `going to run` for a rule immediately when the user enables this rule. Instead, the rule executor now does this as soon as the rule starts.

This should fix the 409 conflict error that has been happening due to a race condition between the route handler and the executor both changing the status to `going to run` at almost the same time.

### Checklist

- [ ] [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
banderror added a commit to banderror/kibana that referenced this issue Dec 1, 2021
@banderror
[Security Solution][Detections] Fix 409 conflict error happening when…
aaeb846 
… user enables a rule (elastic#120088)

**Tickets:** elastic#119334, elastic#119596

## Summary

With this PR, we do not update rule execution status to `going to run` for a rule immediately when the user enables this rule. Instead, the rule executor now does this as soon as the rule starts.

This should fix the 409 conflict error that has been happening due to a race condition between the route handler and the executor both changing the status to `going to run` at almost the same time.

### Checklist

- [ ] [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

# Conflicts:
#	x-pack/plugins/security_solution/server/lib/detection_engine/rules/enable_rule.ts
This was linked to pull requests Dec 1, 2021
[Security Solution][Detections] Fix 409 conflict error happening when user enables a rule #120088
Merged
[8.0] [Security Solution][Detections] Fix 409 conflict error happening when user enables a rule (#120088) #120161
Merged
[7.16] [Security Solution][Detections] Fix 409 conflict error happening when user enables a rule (#120088) #120162
Merged
kibanamachine added a commit that referenced this issue Dec 1, 2021
@kibanamachine @banderror
[Security Solution][Detections] Fix 409 conflict error happening when…
e94156f 
… user enables a rule (#120088) (#120161)

**Tickets:** #119334, #119596

## Summary

With this PR, we do not update rule execution status to `going to run` for a rule immediately when the user enables this rule. Instead, the rule executor now does this as soon as the rule starts.

This should fix the 409 conflict error that has been happening due to a race condition between the route handler and the executor both changing the status to `going to run` at almost the same time.

### Checklist

- [ ] [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

Co-authored-by: Georgii Gorbachev <[email protected]>
banderror added a commit that referenced this issue Dec 1, 2021
@banderror
[Security Solution][Detections] Fix 409 conflict error happening when…
042318c 
… user enables a rule (#120088) (#120162)

**Tickets:** #119334, #119596

## Summary

With this PR, we do not update rule execution status to `going to run` for a rule immediately when the user enables this rule. Instead, the rule executor now does this as soon as the rule starts.

This should fix the 409 conflict error that has been happening due to a race condition between the route handler and the executor both changing the status to `going to run` at almost the same time.

### Checklist

- [ ] [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

# Conflicts:
#	x-pack/plugins/security_solution/server/lib/detection_engine/rules/enable_rule.ts
TinLe pushed a commit to TinLe/kibana that referenced this issue Dec 22, 2021
@banderror @TinLe
[Security Solution][Detections] Fix 409 conflict error happening when…
fbd226b 
… user enables a rule (elastic#120088)

**Tickets:** elastic#119334, elastic#119596

## Summary

With this PR, we do not update rule execution status to `going to run` for a rule immediately when the user enables this rule. Instead, the rule executor now does this as soon as the rule starts.

This should fix the 409 conflict error that has been happening due to a race condition between the route handler and the executor both changing the status to `going to run` at almost the same time.

### Checklist

- [ ] [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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@banderror banderror

Labels
Feature:Rule Monitoring Security Solution Detection Rule Monitoring 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. technical debt Improvement of the software architecture and operational architecture v7.16.1
Projects
None yet
Milestone
No milestone
2 participants
@rylnd @banderror