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.

Sign up for GitHub

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

Jump to bottom

[SecuritySolution] Revamp entity analytics Open API schemas #182666

Merged
merged 19 commits into from
May 16, 2024
Merged

[SecuritySolution] Revamp entity analytics Open API schemas #182666

merged 19 commits into from
May 16, 2024

Conversation

machadoum
Copy link
Member

@machadoum machadoum commented May 6, 2024 •
edited
Loading

Summary

  • Declare all API types using Open API Schema (OAS).
  • Generate TS types from schemas
  • Update the code to use the generated types
  • Validate requests with buildRouteValidationWithZod
  • Delete x-pack/plugins/security_solution/common/api/entity_analytics/risk_engine/common.schema.yaml. There were 2 common files.
  • Auto format yaml files for consistency.
  • Rename yaml files from {name}_schema.yml to {name}.schema.yml. Otherwise, they don't work with the generator.
  • Rename RiskScore to EntityRiskScoreRecord (requested by Maxim)
  • Update yaml version to '1'

@machadoum
Copy link
Member Author

/ci

@machadoum
Copy link
Member Author

/ci

@machadoum machadoum self-assigned this May 13, 2024
@machadoum machadoum added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Entity Analytics Security Entity Analytics Team technical debt Improvement of the software architecture and operational architecture labels May 13, 2024
@machadoum
Copy link
Member Author

/ci

@machadoum machadoum changed the title wip [SecuritySolution] Revamp entity analytics Open API schemas May 13, 2024
@machadoum machadoum marked this pull request as ready for review May 13, 2024 13:43
@machadoum machadoum requested a review from a team as a code owner May 13, 2024 13:43
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-entity-analytics (Team:Entity Analytics)

@machadoum machadoum added the release_note:skip Skip the PR/issue when compiling release notes label May 13, 2024
@machadoum machadoum requested a review from maximpn May 13, 2024 13:43
@machadoum
Copy link
Member Author

/ci

@machadoum machadoum requested review from a team as code owners May 14, 2024 08:56
semd
semd approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@semd semd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Threat hunting explore LGTM

hop-dev
hop-dev approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@hop-dev hop-dev left a comment

Choose a reason for hiding this comment

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

Love it! 🚀

I think I prefer using the RiskEngineStatusEnum type instead of RiskEngineStatus.enum but its completely a matter of opinion so not blocking this from being merged 👍

x-pack/plugins/security_solution/common/entity_analytics/risk_engine/risk_levels.ts Outdated Show resolved Hide resolved
...k/plugins/security_solution/common/search_strategy/security_solution/risk_score/all/index.ts Outdated Show resolved Hide resolved
.../security_solution/public/entity_analytics/api/hooks/use_calculate_entity_risk_score.test.ts Outdated Show resolved Hide resolved
PhilippeOberti
PhilippeOberti approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

LGTM for the Threat Hunting Investigations team!

maximpn
maximpn approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@machadoum thank you for addressing Entity Analytics OAS issues and making them better 🙏

There is no public API endpoint here so there is no immediate impact on Serverless API documentation effort. But it's good to have ready to use OAS when we need them. Rule Management team have work related to UI and test clients generation in the backlog. AS part of this task bundling public and internal OAS will be necessary.

I tested revamped OAS files with @kbn/openapi-bundler and didn't find any issues.

While I didn't review all the changes I left a few comments I found on discrepancies.

...ity_solution/common/api/entity_analytics/asset_criticality/get_asset_criticality.schema.yaml Outdated Show resolved Hide resolved
...k/plugins/security_solution/common/api/entity_analytics/asset_criticality/common.schema.yaml Outdated Show resolved Hide resolved
x-pack/plugins/security_solution/common/api/entity_analytics/common/common.schema.yaml Outdated Show resolved Hide resolved
@machadoum
Implemet several code review suggestions
e02bf4e 
* Rename EntityRiskScore type
* Use enum type when possible
* Update yaml to use sinigle quote
* Rename EntityRiskScore to EntityRiskScoreRecord
@machadoum
Update open api yaml version
84b2662
@machadoum machadoum enabled auto-merge (squash) May 15, 2024 08:19
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5474 5477 +3

Async chunks

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

id before after diff
securitySolution 15.1MB 15.1MB +1.7KB

History

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

cc @machadoum

@machadoum machadoum merged commit a65b688 into elastic:main May 16, 2024
36 checks passed
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maximpn maximpn maximpn approved these changes

@hop-dev hop-dev hop-dev approved these changes

@PhilippeOberti PhilippeOberti PhilippeOberti approved these changes

@semd semd semd approved these changes

@jaredburgettelastic jaredburgettelastic Awaiting requested review from jaredburgettelastic jaredburgettelastic is a code owner automatically assigned from elastic/security-entity-analytics

Assignees

@machadoum machadoum

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Entity Analytics Security Entity Analytics Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture v8.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@machadoum @elasticmachine @kibana-ci @hop-dev @maximpn @PhilippeOberti @semd @kibanamachine