[Security Solution] Rule upgrade JSON diff: Hide runtime and internal properties #174789
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nikitaindik
force-pushed
the
hide-runtime-and-internal-properties-in-rule-diff
branch
from
aaca22c to
52b01ae
Compare
nikitaindik
force-pushed
the
hide-runtime-and-internal-properties-in-rule-diff
branch
from
041347c to
5d1316c
Compare
nikitaindik
added
bug
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
jpdjere
reviewed
Jan 16, 2024
| 'revision', | ||
|
|
||
| /* | ||
| This info is not yet exposed by prebuilt rules. |
There was a problem hiding this comment.
I would change this explanation. This updated_at field is not related to the field that we are trying to add in that ticket. That information is going to be called elastic_last_update or something similar and will live within a new prebuilt property.
I would just explain, for this case, that updated_at is regenerated every time the /upgrade/_review endpoint runs and will therefore always show a diff, but it adds no value or make sense for the user.
There was a problem hiding this comment.
Thanks! Updated!
nikitaindik
force-pushed
the
hide-runtime-and-internal-properties-in-rule-diff
branch
from
285a230 to
99b81c4
Compare
jpdjere
approved these changes
Jan 16, 2024
10 tasks
nikitaindik
force-pushed
the
hide-runtime-and-internal-properties-in-rule-diff
branch
from
99b81c4 to
03055c8
Compare
jpdjere
reviewed
Jan 24, 2024
...y_solution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx
Show resolved
Hide resolved
jpdjere
reviewed
Jan 24, 2024
...y_solution/public/detection_engine/rule_management/components/rule_details/rule_diff_tab.tsx
Show resolved
Hide resolved
nikitaindik
force-pushed
the
hide-runtime-and-internal-properties-in-rule-diff
branch
from
03055c8 to
198db9c
Compare
💛 Build succeeded, but was flaky
Failed CI Steps
Test Failures
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @nikitaindik |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jan 25, 2024
… properties (elastic#174789) **Resolves: elastic#174844 ## Summary Hides technical/runtime fields that shouldn't be displayed in the JSON diff view. We used to hide only the `revision` field because it can be confused with `version`. This PR hides more fields. Properties that might be displayed as having diff, but shouldn't: - `actions`: shown as diff if user defined an action for a rule - `exceptions_list`: shown as diff if user defined an exception list for a rule - `execution_summary`: shown as diff if user has enabled a rule and it executed at least once - `enabled`: shown as diff if user enabled a rule that's disabled by default (or vice versa) - `updated_at`: always shown as diff because its value is a timestamp of when an API request made - `from`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code converts value to a different time unit, like 2h -> 7200s - `note`: shown as diff if an old version of a rule didn't define this property, but a new version of a rule has it defined as '' - `threat`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds empty arrays as defaults if threats/techniques/subtechniques weren't set by the user - `machine_learning_job_id`: might be shown as diff if a prebuilt rule uses the legacy string format for this property. On installation, the value is converted into a new array format, which creates a difference between the installed rule (array format) and the update (string format) - `threat_filters`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds null as a default value for "meta" subproperty - `filters`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds [] as a default value - `timestamp_override_fallback_disabled`: might be shown as diff if user has clicked "save" after editing a rule - `meta`: might be shown as diff if user has clicked "save" after editing a rule - `output_index`: unused, shouldn't be shown - `updated_at`, `updated_by`, `created_at`, `created_by`: should be hidden because these are not relevant for the upgrade flow #### Before <img width="1271" alt="Schermafbeelding 2024-01-16 om 13 50 00" src="https://github.com/elastic/kibana/assets/15949146/f72283dc-9a8a-4c95-a9b6-daa84d9356da"> #### After <img width="1271" alt="Schermafbeelding 2024-01-16 om 13 50 36" src="https://github.com/elastic/kibana/assets/15949146/080ef2ea-c108-4d05-8814-0a2ce7f5a0b0"> (cherry picked from commit 5bf935b)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Jan 25, 2024
…nternal properties (#174789) (#175625) # Backport This will backport the following commits from `main` to `8.12`: - [[Security Solution] Rule upgrade JSON diff: Hide runtime and internal properties (#174789)](#174789) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Nikita Indik","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-01-25T17:25:23Z","message":"[Security Solution] Rule upgrade JSON diff: Hide runtime and internal properties (#174789)\n\n**Resolves: https://github.com/elastic/kibana/issues/174844**\r\n\r\n## Summary\r\nHides technical/runtime fields that shouldn't be displayed in the JSON\r\ndiff view.\r\nWe used to hide only the `revision` field because it can be confused\r\nwith `version`. This PR hides more fields.\r\n\r\nProperties that might be displayed as having diff, but shouldn't:\r\n- `actions`: shown as diff if user defined an action for a rule\r\n- `exceptions_list`: shown as diff if user defined an exception list for\r\na rule\r\n- `execution_summary`: shown as diff if user has enabled a rule and it\r\nexecuted at least once\r\n- `enabled`: shown as diff if user enabled a rule that's disabled by\r\ndefault (or vice versa)\r\n- `updated_at`: always shown as diff because its value is a timestamp of\r\nwhen an API request made\r\n- `from`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code converts value to a\r\ndifferent time unit, like 2h -> 7200s\r\n- `note`: shown as diff if an old version of a rule didn't define this\r\nproperty, but a new version of a rule has it defined as ''\r\n- `threat`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code adds empty arrays as\r\ndefaults if threats/techniques/subtechniques weren't set by the user\r\n- `machine_learning_job_id`: might be shown as diff if a prebuilt rule\r\nuses the legacy string format for this property. On installation, the\r\nvalue is converted into a new array format, which creates a difference\r\nbetween the installed rule (array format) and the update (string format)\r\n- `threat_filters`: might be shown as diff if user has clicked \"save\"\r\nafter editing a rule, because edit screen's FE code adds null as a\r\ndefault value for \"meta\" subproperty\r\n- `filters`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code adds [] as a default value\r\n- `timestamp_override_fallback_disabled`: might be shown as diff if user\r\nhas clicked \"save\" after editing a rule\r\n- `meta`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule\r\n- `output_index`: unused, shouldn't be shown\r\n- `updated_at`, `updated_by`, `created_at`, `created_by`: should be\r\nhidden because these are not relevant for the upgrade flow\r\n\r\n\r\n\r\n#### Before\r\n<img width=\"1271\" alt=\"Schermafbeelding 2024-01-16 om 13 50 00\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/f72283dc-9a8a-4c95-a9b6-daa84d9356da\">\r\n\r\n\r\n\r\n#### After\r\n<img width=\"1271\" alt=\"Schermafbeelding 2024-01-16 om 13 50 36\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/080ef2ea-c108-4d05-8814-0a2ce7f5a0b0\">","sha":"5bf935b5c30dd489ce381fc337e674443349940c","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","v8.12.1","v8.13.0"],"title":"[Security Solution] Rule upgrade JSON diff: Hide runtime and internal properties","number":174789,"url":"https://github.com/elastic/kibana/pull/174789","mergeCommit":{"message":"[Security Solution] Rule upgrade JSON diff: Hide runtime and internal properties (#174789)\n\n**Resolves: https://github.com/elastic/kibana/issues/174844**\r\n\r\n## Summary\r\nHides technical/runtime fields that shouldn't be displayed in the JSON\r\ndiff view.\r\nWe used to hide only the `revision` field because it can be confused\r\nwith `version`. This PR hides more fields.\r\n\r\nProperties that might be displayed as having diff, but shouldn't:\r\n- `actions`: shown as diff if user defined an action for a rule\r\n- `exceptions_list`: shown as diff if user defined an exception list for\r\na rule\r\n- `execution_summary`: shown as diff if user has enabled a rule and it\r\nexecuted at least once\r\n- `enabled`: shown as diff if user enabled a rule that's disabled by\r\ndefault (or vice versa)\r\n- `updated_at`: always shown as diff because its value is a timestamp of\r\nwhen an API request made\r\n- `from`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code converts value to a\r\ndifferent time unit, like 2h -> 7200s\r\n- `note`: shown as diff if an old version of a rule didn't define this\r\nproperty, but a new version of a rule has it defined as ''\r\n- `threat`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code adds empty arrays as\r\ndefaults if threats/techniques/subtechniques weren't set by the user\r\n- `machine_learning_job_id`: might be shown as diff if a prebuilt rule\r\nuses the legacy string format for this property. On installation, the\r\nvalue is converted into a new array format, which creates a difference\r\nbetween the installed rule (array format) and the update (string format)\r\n- `threat_filters`: might be shown as diff if user has clicked \"save\"\r\nafter editing a rule, because edit screen's FE code adds null as a\r\ndefault value for \"meta\" subproperty\r\n- `filters`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code adds [] as a default value\r\n- `timestamp_override_fallback_disabled`: might be shown as diff if user\r\nhas clicked \"save\" after editing a rule\r\n- `meta`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule\r\n- `output_index`: unused, shouldn't be shown\r\n- `updated_at`, `updated_by`, `created_at`, `created_by`: should be\r\nhidden because these are not relevant for the upgrade flow\r\n\r\n\r\n\r\n#### Before\r\n<img width=\"1271\" alt=\"Schermafbeelding 2024-01-16 om 13 50 00\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/f72283dc-9a8a-4c95-a9b6-daa84d9356da\">\r\n\r\n\r\n\r\n#### After\r\n<img width=\"1271\" alt=\"Schermafbeelding 2024-01-16 om 13 50 36\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/080ef2ea-c108-4d05-8814-0a2ce7f5a0b0\">","sha":"5bf935b5c30dd489ce381fc337e674443349940c"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/174789","number":174789,"mergeCommit":{"message":"[Security Solution] Rule upgrade JSON diff: Hide runtime and internal properties (#174789)\n\n**Resolves: https://github.com/elastic/kibana/issues/174844**\r\n\r\n## Summary\r\nHides technical/runtime fields that shouldn't be displayed in the JSON\r\ndiff view.\r\nWe used to hide only the `revision` field because it can be confused\r\nwith `version`. This PR hides more fields.\r\n\r\nProperties that might be displayed as having diff, but shouldn't:\r\n- `actions`: shown as diff if user defined an action for a rule\r\n- `exceptions_list`: shown as diff if user defined an exception list for\r\na rule\r\n- `execution_summary`: shown as diff if user has enabled a rule and it\r\nexecuted at least once\r\n- `enabled`: shown as diff if user enabled a rule that's disabled by\r\ndefault (or vice versa)\r\n- `updated_at`: always shown as diff because its value is a timestamp of\r\nwhen an API request made\r\n- `from`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code converts value to a\r\ndifferent time unit, like 2h -> 7200s\r\n- `note`: shown as diff if an old version of a rule didn't define this\r\nproperty, but a new version of a rule has it defined as ''\r\n- `threat`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code adds empty arrays as\r\ndefaults if threats/techniques/subtechniques weren't set by the user\r\n- `machine_learning_job_id`: might be shown as diff if a prebuilt rule\r\nuses the legacy string format for this property. On installation, the\r\nvalue is converted into a new array format, which creates a difference\r\nbetween the installed rule (array format) and the update (string format)\r\n- `threat_filters`: might be shown as diff if user has clicked \"save\"\r\nafter editing a rule, because edit screen's FE code adds null as a\r\ndefault value for \"meta\" subproperty\r\n- `filters`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule, because edit screen's FE code adds [] as a default value\r\n- `timestamp_override_fallback_disabled`: might be shown as diff if user\r\nhas clicked \"save\" after editing a rule\r\n- `meta`: might be shown as diff if user has clicked \"save\" after\r\nediting a rule\r\n- `output_index`: unused, shouldn't be shown\r\n- `updated_at`, `updated_by`, `created_at`, `created_by`: should be\r\nhidden because these are not relevant for the upgrade flow\r\n\r\n\r\n\r\n#### Before\r\n<img width=\"1271\" alt=\"Schermafbeelding 2024-01-16 om 13 50 00\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/f72283dc-9a8a-4c95-a9b6-daa84d9356da\">\r\n\r\n\r\n\r\n#### After\r\n<img width=\"1271\" alt=\"Schermafbeelding 2024-01-16 om 13 50 36\"\r\nsrc=\"https://github.com/elastic/kibana/assets/15949146/080ef2ea-c108-4d05-8814-0a2ce7f5a0b0\">","sha":"5bf935b5c30dd489ce381fc337e674443349940c"}}]}] BACKPORT--> Co-authored-by: Nikita Indik <[email protected]>
CoenWarmer
pushed a commit
to CoenWarmer/kibana
that referenced
this pull request
Feb 15, 2024
… properties (elastic#174789) **Resolves: elastic#174844 ## Summary Hides technical/runtime fields that shouldn't be displayed in the JSON diff view. We used to hide only the `revision` field because it can be confused with `version`. This PR hides more fields. Properties that might be displayed as having diff, but shouldn't: - `actions`: shown as diff if user defined an action for a rule - `exceptions_list`: shown as diff if user defined an exception list for a rule - `execution_summary`: shown as diff if user has enabled a rule and it executed at least once - `enabled`: shown as diff if user enabled a rule that's disabled by default (or vice versa) - `updated_at`: always shown as diff because its value is a timestamp of when an API request made - `from`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code converts value to a different time unit, like 2h -> 7200s - `note`: shown as diff if an old version of a rule didn't define this property, but a new version of a rule has it defined as '' - `threat`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds empty arrays as defaults if threats/techniques/subtechniques weren't set by the user - `machine_learning_job_id`: might be shown as diff if a prebuilt rule uses the legacy string format for this property. On installation, the value is converted into a new array format, which creates a difference between the installed rule (array format) and the update (string format) - `threat_filters`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds null as a default value for "meta" subproperty - `filters`: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds [] as a default value - `timestamp_override_fallback_disabled`: might be shown as diff if user has clicked "save" after editing a rule - `meta`: might be shown as diff if user has clicked "save" after editing a rule - `output_index`: unused, shouldn't be shown - `updated_at`, `updated_by`, `created_at`, `created_by`: should be hidden because these are not relevant for the upgrade flow #### Before <img width="1271" alt="Schermafbeelding 2024-01-16 om 13 50 00" src="https://github.com/elastic/kibana/assets/15949146/f72283dc-9a8a-4c95-a9b6-daa84d9356da"> #### After <img width="1271" alt="Schermafbeelding 2024-01-16 om 13 50 36" src="https://github.com/elastic/kibana/assets/15949146/080ef2ea-c108-4d05-8814-0a2ce7f5a0b0">
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: #174844
Summary
Hides technical/runtime fields that shouldn't be displayed in the JSON diff view.
We used to hide only the
revisionfield because it can be confused withversion. This PR hides more fields.Properties that might be displayed as having diff, but shouldn't be displayed:
actions: shown as diff if user defined an action for a ruleexceptions_list: shown as diff if user defined an exception list for a ruleexecution_summary: shown as diff if user has enabled a rule and it executed at least onceenabled: shown as diff if user enabled a rule that's disabled by default (or vice versa)updated_at: always shown as diff because its value is a timestamp of when an API request madenote: shown as diff if an old version of a rule didn't define this property, but a new version of a rule has it defined as ''threat: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds empty arrays as defaults if threats/techniques/subtechniques weren't set by the usermachine_learning_job_id: might be shown as diff if a prebuilt rule uses the legacy string format for this property. On installation, the value is converted into a new array format, which creates a difference between the installed rule (array format) and the update (string format)threat_filters: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds null as a default value for "meta" subpropertyfilters: might be shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code adds [] as a default valuetimestamp_override_fallback_disabled: might be shown as diff if user has clicked "save" after editing a rulemeta: might be shown as diff if user has clicked "save" after editing a ruleoutput_index: unused, shouldn't be shownupdated_at,updated_by,created_at,created_by: should be hidden because these are not relevant for the upgrade flowAlso the
fromproperty might be incorrectly shown as diff if user has clicked "save" after editing a rule, because edit screen's FE code converts value to a different time unit, like 7200s -> 2h. Since 2h = 7200s, user shouldn't see a diff in cases where the old and new values represent the same duration with different time units.Before
After