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] Fix general Security Solution API reference documentation issues #190035

Closed
3 of 4 tasks
maximpn opened this issue Aug 7, 2024 · 7 comments
Closed
3 of 4 tasks
Assignees
Labels
8.16 candidate docs Feature:OAS Work or issues related to Core-provided mechanisms for generating OAS Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0

Comments

@maximpn
Copy link
Contributor

maximpn commented Aug 7, 2024

Epic: https://github.com/elastic/security-team/issues/9400 (internal)
Relates to: https://github.com/elastic/security-team/issues/9398
Relates to: https://github.com/elastic/security-team/issues/9526
Relates to: https://github.com/elastic/security-team/issues/9530
Relates to: https://github.com/elastic/security-team/issues/9531
Relates to: https://github.com/elastic/security-team/issues/9524
Relates to: https://github.com/elastic/security-team/issues/9528
Relates to: https://github.com/elastic/security-team/issues/9525
Relates to: https://github.com/elastic/security-team/issues/9527

Summary

This ticket collects general API reference documentation issues spotted after deployment bundled and merged Security Solution OpenAPI specs to Bump.sh (new API reference documentation platform). It should focus on issues related to OpenAPI bundler kbn-openapi-bundler and/or source OpenAPI specs problems causing exposing wrong API endpoints, missing API endpoints, having wrong operationId and etc. In the other words critical issues violating documentation correctness.

It shouldn't include cosmetic documentation readiness issues (like missing summary, description, etc.) since it's covered in team specific tickets

Spotted issues

  • Conflicting operationIds in Detections and AI Assistant APIs (operationIds must be unique throughout whole Kibana) (fix)

image

  • Response schema descriptions are duplicated in the bump.sh output (comment) (fix comment)

  • Can we provide Elastic-Api-Version header for curl examples?

  • Bump.sh doesn't render objects with additionalProperties: SomeSchema correctly (schemas declaring an object accepting arbitrary keys where values are restricted to some type which could be represented as a record Record<string, SomeType>).

    requestBody.queries in /api/osquery/packs has ObjectQueries type which accepts any string key whose values is restricted to ObjectQueriesItem

image
@maximpn maximpn added docs Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:OAS Work or issues related to Core-provided mechanisms for generating OAS 8.16 candidate labels Aug 7, 2024
@maximpn maximpn self-assigned this Aug 7, 2024
@elasticmachine
Copy link
Contributor

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

@maximpn maximpn changed the title [Security Solution] Fix Security Solution API reference documentation issues [Security Solution] Fix generic Security Solution API reference documentation issues Aug 7, 2024
@maximpn maximpn changed the title [Security Solution] Fix generic Security Solution API reference documentation issues [Security Solution] Fix general Security Solution API reference documentation issues Aug 7, 2024
maximpn added a commit that referenced this issue Aug 12, 2024
**Addresses:** #190035

## Summary

This PR fixes `operationId` conflicts in Security Solution OpenAPI specs.

## Details

API reference documentation platform (Bump.sh) expects unique `operationId`s to build proper navigation on the documentation page. It's expected each `operationId` throughout whole Kibana since Kibana API reference documentation will contain all available API endpoints.
bryce-b pushed a commit to bryce-b/kibana that referenced this issue Aug 13, 2024
…ic#190040)

**Addresses:** elastic#190035

## Summary

This PR fixes `operationId` conflicts in Security Solution OpenAPI specs.

## Details

API reference documentation platform (Bump.sh) expects unique `operationId`s to build proper navigation on the documentation page. It's expected each `operationId` throughout whole Kibana since Kibana API reference documentation will contain all available API endpoints.
@lcawl
Copy link
Contributor

lcawl commented Aug 14, 2024

With respect to the rendering of additionalProperties, I can recreate that with a simple example:

openapi: 3.0.3
info:
  title: Test
  version: '0.2'
servers:
  - url: /
paths:
   /api/status:
      get:
        operationId: /api/status#0
        responses:
          '200':
            description: 'Success'
            content:
              application/json:
                schema:
                  type: object
                  additionalProperties:
                    type: object
                    properties:
                      code:
                        type: integer
                      text:
                        type: string

I will follow up with the support folks.

@natasha-moore-elastic
Copy link
Contributor

A couple of issues spotted while reviewing:

  1. Response schema descriptions are duplicated in the bump.sh output. Examples:

image

  1. When a property has both a description and a $ref, the property's description is ignored in the output, and the referenced schema’s description is shown instead. For example, in the Detections API we have the following spec:
      properties:
        ecs:
          description: Whether the field is an ECS field
          type: boolean
        name:
          $ref: '#/components/schemas/NonEmptyString'
          description: Name of an Elasticsearch field
        type:
          $ref: '#/components/schemas/NonEmptyString'
          description: Type of the Elasticsearch field

In the output, the name and type properties both show the NonEmptyString description ("A string that is not empty and does not contain only whitespace") but their respective descriptions ("Name of an Elasticsearch field" and "Type of the Elasticsearch field") aren't shown:

image

@maximpn
Copy link
Contributor Author

maximpn commented Aug 27, 2024

@lcawl I doubled check @natasha-moore-elastic's findings and it looks like Response schema descriptions are duplicated in the bump.sh output is a Bump.sh bug. I re-uploaded Kibana staging OpenAPI merged specs and got the same result for POST /api/detection_engine/signals/tags and POST /api/detection_engine/rules/_export. Response description is duplicated.

@maximpn
Copy link
Contributor Author

maximpn commented Aug 27, 2024

When a property has both a description and a $ref, the property's description is ignored in the output, and the referenced schema’s description is shown instead.

It's an expected behavior. And basically OpenAPI 3.1 behavior despite we use OpenAPI 3.0 right now. The OpenAPI 3.1 specification states that $ref property expands to the current object replacing existing fields in case of conflicts. Bump.sh follows the specification. Strictly speaking OpenAPI 3.0 doesn't allow any properties next to $ref but we intentionally violate that requirement in some cases related to default values and custom descriptions. Though fixing this problem in specs could be quite tricky.

delanni pushed a commit that referenced this issue Sep 9, 2024
…192348)

**Relates to:** #190035

## Summary

This PR sets proper teams ownership to Security Solution OpenAPI domain
bundles. It will help to avoid unnecessary code review dependencies and
allow teams to focus on OpenAPI domain changes.
@lcawl
Copy link
Contributor

lcawl commented Sep 26, 2024

@lcawl I doubled check @natasha-moore-elastic's findings and it looks like Response schema descriptions are duplicated in the bump.sh output is a Bump.sh bug. I re-uploaded Kibana staging OpenAPI merged specs and got the same result for POST /api/detection_engine/signals/tags and POST /api/detection_engine/rules/_export. Response description is duplicated.

This issue with the duplicated info has now been fixed in Bump.sh and I re-tested and could no longer recreate the problem. Thanks for reporting it!

@banderror
Copy link
Contributor

@maximpn Since the docs have been published, I think we can safely assume that all the main issues have been addressed. A couple of unchecked issues remain in the description - if you think this is something worth working on later, please extract them to their own separate tickets. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.16 candidate docs Feature:OAS Work or issues related to Core-provided mechanisms for generating OAS Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0
Projects
None yet
Development

No branches or pull requests

5 participants