Open banderror opened 1 year ago
Pinging @elastic/security-detections-response (Team:Detections and Resp)
Pinging @elastic/security-solution (Team: SecuritySolution)
This ticket will be worked on by following the steps:
Step 2 to 6 will be a second, single PR.
fields
object of the Request Payload.
globalPickVersion
from TARGET to MERGED: this is the safest option from a UX perspective - if there's a conflict, we will simply reject the update, instead of doing any unwanted updates.pick_version
. Use this function's return value to pass to the Detection's Client's upgradePrebuiltRule
method.upgradePrebuiltRule
method to allow passing in the base version of a rule, so it doesn't need to be refetched for the calculation of rule_source
.NON_SOLVABLE
:
2.1 If the whole rule is being updated to MERGED, and ANY fields return with a NON_SOLVABLE conflict, reject the whole update for that rule: create new error and add to ruleErrors array.
2.2 EXCEPTION for case above: the whole rule is being updated to MERGED, and one or more of the fields return with a NON_SOLVABLE conflict, BUT those same fields have a specific pick_version
for them in the fields
object which ARE NOT "MERGED". No error should be reported in this case.
2.3 The whole rule is being updated to any pick_version other than MERGED, but any specific field in the fields
object is set to upgrade to "MERGED", and the diff for that fields returns a NON_SOLVABLE conflict. In that case, create new error and add to ruleErrors array.@jpdjere We talked with @xcrzx yesterday and figured that it would be great, besides writing a test plan, to first create a list of edge cases that the endpoint should handle. I see that you have already added such a list to https://github.com/elastic/kibana/issues/166376#issuecomment-2273466115, could you please revisit it and add to the ticket description as an AC?
One thing we discussed is that any rule type change should yield an UNSOLVABLE_CONFLICT
. We can do it via creating a specialized diff algorithm, and @xcrzx created a ticket for that. Still, the question is: how exactly should the /upgrade/_perform
endpoint handle rule type changes. I believe the type
field should not be part of its request schema in the upgradeable fields
object, and nothing from this request schema should impact the logic used for this field. We need to think through details for this edge case.
@jpdjere As a follow-up to https://github.com/elastic/kibana/pull/190440, we'll need to handle the items_per_search and concurrent_searches fields in the upgrade/_perform
endpoint. Please add it to the plan. Also, please check the previous comment.
@banderror
first create a list of edge cases that the endpoint should handle.
I added this to the ticket description.
Still, the question is: how exactly should the /upgrade/_perform endpoint handle rule type changes. I believe the type field should not be part of its request schema in the upgradeable fields object, and nothing from this request schema should impact the logic used for this field.
As part of the preparation PR, the type
was removed from the list of upgradable fields, so it is not part of the request schema. This means that rules can only be upgraded to the target version's type with this endpoint.
I'll include two runtime checks related to this, which I described in the description above:
currentVersion.type !== targetVersion.type
), all pick_version
fields, at all levels, need to match TARGET
. Otherwise, reject the upgrade of that rule.targetVersion.type
(for example, EQL) and the user includes in its fields
object of the request payload any fields which do not match that rule type (in this case, for example, sending in machine_learning_job_id
as part of fields
), throw an error.we'll need to handle the items_per_search and concurrent_searches fields in the upgrade/_perform endpoint. Please add it to the plan. Also, please check the previous comment.
These two fields need to be handled with the rest of fields that need special treatment as described here. In this case, the endpoint will always force their update to the CURRENT
version.
Epics: https://github.com/elastic/security-team/issues/1974 (internal), https://github.com/elastic/kibana/issues/174168
Summary
In https://github.com/elastic/kibana/issues/148184 we implemented a basic version of the
upgrade/_perform
API endpoint.We need to enhance this endpoint so it can work with prebuilt rules customized by users and resolve conflicts between user customizations and updates from Elastic in the target version.
Acceptance criteria
MERGED
version for rule upgrades.If the
MERGED
version is selected, the diffs are recalculated and the rule fields are updated to the result of the diff calculation. This is only possible if all field diffs return aconflict
value of eitherNO
. If any fields returns a value ofNON_SOLVABLE
orSOLVABLE
, reject the request with an error specifying that there are conflicts, and that they must be resolved on a per-field basis.pick_version
isMERGED
.pick_versions
:BASE' | 'CURRENT' | 'TARGET' | 'MERGED' | 'RESOLVED'
(SeeFieldUpgradeRequest
in PoC for details)Handling of special fields
Specific fields need to be handled under the hood based on https://github.com/elastic/kibana/issues/186544
Edge cases
pick_version
, at all levels, matchTARGET
. Otherwise, create new error and add to ruleErrors array.targetVersion.type
(for example, EQL) and the user includes in itsfields
object of the request payload any fields which do not match that rule type (in this case, for example, sending inmachine_learning_job_id
as part offields
), throw an error for that rule.NON_SOLVABLE
:MERGED
, and ANY fields return with aNON_SOLVABLE
conflict, reject the whole update for that rule: create new error and add to ruleErrors array.MERGED
, and one or more of the fields return with aNON_SOLVABLE
conflict, BUT those same fields have a specificpick_version
for them in thefields
object which ARE NOTMERGED
. No error should be reported in this case.pick_version
other than MERGED, but any specific field in thefields
object is set to upgrade toMERGED
, and the diff for that fields returns aNON_SOLVABLE
conflict. In that case, create new error and add to ruleErrors array.