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

[Security Solution] Initial OpenAPI codegen implementation #163186

Merged
merged 2 commits into from
Aug 14, 2023

Conversation

xcrzx
Copy link
Contributor

@xcrzx xcrzx commented Aug 4, 2023

Resolves: https://github.com/elastic/security-team/issues/7134

Summary

Implemented request and response schema generation from OpenAPI specifications.

The code generator script scans the x-pack/plugins/security_solution/common/api directory, locates all *.schema.yaml files, and generates a corresponding *.gen.ts artifact for each, containing zod schema definitions.


Right now, all generation sources are set to x-codegen-enabled: false to prevent the creation of duplicate schemas. Maintaining the old io-ts schemas alongside the new zod ones could potentially lead to confusion among developers. Thus, the recommended migration strategy is to incrementally replace old schema usages with new ones, subsequently removing outdated ones. I'll be implementing this approach in the upcoming PRs.

How to use the generator

If you need to test the generator locally, enable several sources and run the generator script to see the results.

Navigate to x-pack/plugins/security_solution and run yarn openapi:generate

image

Important note: if you want to enable route schemas, ensure you also enable all their dependencies, such as common schemas. Failing to do so will result in the generated code importing non-existent files.

Example

Input file (x-pack/plugins/security_solution/common/api/detection_engine/model/error_schema.schema.yaml):

openapi: 3.0.0
info:
  title: Error Schema
  version: 'not applicable'
paths: {}
components:
  schemas:
    ErrorSchema:
      type: object
      required:
        - error
      properties:
        id:
          type: string
        rule_id:
          $ref: './rule_schema/common_attributes.schema.yaml#/components/schemas/RuleSignatureId'
        list_id:
          type: string
          minLength: 1
        item_id:
          type: string
          minLength: 1
        error:
          type: object
          required:
            - status_code
            - message
          properties:
            status_code:
              type: integer
              minimum: 400
            message:
              type: string

Generated output file (x-pack/plugins/security_solution/common/api/detection_engine/model/error_schema.gen.ts):

/*
 * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
 * or more contributor license agreements. Licensed under the Elastic License
 * 2.0; you may not use this file except in compliance with the Elastic License
 * 2.0.
 */

import { z } from 'zod';

/*
 * NOTICE: Do not edit this file manually.
 * This file is automatically generated by the OpenAPI Generator `yarn openapi:generate`.
 */

import { RuleSignatureId } from './rule_schema/common_attributes.gen';

export type ErrorSchema = z.infer<typeof ErrorSchema>;
export const ErrorSchema = z.object({
  id: z.string().optional(),
  rule_id: RuleSignatureId.optional(),
  list_id: z.string().min(1).optional(),
  item_id: z.string().min(1).optional(),
  error: z.object({
    status_code: z.number().min(400),
    message: z.string(),
  }),
});

@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.10.0 labels Aug 4, 2023
@xcrzx xcrzx self-assigned this Aug 4, 2023
@xcrzx xcrzx force-pushed the openapi-codegen branch 5 times, most recently from 9ed0d08 to 9a1b4c9 Compare August 10, 2023 13:12
@xcrzx xcrzx marked this pull request as ready for review August 10, 2023 16:17
@xcrzx xcrzx requested review from a team as code owners August 10, 2023 16:17
@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 requested a review from marshallmain August 10, 2023 16:26
@banderror banderror requested review from a team and spong and removed request for a team August 10, 2023 16:51
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests #1 / Endpoint Policy Response from Endpoint List page should display policy response with errors should display policy response with errors
  • [job] [logs] Defend Workflows Cypress Tests #1 / Endpoint Policy Response from Fleet Agent Details page should display policy response with errors should display policy response with errors

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
securitySolution 63 64 +1

ESLint disabled line counts

id before after diff
securitySolution 462 463 +1

Total ESLint disabled count

id before after diff
securitySolution 525 527 +2

History

  • 💚 Build #149203 succeeded 632ee06dfbbd475dc1e0f4c77a2594e3d3a38155
  • 💔 Build #148769 failed 9ed0d08af1221e1efe96a8dde6b26e6151efb7d4
  • 💔 Build #147997 failed 0d5c4d88220075abea7aabb60f14e1e628caadba
  • 💔 Build #147959 failed a31196c1599adf3c07f6c2e964a736b47210df14
  • 💔 Build #147595 failed 0122bd98d75f8b2d9d8b13263efa5b18488b14bf

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

cc @xcrzx

Copy link
Contributor

@marshallmain marshallmain left a comment

Choose a reason for hiding this comment

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

This is fantastic! I'm approving it now so we can more easily get this in front of other teams who may want to start experimenting with the code generation. I've listed some some important enhancements below that we should also aim to make in the near future in (IMO) priority order.

  1. Automatically create index.ts files and add generated schema files to those index.ts files. A first implementation of this could simply be a top-level index.ts file that exports directly from each individual generated schema file. The index file is important to isolate the consumers of API schemas from the internal /api folder structure in case we decide the domain structure should be changed.

A downside of a mega-index file like this that I encountered while moving schemas into /common/api is that the public code does not tree-shake, so public code that imports from an index.ts file that contains many unnecessary dependencies can vastly increase the bundle size. (this commit was added just to remove @kbn/config-schema as a dependency from the timeline schemas so that public code could import from /common/api/timeline without importing @kbn/config-schema and adding 840KB to the chunk size).

If we only import type from /common/api into public then perhaps a mega-index wouldn't be an issue. Alternatively, an index.ts file per top-level domain could be nice and avoid name clashes between domains.

  1. It would be great to move the generation script to a package so we can easily reuse it in the lists plugin immediately and other plugins in the future.
  2. Run the code generation script and apply any changes in CI so the generated code is guaranteed to stay in sync with the yaml files. The linter auto-committing changes in CI is a good model to follow here. In theory we can add a line here to call a script like this ts_projects script which could then call the generate script you built and commit any files that were changed after running the generate script.

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Thank you for the overview with the team and for taking the time to answer questions today @xcrzx. LGTM! 👍 🚀 So awesome that we're finally here with codegen!! 🙌

@xcrzx xcrzx merged commit bc37dc2 into elastic:main Aug 14, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 14, 2023
@xcrzx xcrzx deleted the openapi-codegen branch August 15, 2023 08:18
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: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.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants