FTL icon indicating copy to clipboard operation
FTL copied to clipboard

fix: make `get_domains` parameters optional

Open tien opened this issue 10 months ago • 6 comments

I'm trying to generate a client from this OpenAPI spec. And it appears that the parameter type for get_domains is incorrect.

tien avatar Feb 25 '25 03:02 tien

Please base your PR on and target to development branch.

yubiuser avatar Feb 25 '25 04:02 yubiuser

Looks like the validator is just buggy and won't accept the allOf syntax :v

So I've replaced it with another one, also with added bonus of this one working with relative paths.

tien avatar Feb 27 '25 00:02 tien

This is too much of a change, looking at various documentation, e.g. Swagger it seems what you want to add is not valid/possible plus invalid syntax.

Instead of

        parameters:
          - allOf:
            - $ref: 'domains.yaml#/components/parameters/type'
            - required: false
          - allOf:
            - $ref: 'domains.yaml#/components/parameters/kind'
            - required: false
          - allOf:
            - $ref: 'domains.yaml#/components/parameters/domain'
            - required: false

it should rather be

        parameters:
            - $ref: 'domains.yaml#/components/parameters/type'
              required: false
            - $ref: 'domains.yaml#/components/parameters/kind'
              required: false
            - $ref: 'domains.yaml#/components/parameters/domain'
              required: false

but even this is invalid because of grafik (screenshot from my link above) and would need to be added further up in this endpoint definition (there is already a parameters object).

The validator change you suggest does not solve the issue, it just doesn't check for this requirement of the OpenAPI specs and, hence, is incomplete.


What we'd need instead is to define new endpoints and use referencing to keep copy-pasting minimal:

+GET /domains
+GET /domains/{type}
+GET /domains/{type}/{kind}
 GET /domains/{type}/{kind}/{domain}

All other (POST, DELETE, PUT) require the parameters. And this then also for

+GET /groups
 GET /groups/{name}
+GET /clients
 GET /clients/{client}
+GET /lists
 GET /lists/{list}

DL6ER avatar Feb 27 '25 05:02 DL6ER

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Mar 06 '25 20:03 github-actions[bot]

Conflicts have been resolved.

github-actions[bot] avatar Mar 24 '25 03:03 github-actions[bot]

@DL6ER I've undo the unnecessary changes & split the endpoints into multiple ones as requested.

tien avatar Mar 24 '25 08:03 tien