redocly-cli icon indicating copy to clipboard operation
redocly-cli copied to clipboard

Bundler Does Not Resolve Discriminator Mapping Values

Open its-hammer-time opened this issue 1 year ago • 12 comments

Describe the bug

When using the --dereferenced argument with the Redocly CLI bundler, references found under the discriminator mapping section are not resolved and therefore may be broken. For example, given the schema below, the redocly bundler would appropriately resolve the references under allOf and add them to the bundled result, but the mapping references would remain as-is. Depending on the values of the mapping references, those references may no longer exist (i.e. if the bundled file is in a different location).

discriminator:
  propertyName: type
  # These are not dereferenced
  mapping:
    VAL_1: ../values/val1.yaml
    VAL_2: ../values/val2.yaml
# These are dereferenced
allOf:
  - $ref: ../values/val1.yaml
  - $ref: ../values/val2.yaml

To Reproduce

Commands I ran:

redocly --version
1.16.0

redocly bundle --dereferenced openapi.yaml --output bundled.yaml

I've provided an example repo which you can use to reproduce this, but it's really easy to setup a fresh example as well. You need something like the following structure:

/
  openapi.yaml
  example/
    my-example.yaml
  values/
    val1.yaml
    val2.yaml

Let's say I bundle the setup above and I place my bundled folder at the root (next to openapi.yaml). If the my-example.yaml has references to ../values/val1.yaml then the resulting bundled file will still have ../values/val1.yaml which is no longer a valid location since it should have been moved to ./values/val1.yaml.

example.zip

I imagine the example your company has on their docs would also run into this issue if you bundle with a dereferenced spec. In the example with $refs you can see they have ../components/..... which may no longer be valid locations since the bundled spec can be output anywhere.

Expected behavior

According to the official OpenAPI 3.1.0 documentation, the mapping section is "An object to hold mappings between payload values and schema names or references". If I'm using the Redocly CLI to dereference while bundling, then the CLI should attempt to resolve these values much like it does when it sees the $ref keyword.

This isn't from your code, but we use the Python openapi-core package which internally uses openapi-schema-validator. For their implementation of discriminators, they do attempt to resolve the file references which is how we noticed this behavior.

Logs

N/A

OpenAPI description

Here's the openapi.yaml file I used in my example.zip folder above.

openapi: 3.1.0
servers:
  - url: http://localhost:8080
info:
  title: Example Service
  version: 1.0.0
paths:
  /test:
    get:
      responses:
        default:
          content:
            application/json:
              schema:
                type: object
                properties:
                  test:
                    type: string
                    example: "test"
components:
  schemas:
    Example:
      $ref: example/my-example.yaml

Redocly Version(s)

1.16.0

Node.js Version(s)

v18.17.1

Additional context

N/A

its-hammer-time avatar Jun 28 '24 17:06 its-hammer-time

this was recently clarified in the upcoming 3.0.4 patch release https://github.com/handrews/OpenAPI-Specification/blob/4a46b20c451de02e5dee9974fbcbb9be24098d10/versions/3.0.4.md#options-for-mapping-values-to-schemas

related: https://github.com/OAI/OpenAPI-Specification/issues/2520

jeremyfiel avatar Jul 02 '24 05:07 jeremyfiel

Okay, so to clarify, here's the three pieces I got from those links:

Implicit Mapping: If a mapping section is not provided, the value of each key will be used to map to a schema found under the Component section of the spec.

Explicit Mapping - Value: If the mapping section is provided and the value DOES NOT being with ./ then it will be inferred as a name for a schema found under the Component section of the spec

Explicit Mapping - Reference: If the mapping section is provided and the value DOES being with ./ then it will be inferred as a reference that needs to be resolved


So assuming the 3 statements above are correct, it sounds like we just need to prepend ./ to all of our file references?

its-hammer-time avatar Jul 02 '24 16:07 its-hammer-time

I just spoke to one of my teammates, and the spec that's live at the moment (which doesn't work) has the ./ prepended like

discriminator:
  propertyName: type
  mapping:
    SCHEMA_A: ./schema_a.yaml
    SCHEMA_B: ./schema_b.yaml
    SCHEMA_C: ./schema_c.yaml
oneOf:
  - $ref: ./schema_a.yaml
  - $ref: ./schema_b.yaml
  - $ref: ./schema_c.yaml

When we bundle using the Redocly CLI, the $refs are resolved in the oneOf section, but the mapping section remains as is and the result is those are now no longer valid file paths.

its-hammer-time avatar Jul 02 '24 16:07 its-hammer-time

Ahh wait, I think I'm understanding now. The definition for this mapping section is for a "relative reference" not a JSON reference which is what the $ref is.

https://github.com/handrews/OpenAPI-Specification/blob/4a46b20c451de02e5dee9974fbcbb9be24098d10/versions/3.0.4.md#relative-references-in-urls

Unless specified otherwise, all properties that are URLs MAY be relative references as defined by RFC3986. Relative references are resolved using the URLs defined in the Server Object as a Base URI.

Relative references used in $ref are processed as per JSON Reference, using the URL of the current document as the base URI. See also the Reference Object.

So it sounds like we can't actually use file references here since it's not using the "JSON Reference" and instead must be relative to the Server URI?

its-hammer-time avatar Jul 02 '24 16:07 its-hammer-time

I believe your understanding to be correct regarding relative refs and the server object as the base uri. No json references are expected to resolve in this location

You can also ask in the OAS slack https://join.slack.com/t/open-api/shared_invite/zt-2lzkwb5n7-nHiDdJ3eVUqXCadhzUPJlg

On Tue, Jul 2, 2024, 09:59 Zachary Hamm @.***> wrote:

Ahh wait, I think I'm understanding now. The definition for this mapping section is for a "relative reference" not a JSON reference which is what the $ref is.

(which doesn't work)

https://github.com/handrews/OpenAPI-Specification/blob/4a46b20c451de02e5dee9974fbcbb9be24098d10/versions/3.0.4.md#relative-references-in-urls

Unless specified otherwise, all properties that are URLs MAY be relative references as defined by RFC3986 https://tools.ietf.org/html/rfc3986#section-4.2. Relative references are resolved using the URLs defined in the Server Object https://github.com/handrews/OpenAPI-Specification/blob/4a46b20c451de02e5dee9974fbcbb9be24098d10/versions/3.0.4.md#serverObject as a Base URI.

Relative references used in $ref are processed as per JSON Reference https://tools.ietf.org/html/draft-pbryan-zyp-json-ref-03, using the URL of the current document as the base URI. See also the Reference Object https://github.com/handrews/OpenAPI-Specification/blob/4a46b20c451de02e5dee9974fbcbb9be24098d10/versions/3.0.4.md#referenceObject .

So it sounds like we can't actually use file references here since it's not using the "JSON Reference" and instead must be relative to the Server URI?

— Reply to this email directly, view it on GitHub https://github.com/Redocly/redocly-cli/issues/1602#issuecomment-2203847437, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHU7MTJJH34HD7GVGPUF5WLZKLL77AVCNFSM6AAAAABKCHTNICVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBTHA2DONBTG4 . You are receiving this because you commented.Message ID: @.***>

jeremyfiel avatar Jul 02 '24 17:07 jeremyfiel

So if that understanding is correct, that essentially means we can't use discriminators with the redocly bundler if we have our schema split across multiple files? We tried the following cases and all of them were invalid. Note that SCHEMA_A, SCHEMA_B, and SCHEMA_C are all under the components schemas section.

Schema Name Approach: Issue: Invalid $ref from Redocly bundler and linter

Redocly was reporting that SCHEM_A (and the others) was an invalid references

discriminator:
  propertyName: type
  mapping:
    SCHEMA_A: SCHEMA_A
    SCHEMA_B: SCHEMA_B
    SCHEMA_C: SCHEMA_C
oneOf:
  - $ref: ./schema_a.yaml
  - $ref: ./schema_b.yaml
  - $ref: ./schema_c.yaml

Relative Reference Approach: Issue: Invalid $ref from Redocly bundler and linter

Redocly was reporting that SCHEM_A (and the others) was an invalid references. I'm guessing this approach doesn't work because we're in a separate file so the # isn't starting from the openapi.yaml? It may work in a single, large file?

discriminator:
  propertyName: type
  mapping:
    SCHEMA_A: '#/components/schemas/SCHEMA_A'
    SCHEMA_B: '#/components/schemas/SCHEMA_B'
    SCHEMA_C: '#/components/schemas/SCHEMA_C'
oneOf:
  - $ref: ./schema_a.yaml
  - $ref: ./schema_b.yaml
  - $ref: ./schema_c.yaml

Server Reference Approach: Issue: Invalid $ref from Redocly bundler and linter

Redocly was reporting that SCHEM_A (and the others) was an invalid references. When I tried this, it was complaining that the file/directory doesn't exist

discriminator:
  propertyName: type
  mapping:
    SCHEMA_A: './components/schemas/SCHEMA_A'
    SCHEMA_B: './components/schemas/SCHEMA_B'
    SCHEMA_C: './components/schemas/SCHEMA_C'
oneOf:
  - $ref: ./schema_a.yaml
  - $ref: ./schema_b.yaml
  - $ref: ./schema_c.yaml

Server Reference V2 Approach: Issue: Invalid $ref from Redocly bundler and linter

Redocly was reporting that SCHEM_A (and the others) was an invalid references. I was just guessing at this point

discriminator:
  propertyName: type
  mapping:
    SCHEMA_A: './#components/schemas/SCHEMA_A'
    SCHEMA_B: './#components/schemas/SCHEMA_B'
    SCHEMA_C: './#components/schemas/SCHEMA_C'
oneOf:
  - $ref: ./schema_a.yaml
  - $ref: ./schema_b.yaml
  - $ref: ./schema_c.yaml

Are you able to get a valid example working that I can use as a reference? I'm not sure how to get this working where my schemas are split across a number of local files and I'm using Redocly to bundle them into a single large file.

its-hammer-time avatar Jul 02 '24 20:07 its-hammer-time

You can create a dummy schema in components schemas with a ref to each of those mapping schemas and then use the plain string reference mapping value


...

discriminator:
  mapping:
    A: schema_a
  ...


components:
  schemas:
    schema_a:
      $ref: './schema_a.yaml'

It should resolve this schema and the discriminator correctly.

I'm on mobile so can't test it, but it should work

jeremyfiel avatar Jul 02 '24 21:07 jeremyfiel

Right, but that's using a single OpenAPI file right? We have our spec split across multiple directories and bundle them into a single file in our docker image.

its-hammer-time avatar Jul 02 '24 21:07 its-hammer-time

You should be able to add them to the root file and they will be resolved and it should work when bundled

jeremyfiel avatar Jul 02 '24 22:07 jeremyfiel

Interesting. It still creates the components section for a --dereferenced bundle. So it should be possible to reference those schemas directly.

I slightly modified the example:

openapi.yaml:

openapi: 3.1.0
info: {}
paths:
  /test:
    get:
      responses:
        default:
          content:
            application/json:
              schema:
                $ref: example/my-example.yaml

example/my-example.yaml:

discriminator:
  propertyName: type
  mapping:
    VAL_1: ../values/val1.yaml
    VAL_2: ../values/val2.yaml
allOf:
  - $ref: ../values/val1.yaml
  - $ref: ../values/val2.yaml

This produces the following bundle (please notice the comments about replacement):

openapi: 3.1.0
info: {}
paths:
  /test:
    get:
      responses:
        default:
          content:
            application/json:
              schema:
                discriminator: &ref_2
                  propertyName: type
                  mapping:
                    VAL_1: ../values/val1.yaml # Replace this with '#/components/schemas/val1'!
                    VAL_2: ../values/val2.yaml # Replace this with '#/components/schemas/val2'!
                allOf: &ref_3
                  - type: object
                    properties: &ref_0
                      type:
                        type: string
                      name:
                        type: string
                  - type: object
                    properties: &ref_1
                      type:
                        type: string
                      address:
                        type: string
components:
  schemas:
    val1:
      type: object
      properties: *ref_0
    val2:
      type: object
      properties: *ref_1
    my-example:
      discriminator: *ref_2
      allOf: *ref_3

@its-hammer-time is this how you think it should work?

tatomyr avatar Jul 22 '24 14:07 tatomyr

@its-hammer-time is this how you think it should work?

Yeah exactly! Either swap it for #/components/... or resolve the references inline much like what happens under allOf. I'm not sure what would be more appropriate, but something needs to happen as the local file reference is no longer valid if the spec is bundled somewhere else.

its-hammer-time avatar Jul 26 '24 19:07 its-hammer-time

Hey team, any update on this?

its-hammer-time avatar Aug 06 '24 15:08 its-hammer-time

Hey @tatomyr - I see you have a PR out with a fix. Any update on when that's expected to be released?

its-hammer-time avatar Sep 09 '24 15:09 its-hammer-time

@its-hammer-time no idea TBH. It's currently under review, and I'll release it as soon as it gets merged. However, I cannot predict when (or even whether) it will be approved.

tatomyr avatar Sep 09 '24 15:09 tatomyr

The fix is available in v1.25.8. @its-hammer-time, please let me know if it works for you.

tatomyr avatar Oct 22 '24 08:10 tatomyr

I'm not sure this is exactly the same case, but experiencing a very similar issue with the latest (1.25.11). Migrating from swagger-cli, multiple directories split. Experiencing partial unresolved references (using --derefereced flag).

E.g. here

  parameters:
    - $ref: '../../../common_components/parameters.yaml#/query/site_id'
  responses:
    '200':
      description: OK
      content:
        application/vnd.name.v1+json:
          schema:
            type: array
            description: List of engines.
            items:
              $ref: '../../src/schemas/engines.yaml#/list_engines'
          example:
            - $ref: '../../src/responses/engines.yaml#/examples/one'
            - $ref: '../../src/responses/engines.yaml#/examples/two'

Referece under "items" gets dereferenced. However, the "example" part does not get dereferenced and ends up as this:

                "example": [
                  {
                    "$ref": "../../src/responses/engines.yaml#/examples/one"
                  },
                  {
                    "$ref": "../../src/responses/engines.yaml#/examples/two"
                  }
                ]

Exactly the same source is successfully fully dereferenced using swagger-cli.

anna-agafonova avatar Nov 05 '24 12:11 anna-agafonova

Anna

A $ref in an example is invalid per the Specification. It should only be a primitive type matching the actual, expected value.

example:
- [{"engines": {}}]

Meanwhile, using examples does indeed accept a $ref.

examples:
  My_example:
    $ref: '../../src/responses/engines.yaml#/examples/one'

jeremyfiel avatar Nov 05 '24 14:11 jeremyfiel