-
Notifications
You must be signed in to change notification settings - Fork 10
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
Non-Routable Swagger #2
Comments
Hi @AlskiOnTheWeb. The OpenAPI definitions generated are a compromise in that many of the AWS definitions do overload paths as you say. Including fragment identifiers is an abuse of the OpenAPI specification (as the path object key should be a path, which does not allow the # character). The intention wasn't to favour client-generation over server-generation (or documentation generation, mocking or anything else), but some mechanism had to be chosen so as to be able to represent the API described by the source definition. Creating API definitions which passed (multiple) existing validators was a concern. Where you say:
Do you mean "are really going to" or "aren't really going to"? My understanding is that the fragment identifier is never sent from client to server, so the server must interpret either the request body properties or the presence/absence of query-string or header parameters to decide which overloaded method was actually intended. The real-world AWS API servers must be doing that inference today when talking to existing clients, so I think the definition is "routable", but possibly not compatible with the expectations of all server-generation tools. It would theoretically be possible to merge all parameters and request body models of overloaded paths/operations into one large mega-operation, but at the cost of losing most of the expressivity of the API description. The logic to untangle the overloaded requests would still be necessary, only it would would be far harder to determine what the logic should actually be from the API definition alone. If you could tell me a little more about the technologies you're using, perhaps we can come up with another solution, but with regards to OpenAPI 2.0 at least, I don't hold out much hope. |
Correct, "aren't" was my intention.
It looks like they mostly trigger rather than infer on the "X-Amz-Target" header that isn't properly modeled (should be an enum?) in the Swagger.
|
Interesting, unfortunately it seems there is some inconsistency in the formatting of the
Though that is present in the "metadata":{
"apiVersion":"2012-08-10",
"endpointPrefix":"dynamodb",
"jsonVersion":"1.0",
"protocol":"json",
"serviceAbbreviation":"DynamoDB",
"serviceFullName":"Amazon DynamoDB",
"signatureVersion":"v4",
"targetPrefix":"DynamoDB_20120810",
"uid":"dynamodb-2012-08-10"
} It doesn't seem to be 'globally' documented. As with the signing logic, there is a level of 'known magic' required to access AWS APIs which can't always be expressed in an OpenAPI definition. Would it help you if the |
Not really. The duplicate / overlapping paths is more the real issue on my end. I'm just not sure what to do. I think I'm going to try and collapse them and do something less intelligent when I see them. As you said, this stuff doesn't model in OpenAPI...which is part of why I dislike OpenAPI. They wrote it for greenfield usage, not for real world usage. |
If you think it would be easier to collapse the source definition before it is converted to OpenAPI I would happily accept a PR (controlled by a command-line option) :) |
OK, I'll take a look to see if there, at the end of the day, is a way I can make it reasonably consistent. |
I'm considering taking a look at this. I haven't got deeply into it yet, but I think in theory we should be able to solve this with OpenAPI 3, by heavily discriminating schemas everywhere based on the Action parameter: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#discriminator-object. That would mean:
This sounds a bit mad, but in theory it accurately represents what's going on here, and any OpenAPI 3 compatible parser should be able to build a full & correct JSON schema for any request (as a client or a server), and can then validate against that correctly. The main downside is that some fields can't be discriminated, but that mainly just affects the path/operation descriptions. It's annoying, but I can't see a nice way around it. I've put together a quick example below based on the existing cloudformation schema, for just two actions (with the schemas for each action's individual params omitted). Does it make sense? Do you think it'd work? I'm not too sure how to test this. openapi: 3.0.0
info:
version: '2010-05-15'
title: AWS CloudFormation
paths:
/:
post:
operationId: POST
parameters:
- $ref: '#/components/parameters/Action'
- in: query
name: params
required: true
style: form
explode: true
schema:
oneOf:
- $ref: '#/components/schemas/ContinueUpdateRollbackParams'
- $ref: '#/components/schemas/CreateChangeSetParams'
discriminator:
propertyName: Action
mapping:
ContinueUpdateRollback: '#/components/schemas/ContinueUpdateRollbackParams'
CreateChangeSet: '#/components/schemas/CreateChangeSetParams'
responses:
'200':
description: Success
content:
'*/*':
schema:
oneOf:
- $ref: '#/components/schemas/ContinueUpdateRollbackOutput'
- $ref: '#/components/schemas/CreateChangeSetOutput'
discriminator:
propertyName: Action
mapping:
ContinueUpdateRollback: '#/components/schemas/ContinueUpdateRollbackOutput'
CreateChangeSet: '#/components/schemas/CreateChangeSetOutput'
servers:
- url: 'https://cloudformation.amazonaws.com/'
components:
parameters:
Action:
name: Action
in: query
required: true
schema:
type: string
enum:
- ContinueUpdateRollback
- CreateChangeSet
schemas:
ContinueUpdateRollbackOutput:
type: object
properties: {}
description: The output for a <a>ContinueUpdateRollback</a> action.
ContinueUpdateRollbackParams:
type: object
required:
- StackName
properties:
StackName:
$ref: '#/components/schemas/StackNameOrId'
RoleARN:
$ref: '#/components/schemas/RoleARN'
ResourcesToSkip:
$ref: '#/components/schemas/ResourcesToSkip'
ClientRequestToken:
$ref: '#/components/schemas/ClientRequestToken'
description: The input for the <a>ContinueUpdateRollback</a> action.
CreateChangeSetOutput:
type: object
properties:
Id:
$ref: '#/components/schemas/ChangeSetId'
StackId:
$ref: '#/components/schemas/StackId'
description: The output for the <a>CreateChangeSet</a> action.
CreateChangeSetParams:
type: object
required:
- StackName
- ChangeSetName
properties:
StackName:
$ref: '#/components/schemas/StackNameOrId'
TemplateBody:
$ref: '#/components/schemas/TemplateBody'
TemplateURL:
$ref: '#/components/schemas/TemplateURL'
UsePreviousTemplate:
$ref: '#/components/schemas/UsePreviousTemplate'
Parameters:
$ref: '#/components/schemas/Parameters'
Capabilities:
$ref: '#/components/schemas/Capabilities'
ResourceTypes:
$ref: '#/components/schemas/ResourceTypes'
RoleARN:
$ref: '#/components/schemas/RoleARN'
RollbackConfiguration:
$ref: '#/components/schemas/RollbackConfiguration'
NotificationARNs:
$ref: '#/components/schemas/NotificationARNs'
Tags:
$ref: '#/components/schemas/Tags'
ChangeSetName:
$ref: '#/components/schemas/ChangeSetName'
ClientToken:
$ref: '#/components/schemas/ClientToken'
Description:
$ref: '#/components/schemas/Description'
ChangeSetType:
$ref: '#/components/schemas/ChangeSetType'
description: The input for the <a>CreateChangeSet</a> action. |
Alternatively OAI/OpenAPI-Specification#182 would solve this, and it sounds like that still an option for OpenAPI 3.1 (OAI/OpenAPI-Specification#1466). Presumably that's a way off though, so a short-term workaround like this would be nice. |
We're going to discuss that in an upcoming OpenAPI TSC meeting. 3.1 is hopefully due in May, and this issue has been open much longer than 3 months. Maybe the first step should be migrating the converter to OAS v3? |
Couple of questions re: your first proposal @pimterry
Is this always (necessarily) the case for every AWS API with this problem, or should each set of 'conflicting' operations be coalesced under whatever path value makes sense?
If Maybe allowing |
I'm not super familiar with the AWS APIs, so I'm not totally sure, but it does look like in practice many of them have many operations which all share one single path, with with an action param. It's certainly not necessarily always one operation though, I'm just suggesting collapsing any conflicting operations like this.
Hmm, yes, I think I've misinterpreted this. I was hoping that you could discriminate responses on request params, but I don't think that's true. From the AWS APIs themselves, I think you can always find a part of the response to discriminate from instead, but it is more complicated. E.g.
I'm a bit cautious about runtime expressions, rather than generic pattern matching in some form. I think one nice way to improve all this would be:
|
Sorry, that's exactly what I meant by "runtime expression" as used in the OAS:
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#runtimeExpression |
I've been thinking more about this. Some kind of runtime expression discriminator would be good, but for my case at least it'd be very useful to discriminate the entire operation (the description especially). Long-term OpenAPI spec changes might help there, but I'd love to come up with a short term solution that makes it possible to route/match requests in the meantime, under the current spec. Right now the specs here are not only not automatically routed by existing tools, it's effectively impossible to build or change tools to start routing these correctly without a special case for each spec here, as there's no info on how to decide which I think the algorithm to find the relevant path for a request is usually to match the Given that, would you be open to updating the format to something like this: openapi: 3.0.0
info:
version: '2010-05-15'
title: AWS CloudFormation
paths:
/#Action=CreateChangeSetParams:
post:
operationId: CreateChangeSetParams
parameters:
- $ref: '#/components/parameters/Action'
responses:
'200':
description: Success
/#Action=ContinueUpdateRollbackParams:
post:
operationId: ContinueUpdateRollbackParams
parameters:
- $ref: '#/components/parameters/Action'
responses:
'200':
description: Success The only real difference is the This still validates just like the existing specs you're producing here, and existing tools should work fine (ignoring fragments and treating the paths as ambiguous), but it'd also be immediately possible to add some easy logic that can automatically route these requests. In future, when there's formal spec support for some kind of operation discriminator, it should be easy to convert this format into an officially supported runtime expression matching approach, and in the meantime it's much simpler than my previous suggestion. What do you think? If that format would be acceptable, I'd be happy to investigate opening a PR here for this change. |
Yes, I think the explicit |
Ok, great, thanks! I'll try and put a PR together soon, probably next week. I think OpenAPI v2 is fine for now, it's probably better to do that change independently. |
@AlskiOnTheWeb feel free to re-open this if it doesn't fix your issue or you can see further changes that are required. |
Hello,
It appears that the Swagger/OpenAPI definitions you generate for the AWS APIs are "client side only". We have a need for a "server side" version. The difference can be seen in APIs like ACM where the Swagger is using the root (/) path with a number of hashes (/#DescribeCertificate) to differentiate the supplied JSON request body. Well, this hash-based path isn't "routable" in terms of a server as clients are really going to supply the hash, this is more a mechanism for you to overload the definition of the root path. Can you think of a way to make these Swagger files support both a client with the proper body but also the routable server? I realize that the Swagger specification does a real poor job of defining what the paths should be but to me, hashes are sort of abusing it.
Thanks.
The text was updated successfully, but these errors were encountered: