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] A proper location for OpenAPI specs #184859

Closed
maximpn opened this issue Jun 5, 2024 · 6 comments
Closed

[Security Solution] A proper location for OpenAPI specs #184859

maximpn opened this issue Jun 5, 2024 · 6 comments
Assignees
Labels
8.15 candidate discuss docs refactoring Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0

Comments

@maximpn
Copy link
Contributor

maximpn commented Jun 5, 2024

Epic: https://github.com/elastic/security-team/issues/9401 (internal)

Summary

While working on adding missing OpenAPI spec (OAS) for Detections API it became prominent that the current OAS placement in security_solution/common/api brings the following challenges

  • Bad DX when using OAS references e.g. ../../../../../../../lists/common/api/exceptions/exception.schema.yaml

    Some API endpoints (e.g. POST /api/detection_engine/rules/{id}/exceptions) have external dependencies. It requires referencing shared OAS schemas via $ref from the other plugins (e.g. lists). Having such long relative paths is brittle and inconvenient. Moving OAS files to another folder will be painful. There is no aliasing solution out of the box.

  • A necessity to import generated Zod schemas and inferred types from the other plugins.

Currently we have io-ts schemas defined in plugins e.g. kbn-securitysolution-io-ts-list-types.

  • Primitives (NonEmptyString or UUID) location. What's a proper location for them?

There some shared OAS schemas considered like primitives like NonEmptyString and UUID defined in Security Solution plugin. Obviously such primitives need to be reused in the other plugins.

@maximpn maximpn added discuss impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. docs Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Project:Serverless Work as part of the Serverless project for its initial release labels Jun 5, 2024
@elasticmachine
Copy link
Contributor

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

@maximpn
Copy link
Contributor Author

maximpn commented Jun 5, 2024

One of the ideas suggested by @banderror is to use Security Solution domain packages like kbn-security-solution-detections-api and kbn-security-solution-exceptions-api. It solves the above problems in the following way

  • Packages reside one to the other which simplifies OAS references usage while not completely eliminates the problem
  • Generated artefacts can be easily imported by any package or plugin
  • It follows a similar approach we have with io-ts schemas
  • An OAS common or primitives package can have shared primitives like NonEmptyString or UUID

The only question is to decide where should resulting OAS bundles be placed

  • in each domain package
  • in meta package like kbn-security-solution-oas-bundles
  • in a plugin, e.g. security_solution

@banderror banderror added refactoring 8.15 candidate and removed impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Project:Serverless Work as part of the Serverless project for its initial release labels Jun 5, 2024
@maximpn
Copy link
Contributor Author

maximpn commented Jun 7, 2024

Zoomed with @marshallmain and @banderror regarding this topic. Discussed possible ways to solve bad DX related to relative schema imports like ../../../../../../../lists/common/api/exceptions/exception.schema.yaml and reusing of shared primitives. Based on the discussion we worked out the following actions items

  • Investigate usage of Base URI

    Moving OAS files into packages only mitigates bad DX and doesn't solve it completely. We need something like aliasing where reference can start from an alias like @lists/ or @shared/ but it's not supported by OAS tooling. Since OAS is based on JSON Schema it's worth to investigate Base URI usage. Ideally we could set the base path like <Kibana root folder>/x-pack/plugins/ or <Kibana root folder>/packages and references like /lists/path/to/my_schema.schema.yaml. But we still should have an option to use relative path when it's shorter. It doesn't solve the problem completely but significantly improves DX.

  • Investigate OAS re-exporting

    There is an idea to export only a limited set of shared OAS schemas to be reused in the other plugins or packages. The idea is based on adding a mediator OAS file somewhere like lists/common/api (at the top level folder as possible) to reference it from another plugin or package.

  • Investigate placing OAS primitives like NonEmptyString into a package

    OAS primitives like NonEmptyString will be required almost everywhere so having them in a specially dedicated packages looks reasonable. We need to understand easiness to consume that primitives from a package.

@maximpn
Copy link
Contributor Author

maximpn commented Jun 10, 2024

Hi @marshallmain and @banderror,

I've investigated usage of Base URI, OAS shared schemas re-exporting and using a package for schema primitives.

TLDR: There is a no working solution out of the box. We need noticeable amount of effort to make it working.

Details

Investigate usage of Base URI

I analyzed it from two different sides OAS and JSON schema specifications and tooling (kbn-openapi-generator uses @apidevtools/swagger-parser package).

  • OAS and JSON schema specifications

    Checking OAS 3.0 specification page there is no mention of base URI. There is an additional page with usage examples limited to relative path or urls. Where the first page's note says

    Relative references used in $ref are processed as per JSON Reference, using the URL of the current document as the base URI. See also the Reference Object.

Analyzing JSON schema specification it becomes clear that references are considered as links. Links could be relative or absolute. Relative links resolved by using baseURI which is a URI the specification was retrieved from. Or baseURI can be explicitly specified via $id property. However $id property (or any alternative) isn't mentioned anywhere in OAS 3.0 specification.

  • Tooling

    kbn-openapi-generator uses @apidevtools/swagger-parser package. It allows to parse OAS files and get it's representation in memory. I analyzed @apidevtools/swagger-parser source code and looked at its dependency @apidevtools/json-schema-ref-parser as well. There are no signs of baseURI functionality we could use to improve DX. It doesn't provide a way to specify a base path/base url to resolve references.

    Taking the above into account there is no way to use Base URI in OAS.

Investigate OAS re-exporting

I approached this part by creating a shared list schema in List plugin at x-pack/plugins/lists/common/api/values/model/list_common.schema.yaml and adding a re-export spec at x-pack/plugins/lists/common/api/export.schema.yaml looking like

openapi: 3.0.0
info:
  title: Error Schema
  version: 'not applicable'
paths: {}
components:
  x-codegen-enabled: false
  schemas:
    List:
      $ref: './values/model/list_common.schema.yaml#/components/schemas/List'

then I referenced re-exported List schema in Security Solution plugin at x-pack/plugins/security_solution/common/api/model/test_re_export.gen.ts like

openapi: 3.0.0
info:
  title: Test shared schemas re-export
  version: 'not applicable'
paths: {}
components:
  x-codegen-enabled: true
  schemas:
    TestSchema:
      type: object
      properties:
        localId:
          $ref: './primitives.schema.yaml#/components/schemas/NonEmptyString'
        listData:
          $ref: '../../../../lists/common/api/export.schema.yaml#/components/schemas/List'

The problem with that approach is that kbn-openapi-generator doesn't unfold references (a folded reference means a chain of referenced like above '../../../../lists/common/api/export.schema.yaml#/components/schemas/List' -> './values/model/list_common.schema.yaml#/components/schemas/List'). Its implementation is pretty straightforward. It simply read an OAS file and generates artefacts add imports for external references. The generated code relies on that List Zod schema exists next to export.schema.yaml in the List plugin. But it's a proxy schema we don't want to generate an artefact for.

Investigate placing OAS primitives like NonEmptyString into a package

It's not a problem to place shared schema primitives in a package. The only problem here is caused by two bullets above. Reference paths are gonna be long and looking like ../../../../../../../packages/kbn-oas-common/primitives.schema.yaml.

@banderror
Copy link
Contributor

@maximpn Thank you for doing this research and describing the results. My suggestion is:

  • Let's not invest any more time in any tooling related to refs. We have a very big scope of work that we can't avoid in the bundler and other epics, and we have limited time till the first deadline due to time off in the summer.
  • I think we can tolerate - at least for some time - long reference paths that would look like ../../../../../../../packages/kbn-oas-common/primitives.schema.yaml. Probably it's not the best DX, but it's also not a high-impact issue.
  • Let's create a new package for storing primitives. It could be a kbn-openapi-common or something else.
  • Let's start adding Lists API and Exceptions API schemas to their own dedicated packages from the beginning. Naming is arguable, but could be something like:
    • kbn-securitysolution-lists-api and kbn-securitysolution-exceptions-api
    • kbn-securitysolution-lists-common/api and kbn-securitysolution-exceptions-common/api (preferred from my side)

@maximpn
Copy link
Contributor Author

maximpn commented Jun 18, 2024

Based on the research we decided to proceed with packages for OpenAPI specs as describe in the previous comment #184859 (comment).

Actual code change will be addressed as a part of separate tickets

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.15 candidate discuss docs refactoring Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0
Projects
None yet
Development

No branches or pull requests

4 participants