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

[Security Solution] Initial migration of API endpoints to OpenAPI and code generation #164482

Merged
merged 1 commit into from
Aug 25, 2023
Merged

[Security Solution] Initial migration of API endpoints to OpenAPI and code generation #164482

merged 1 commit into from
Aug 25, 2023

Conversation

xcrzx
Copy link
Contributor

@xcrzx xcrzx commented Aug 22, 2023 •
edited
Loading

Part of: https://github.com/elastic/security-team/issues/6726

Summary

Migrates the prebuilt rules and timelines status API route schema to OpenAPI. This is exploratory work to assess the level of effort required to migrate API route schemas from io-ts to zod generated by OpenAPI codegen.

Summary of the changes:

  • Added a CI job that runs code generation in Security Solution and comments change if there are any.
  • Migrated the /api/detection_engine/rules/prepackaged/_status route to use generated zod schemas
  • Updated schema tests
  • Adjusted the code generator templates to handle strict schemas, i.e., schemas that do not allow any extra params
  • Updated the error transformation code to work with zod errors. Validation errors are converted to string representations, like the following:
image

@xcrzx xcrzx self-assigned this Aug 22, 2023
@xcrzx xcrzx force-pushed the openapi-migration branch 8 times, most recently from fa986d3 to e268bd6 Compare August 23, 2023 16:34
@xcrzx xcrzx marked this pull request as ready for review August 23, 2023 16:43
@xcrzx xcrzx requested review from a team as code owners August 23, 2023 16:43
@xcrzx xcrzx requested a review from dplumlee August 23, 2023 16:43
@xcrzx xcrzx added release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team v8.11.0 labels Aug 23, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

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

xcrzx
xcrzx commented Aug 23, 2023
View reviewed changes
.buildkite/scripts/common/util.sh
@@ -31,7 +31,7 @@ check_for_changed_files() {

SHOULD_AUTO_COMMIT_CHANGES="${2:-}"
CUSTOM_FIX_MESSAGE="${3:-}"
GIT_CHANGES="$(git ls-files --modified -- . ':!:.bazelrc')"
GIT_CHANGES="$(git status --porcelain -- . ':!:.bazelrc')"
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 line had to be changed to support detecting files added or removed by code generation scripts.

jbudz
jbudz approved these changes Aug 23, 2023
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

If auto committing docs ends up causing too many changes on pull requests and there's interest in an alternative daily build feel free to reach out.

We run the jsdoc API builds daily and auto commit those changes to the repository.

banderror
banderror approved these changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Reviewed the PR together with @xcrzx over Zoom. It looks awesome! There were a few comments and action items that Dmitrii will post as comments and address within the PR.

@xcrzx xcrzx force-pushed the openapi-migration branch from e268bd6 to de960ef Compare August 25, 2023 09:55
@xcrzx xcrzx force-pushed the openapi-migration branch from de960ef to 62a5efd Compare August 25, 2023 14:41
@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 4454 4464 +10

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/securitysolution-es-utils 76 78 +2

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.7MB 15.9MB ⚠️ +261.1KB
securitySolutionServerless 259.9KB 266.2KB +6.2KB
total +267.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolutionServerless 38.4KB 38.4KB +1.0B
Unknown metric groups

API count

id before after diff
@kbn/securitysolution-es-utils 87 89 +2

History

  • 💛 Build #153446 was flaky de960ef6336b2241647cb3bafb515777d05d8433
  • 💔 Build #152866 failed e268bd6037854eac546771182e62d5eda9bee4dd
  • 💔 Build #152827 failed fa986d30154e046f42728ac4bd6e73f57e708612
  • 💔 Build #152810 failed 2ec6f14583aad94e90e439f2f177f320626beda4
  • 💔 Build #152789 failed d636286eec1e086f4124dc051642606fabe593ff

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

cc @xcrzx

@xcrzx xcrzx merged commit 2171ecc into elastic:main Aug 25, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 25, 2023
@marshallmain marshallmain mentioned this pull request Aug 29, 2023
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@banderror banderror banderror approved these changes

@jbudz jbudz jbudz approved these changes

@dhurley14 dhurley14 dhurley14 approved these changes

@dplumlee dplumlee Awaiting requested review from dplumlee dplumlee is a code owner automatically assigned from elastic/security-detection-rule-management

Assignees

@xcrzx xcrzx

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@xcrzx @elasticmachine @kibana-ci @dhurley14 @jbudz @banderror @kibanamachine