Skip to content
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

[Threat Hunting Investigations][OpenAPI] Use timeline's generated enums #189410

Conversation

janmonschke
Copy link
Contributor

@janmonschke janmonschke commented Jul 29, 2024

Summary

Fixes https://github.com/elastic/security-team/issues/10132.

This PR is the first on in a series of PRs to adopt the newly generate OpenAPI types in the timeline server code base. As a first step, we're migrating to the newly generated enums only. This has almost no impact on the schemas and is mostly a one-to-one change.

Despite there being changes in more than 150 files, the review should be pretty straight-forward. The most changes come from the new distinction of enums and the actual type of an enum. Meaning a lot of imports and enum usages needed a simple change.

In some places I found duplicate or unused types and the OpenAPI types still had a couple of minor mistakes.

@janmonschke janmonschke self-assigned this Jul 29, 2024
@janmonschke janmonschke added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 labels Jul 29, 2024
@@ -717,11 +664,6 @@ export interface SortTimeline {
sortOrder: Direction;
}

export interface ExportTimelineNotFoundError {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This type was duplicated in this file, so I removed one of them

export interface KueryFilterQueryResult {
kind?: Maybe<string>;
expression?: Maybe<string>;
}

export interface SerializedKueryQueryResult {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one I moved around to match the dependency flow of these types

zeek = 'zeek',
}
export const RowRendererCount = Object.keys(RowRendererIdEnum).length;
export const RowRendererValues = Object.values(RowRendererId.Values);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Object.values(RowRendererId.Values) was copied in a bunch of places, so I created this shared export

threat_match = 'threat_match',
zeek = 'zeek',
}
export const RowRendererCount = Object.keys(RowRendererIdEnum).length;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to the previous version, but uses RowRendererIdEnum now.

@janmonschke janmonschke changed the title [IN PROGRESS] use timeline's generated enums [Threat Hunting Investigations][OpenAPI] Use timeline's generated enums Aug 2, 2024
@janmonschke janmonschke marked this pull request as ready for review August 2, 2024 08:50
@janmonschke janmonschke requested review from a team as code owners August 2, 2024 08:50
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

type: DataProviderType.optional(),
});

export type DataProviderResult = z.infer<typeof DataProviderResult>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, nice catch here. I'm pretty sure I based the original version of this type here, but that is slightly different from the validation schema. 👌🏾

Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Looks good, did some click through testing around row renderers and timeline templates and those look good as well. Thanks!

@janmonschke
Copy link
Contributor Author

@elasticmachine merge upstream

@janmonschke janmonschke enabled auto-merge (squash) August 6, 2024 07:52
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5614 5615 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 20.5MB 20.5MB +5.9KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
securitySolution 36 33 -3
timelines 18 17 -1
total -4

History

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

cc @janmonschke

@janmonschke janmonschke merged commit 13b15bd into elastic:main Aug 6, 2024
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants