elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.69k stars 8.12k forks source link

[Security Solution] Substitute `MissingVersion` symbol in the `ThreeWayDiff` object with a boolean #188277

Closed nikitaindik closed 1 month ago

nikitaindik commented 1 month ago

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

Summary

TL;DR: We can't use the MissingVersion symbol in UI code because it's getting cut away from backend responses.

When calculating a three-way diff for a rule field, we take the value of this field from each of the three rule versions: base, current, and target. In case the base version of a rule is missing, we set the field's value to a MissingVersion symbol.

Thus, we pass an object of this shape to a diffing algorithm function, like simpleDiffAlgorithm:

// Diffing algorithm input - 3 versions of a field value
{
     base_version: field value | MissingVersion,
     current_version: field value,
     target_version: field value
}

Each diffing algorithm function returns a ThreeWayDiff object that is later sent to the front end.

// Diffing algorithm output (ThreeWayDiff)
{
     base_version, // field value or MissingVersion
     current_version, // field value
     target_version, // field value
     merged_version, // field value
     diff_outcome,
     merge_outcome,
     has_update,
     has_conflict
}

Before being sent to the FE the ThreeWayDiff gets converted into a JSON. And so base_version: MissingVersion symbol becomes base_version: undefined since JS symbols don't exist in JSON.

This makes using the ThreeWayDiff TS type unsafe on the FE because the received object can now have base_version equal to undefined, while the TS type "guarantees" base_version to be either a symbol or an actual field value.

One way to solve this would be to not use the MissingVersion symbol in ThreeWayDiff but to add a boolean property like has_base_version.

// Updated diffing algorithm output (ThreeWayDiff)
{
     has_base_version, // boolean
     base_version, // field value or undefined
     current_version, // field value
     target_version, // field value
     merged_version, // field value
     diff_outcome,
     merge_outcome,
     has_update,
     has_conflict
}
elasticmachine commented 1 month ago

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

elasticmachine commented 1 month ago

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

elasticmachine commented 1 month ago

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