daveshanley / vacuum

vacuum is the worlds fastest OpenAPI 3, OpenAPI 2 / Swagger linter and quality analysis tool. Built in go, it tears through API specs faster than you can think. vacuum is compatible with Spectral rulesets and generates compatible reports.
https://quobix.com/vacuum
MIT License
634 stars 51 forks source link

Linting with `--ignore-file` doesn't have a deterministic output #526

Open DimitrisKaramanis opened 4 months ago

DimitrisKaramanis commented 4 months ago

I am in version 0.11.1 and I am trying to use vacuum lint with the --ignore-file argument.

When I lint the yaml file with the ignoring, sometimes I get 5 errors (5 valid ones and 1 ignored), but some other times I get 6 errors (none of them gets ignored). I am executing it one time after the other without changing anything, so there is no difference between the two executions. Also, I tried to do the same things with a smaller OpenAPI file (13 lines), but then it was working perfectly.

And I couldn't find any 'sequence` of how often I get the right and the wrong result. It seems random to me.

Below you can find both my demo OpenAPI file and the ignore file that I use.

OpenAPI file:

openapi: 3.0.1
info:
  contact:
    email: contact@example.com
  description: API definitions
  license:
    name: Apache 2.0
    url: http://www.apache.org/licenses/LICENSE-2.0.html
  termsOfService: https://example.com/terms/
  title: Example API
  version: 1.0.0
security:
  - basicAuth: []
  - bearerAuth: []
paths:
  /health:
    get:
      operationId: checkReadiness
      responses:
        default:
          content:
            application/json;charset=UTF-8:
              schema:
                $ref: "#/components/schemas/ResponseResult"
          description: default response
      summary: Returns the readiness health of the service
  /health/liveness:
    get:
      operationId: checkLiveness
      responses:
        default:
          content:
            application/json;charset=UTF-8:
              schema:
                $ref: "#/components/schemas/ResponseResult"
          description: default response
      summary: Returns the liveness health of the service
  /v1/repo:
    post:
      description: Creates a new repo 
      operationId: createRepo
      requestBody:
        content:
          application/json;charset=UTF-8:
            schema:
              $ref: "#/components/schemas/CreateRepository"
        description: object representation
        required: true
      responses:
        default:
          content:
            application/json;charset=UTF-8:
              schema:
                $ref: "#/components/schemas/ResponseCreateRepo"
          description: default response
      summary: Create a new repo
      tags:
        - Example
components:
  schemas:
    CreateRepository:
      type: object
      properties:
        team:
          type: string
          maxLength: 255
          minLength: 1
        description:
          type: string
          maxLength: 350
          minLength: 10
        name:
          type: string
          maxLength: 100
          minLength: -1
          pattern: "*"
      required:
        - team
        - description
        - name
    CreateRepoResponse:
      type: object
      properties:
        team:
          type: string
        name:
          type: string
        url:
          type: string
      required:
        - team
        - name
        - url
    ResponseCreateRepo:
      type: object
      properties:
        data:
          $ref: "#/components/schemas/CreateRepoResponse"
      required:
        - data
    ResponseResult:
      type: object
      properties:
        data:
          $ref: "#/components/schemas/Result"
      required:
        - data
    Result:
      type: object
      properties:
        details:
          type: object
          additionalProperties:
            type: object
        error:
          type: object
          properties:
            localizedMessage:
              type: string
            message:
              type: string
            stackTrace:
              type: array
              items:
                type: object
                properties:
                  classLoaderName:
                    type: string
                  className:
                    type: string
                  fileName:
                    type: string
                  isNativeMethod:
                    type: boolean
                  lineNumber:
                    type: integer
                    format: int32
                  methodName:
                    type: string
                  moduleName:
                    type: string
                  moduleVersion:
                    type: string
        healthy:
          type: boolean
        message:
          type: string
        time:
          type: string
          format: date-time
      required:
        - healthy
        - time
  securitySchemes:
    basicAuth:
      scheme: basic
      type: http
    bearerAuth:
      bearerFormat: JWT
      scheme: bearer
      type: http

Ignore file:

oas-schema-check:
  - $.components.schemas['CreateRepository'].properties['name'].minLength
daveshanley commented 4 months ago

@lobocv would you like to take a stab at this, as it's your feature?

lobocv commented 4 months ago

Running it with some debug prints I see that the path on the error is sometimes different than the one you are ignoring:

On the run with 6 errors (ignoring is failing) I get the following path

$.paths['/v1/repo'].post.requestBody.content['application/json;charset=UTF-8'].schema.properties['name'].minLength

When it does work (5 errors, 1 ignored) I see the path is:

$.components.schemas['CreateRepository'].properties['name'].minLength

Seems like a bug in the code that generates the path? Looking at the openapi.yaml, it looks like the first one is the real path.

@daveshanley do you know what could be going wrong here?

EDIT: Ah I see. It looks like in one case, the $ref is being resolved and in the other case it's not. Looks like the linting behaviour is not consistent in it's output.

lobocv commented 3 months ago

@DimitrisKaramanis can you recreate this issue on the latest release of vacuum? @daveshanley has made some fixes to how paths are generated in the linting output and it may have resolved this issue. If so, feel free to close the ticket. Thanks!

lobocv commented 2 months ago

I was able to recreate this issue on v0.13.1 đŸ˜¢

This is my rule which checks that all paths start with a version

url-starts-with-major-version: description: Major version must be the first URL component message: All paths must start with a version number, eg /v1, /v2 given: $.paths[*]~ severity: error type: style then: function: pattern functionOptions: match: "$/v[0-9]+/"

About 1/4 times I run the lint I get a failure. When it fails, it matches on some object in the path rather than the path key: For example:

$.paths['/memberAppComponents'].post.responses is what vacuum says the path is that is failing

but it should be

$.paths['/memberAppComponents']

And it is, most of the time (and then the tests pass because I have that ignored)

lobocv commented 2 months ago

I was debugging this and it looks to be coming from this line.

It assumes the thing you are matching is an object. For my rule, I am matching against the path keys with $.paths[*]~

Edit: Maybe it's fine, the node that is matched has a Value of the path key but for some reason GenerateJSONPath() is sometimes generating the path to a field inside the path object. Other time it returns the path. I'm having a hard time figuring it out. Seems like it depends on some other state in the Foundation{}

Not sure what the solution would be here @daveshanley but i'm happy to contribute to a fix.

lobocv commented 2 months ago

Okay I think I've nailed this down the the source. It seems to be an issue in the doctor building a mapping of line number to node objects here. There is a race condition that happens here when using $ref. The line numbers of the processed nodes are relative to the yaml file the object is defined, not relative to the overall document. In my case, I have each path in a separate yaml file. What happens is that all the objects from a particular path (path items, reponses, operations etc) get assigned to the same line number. Only the first node gets used.

Since this happens asynchronously, there is a race condition where the first node is the right one and the path reported by vacuum is correct. But in other cases, one of the nodes from deeper within the $ref is used and is assigned the line number of the $ref.

I hope I am explaining this correctly. Unfortunately I don't know the solution. The lineObjects needs to account for the line numbers injected by the $ref.

I will see if I can recreate this as a test in the doctor