Tufin / oasdiff

OpenAPI Diff and Breaking Changes
https://www.oasdiff.com
Apache License 2.0
689 stars 59 forks source link

basePath and servers.ulr support #512

Closed codeFather2 closed 5 months ago

codeFather2 commented 5 months ago

Is your feature request related to a problem? Please describe. I tried comparing the API specification version 2, which has a basePath field defined, but their comparison does not take this field into account. I also tried to compare the version 3.* specification, but the servers.url field is also not taken into account in the comparison of 2 specifications, for example, if it is defined in one file, and in the other the base path is defined specifically for each path as a prefix e.g. v2 with basePath

basePath: /v1
paths:
  /entry/check:
    ...
  /entry/enter:
    ...

v2 without basePath

paths:
   /v1/entry/check:
    ...
   /v1/entry/enter:
    ...

v3 with servers.url

servers:
    -url: /v1
paths:
  /entry/check:
    ...
  /entry/enter:
    ...

v3 without servers.url

paths:
  /v1/entry/check:
    ...
  /v1/entry/enter:
    ...

Important in swagger specs from version 3, you can provide servers.url by variables with default values

servers:
  - description: Production
    url: {basePath}
    variables:
      basePath:
        default: /v1

Describe the solution you'd like If it's possible can we add support for both versions of spec and comparison between version 2 with basePath and version 3.* with servers.url fields. The way i see it is to use this prefix field from base and try remove it from revision spec if possible or apply it to base spec (it feels more difficult to do and can lead to undesirable effects). If there are base paths in both places, compare them with each other

Describe alternatives you've considered In progress

codeFather2 commented 5 months ago

It is also necessary to take into account that the servers.url may contain the host

servers:
  - description: Production
    url: https://api.test.com{basePath}
    variables:
      basePath:
        default: /v1
reuvenharrison commented 5 months ago

Thanks for the initiative @codeFather2. I think it makes sense for oasdiff to support this use case.

reuvenharrison commented 5 months ago

After taking a closer look at the spec, I am not sure there is much we can do here:

I think the current behavior of oasdiff is adequate:

  1. It assumes that paths exclude the server prefix and can therefore be compared to each-other across base and revision
  2. If the paths have been modified, for example, because a developer decided to remove a common prefix, then oasdiff can overcome this with the prefix flags: --prefix-base, --prefix-revision, --strip-prefix-base or --strip-prefix-revision

Optionally, you could develop an external tool to check if there is a common prefix that was modified and pass it to oasdiff.