-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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][RAC] - Update UI signal references #107713
Conversation
e08a013
to
de21f63
Compare
'signal.status': 'open', | ||
'signal.rule.name': 'Rawr', | ||
[ALERT_STATUS]: 'open', | ||
[ALERT_RULE_NAME]: 'Rawr', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rawr 😂
'signal.rule.note': 'signal.rule.note', | ||
'signal.rule.threshold': 'signal.rule.threshold', | ||
'signal.rule.exceptions_list': 'signal.rule.exceptions_list', | ||
export const alertFieldsMap: Readonly<Record<string, string>> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this mapping from/to? As far as I can tell, it's only used in eventFieldsMap
and that map doesn't appear to be used by anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, this file is actually ported across three different plugins and I don't think any of them are being used. I'll delete them and see what happens 🤷🏾♂️
@@ -162,14 +163,14 @@ describe('Events Details Helpers', () => { | |||
}, | |||
{ | |||
category: 'signal', | |||
field: 'signal.status', | |||
field: ALERT_STATUS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear to me if we're using ALERT_STATUS
or ALERT_WORKFLOW_STATUS
(ALERT_STATUS
isn't actually in the spreadsheet). Open question...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty sure it's going to be ALERT_WORKFLOW_STATUS, we'll update these
@@ -44,10 +45,11 @@ export interface Ecs { | |||
event?: EventEcs; | |||
geo?: GeoEcs; | |||
host?: HostEcs; | |||
kibana?: KibanaEcs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were you able to verify that these are coming back as nested objects, rather than a flattened set of key/values at the top level of the Ecs
object? On the backend, we're handling as a flat set of key/value pairs before indexing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
export const ALERT_RULE_VERSION = '[data-test-subj="draggable-content-signal.rule.version"]'; | ||
export const ALERT_RULE_VERSION = '[data-test-subj="draggable-content-kibana.alert.rule.version"]'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use ALERT_NAMESPACE
here.
Lookin' good, thanks for adding all the constants! |
de21f63
to
3c3850a
Compare
* must be replaced by `ALERT_WORKFLOW_STATUS` field name constant | ||
* @deprecated | ||
*/ | ||
const replaceStatusField = (query: string): string => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@semd just updated the query to use the ALERT_WORKFLOW_STATUS directly
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
x-pack/plugins/security_solution/public/common/mock/mock_detection_alerts.ts
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/detections/components/alerts_table/default_config.tsx
Outdated
Show resolved
Hide resolved
...y_solution/public/detections/components/alerts_table/timeline_actions/alert_context_menu.tsx
Outdated
Show resolved
Hide resolved
...rity_solution/public/detections/containers/detection_engine/rules/use_rule_with_fallback.tsx
Outdated
Show resolved
Hide resolved
⏳ Build in-progress, with failures
Failed CI Steps
Test FailuresKibana Pipeline / general / task-queue-process-15 / X-Pack Endpoint Functional Tests.x-pack/test/security_solution_endpoint/apps/endpoint/endpoint_list·ts.endpoint endpoint list when there is data, finds page titleStandard Out
Stack Trace
Kibana Pipeline / general / task-queue-process-15 / X-Pack Endpoint Functional Tests.x-pack/test/security_solution_endpoint/apps/endpoint/endpoint_list·ts.endpoint endpoint list when there is data, finds page titleStandard Out
Stack Trace
Kibana Pipeline / jest / Jest Tests.x-pack/plugins/security_solution/public/timelines/components/timeline/body/actions.Actions Alert context menu enabled? it enables for eventType=signalStandard Out
Stack Trace
History
To update your PR or re-run it, just comment with: |
@@ -192,7 +191,7 @@ export const buildCombinedQuery = (combineQueriesParams: CombineQueries) => { | |||
const combinedQuery = combineQueries(combineQueriesParams); | |||
return combinedQuery | |||
? { | |||
filterQuery: replaceStatusField(combinedQuery!.filterQuery), | |||
filterQuery: combinedQuery!.filterQuery, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UPDATE: This PR was closed in favor of: #112113
NOTE: This PR will be merged for 7.16 to not cause any disruptions to the 7.15 release
TODO: If any column configurations are stored in localStorage, those will need to be updated from
signal.*
to the newkibana.alert.*
referencesSummary
This work coincides with the work done in #105096 and #106049 to migrate the
signal.x
tokibana.alert.x
fields in the ui.Testing: