Skip to content

[Security Solution][Detections] Extend alerts schema to accommodate the list of assigned users (#7647)#166845

Merged
e40pud merged 5 commits intoelastic:security/feature/alert-user-assignmentfrom
e40pud:security/feature/alert-user-assignment-7647
Sep 22, 2023
Merged

[Security Solution][Detections] Extend alerts schema to accommodate the list of assigned users (#7647)#166845
e40pud merged 5 commits intoelastic:security/feature/alert-user-assignmentfrom
e40pud:security/feature/alert-user-assignment-7647

Conversation

@e40pud
Copy link
Contributor

@e40pud e40pud commented Sep 20, 2023

Summary

Closes https://github.com/elastic/security-team/issues/7647

This PR extends alert's schema. We add a new field kibana.alert.workflow_assignee_ids where assignees will live.

@e40pud e40pud added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. release_note:feature Makes this part of the condensed release notes Team:Detection Engine Security Solution Detection Engine Area labels Sep 20, 2023
@e40pud e40pud self-assigned this Sep 20, 2023
@e40pud e40pud marked this pull request as ready for review September 20, 2023 19:39
@e40pud e40pud requested review from a team as code owners September 20, 2023 19:39
@e40pud e40pud requested a review from a team September 20, 2023 19:39
@e40pud e40pud requested a review from a team as a code owner September 20, 2023 19:39
@elasticmachine
Copy link
Contributor

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

@e40pud e40pud requested review from a team, marshallmain and yctercero and removed request for a team September 20, 2023 19:41
@e40pud e40pud requested a review from a team September 21, 2023 13:11
export type { Ancestor890 as Ancestor8110 };

export interface BaseFields8110 extends BaseFields890 {
[ALERT_WORKFLOW_ASSIGNEE_IDS]: string[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as ALERT_WORKFLOW_ASSIGNEE_IDS defined as not required

  [ALERT_WORKFLOW_ASSIGNEE_IDS]: {
    type: 'keyword',
    array: true,
    required: false,
  },

should it be added here as optional property?

Copy link
Contributor Author

@e40pud e40pud Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question, I mimicked the same behaviour we have for workflow_tags that we introduced in 8.9 https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/common/api/detection_engine/model/alerts/8.9.0/index.ts#L29. It is also not required.

@dplumlee do you remember why we skipped making tags optional property?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made this field optional, @dplumlee let me know if there is a reason to keep it required here.

Copy link
Contributor

@dplumlee dplumlee Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recall if there was a specific reason for not making the tags optional here, I believe since it was a new field, our use case just wanted to always count on it being there. I think the right way to go is keeping it aligned with the field maps definition as @vitaliidm points out though, I probably shouldn't have listed ours as optional in that file.

Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from explore side, thanks!

@e40pud e40pud requested a review from vitaliidm September 22, 2023 11:58
Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks good to me
Left comment on optional typing: with undefined it still will be required to pass everywhere, so proposing to use question mark
just one, note, once you make ALERT_WORKFLOW_ASSIGNEE_IDS I don't think you need to add empty array everywhere

@kibana-ci
Copy link

💔 Build Failed

Failed CI Steps

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [d508c9e]

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @e40pud

@e40pud e40pud merged commit 23f64ed into elastic:security/feature/alert-user-assignment Sep 22, 2023
@e40pud e40pud deleted the security/feature/alert-user-assignment-7647 branch October 11, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:feature Makes this part of the condensed release notes Team:Detection Engine Security Solution Detection Engine Area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants