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

Problem in paths reported for rule violations involving schema references within oneOf #2059

Closed
padamstx opened this issue Feb 11, 2022 · 7 comments · Fixed by #2202
Closed
Labels

Comments

@padamstx
Copy link
Contributor

padamstx commented Feb 11, 2022

Describe the bug
The spectral-core code related to mapping "resolved" paths to "unresolved" paths appears to get tripped up when dealing with schema references within a oneOf list. The testcase mentioned below contains a fairly simple OpenAPI document (api.yaml) along with a custom rule which verifies that each schema contains a valid description.
The custom rule's function logs the individual paths that are visited, along with the individual violations that
are found.
The API contains a handful of re-usable (named) schemas, with one (Soda) that lacks a description.
This Soda schema is referenced from within the Drink schema's oneOf list and is reachable in three
different ways:

  1. the post requestBody
  2. the post response
  3. the get response

To Reproduce
A testcase that demonstrates this problem can be found here:
https://github.com/padamstx/spectral-dedup-issue
The README.md contains detailed instructions to reproduce, but a summary is:

  • clone the repo mentioned above
  • spectral lint api.yaml

Expected behavior
Based on the way in which the api.yaml file is defined, I'd expect a single error to be displayed
by spectral with the path components.schemas.Soda

Screenshots
sample debug output from the custom function:

$ spectral lint api.yaml
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1
>> Violation at: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1
>> Violation at: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1
>> Violation at: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.name

/home/padams/work/test/spectral-dedup-issue/api.yaml
 62:11  error  schema-description  Schema must have a non-empty description.  components.schemas.Drink.oneOf[1]
 75:17  error  schema-description  Schema must have a non-empty description.  components.schemas.Drinks.properties.drinks.items

✖ 2 problems (2 errors, 0 warnings, 0 infos, 0 hints)

Environment:
This problem was found in spectral v6.2.1 running on Linux (Fedora 34)

Additional context
This problem was originally discovered during development of custom rules as part of the IBM openapi-validator project (https://github.com/IBM/openapi-validator), which is tightly integrated with spectral.

@dpopp07
Copy link
Contributor

dpopp07 commented Feb 11, 2022

Offering a +1 to this issue, as it is quite a significant deviation from expected behavior. I think the root of the issue is the buggy path resolution logic - if it were reporting the correctly "decoded" paths, the de-duplication logic would filter out the extra result.

As such, it may be related (or at least, is very similar) to #2043.

@padamstx
Copy link
Contributor Author

padamstx commented Feb 12, 2022

It turns out that this issue also occurs if the schema ref is in an array schema's "items" field.

To reproduce this, make the following changes to api.yaml within the testcase mentioned above:

  1. uncomment the Soda schema's description
  2. comment out the Drink schema's description
  3. spectral lint api.yaml
    results:
$ spectral lint api.yaml
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema
>> Violation at: paths./v1/drinks.post.requestBody.content.application/json.schema
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema
>> Violation at: paths./v1/drinks.post.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items
>> Violation at: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.name

/home/padams/work/test/spectral-dedup-issue/api.yaml
 58:11  error  schema-description  Schema must have a non-empty description.  components.schemas.Drink
 75:17  error  schema-description  Schema must have a non-empty description.  components.schemas.Drinks.properties.drinks.items

Note that the items field within the "Drinks" schema is a ref to the "Drink" schema.
All three of the individual violations found by the rule should be converged into a single path components.schemas.Drink.

@padamstx padamstx changed the title Problem in de-dup logic involving schema references within oneOf Problem in paths reported for rule violations involving schema references within oneOf Feb 14, 2022
@padamstx
Copy link
Contributor Author

After debugging the spectral code a bit for this issue, I think the problem occurs inside the DocumentInventory.findAssociatedItemforPath() method.

@padamstx
Copy link
Contributor Author

@P0lip any chance that someone with detailed knowledge of spectral internals could take a look at this issue? This issue specifically mentions that "oneOf" could trip up the path mapping function, but we've found that this problem can also be triggered by simple nested schemas as well (i.e. a schema property is defined through a $ref, etc.).

@padamstx
Copy link
Contributor Author

padamstx commented Jul 7, 2022

I've opened PR #2202 to fix this issue.

With the changes in that PR, the output is now:

$ node ~/work/tools/spectral/packages/cli/dist/index.js lint api.yaml
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1
>> Violation at: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1
>> Violation at: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.type
Visiting path: paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.properties.name
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.0.properties.fruit
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1
>> Violation at: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.type
Visiting path: paths./v1/drinks.get.responses.201.content.application/json.schema.properties.drinks.items.oneOf.1.properties.name

/home/padams/work/test/spectral-dedup-issue-original/api.yaml
 77:10  error  schema-description  Schema must have a non-empty description.  components.schemas.Soda

✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints)

@stoplight-bot
Copy link
Collaborator

🎉 This issue has been resolved in version @stoplight/spectral-cli-v6.4.2 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

@stoplight-bot
Copy link
Collaborator

🎉 This issue has been resolved in version @stoplight/spectral-core-v1.12.4 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants