elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.71k stars 8.13k forks source link

[Security Solution] Fix general Security Solution API reference documentation issues #190035

Open maximpn opened 1 month ago

maximpn commented 1 month ago

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

image

elasticmachine commented 1 month ago

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

lcawl commented 1 month ago

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 commented 3 weeks ago

A couple of issues spotted while reviewing:

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

    image

  2. 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 commented 2 weeks ago

@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 commented 2 weeks ago

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.