You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/06/06 03:53:46 UTC
[GitHub] [apisix-dashboard] bzp2010 opened a new issue, #2465: Proposal: refactor OpenAPI 3 parse and convert
bzp2010 opened a new issue, #2465:
URL: https://github.com/apache/apisix-dashboard/issues/2465
### Background
Although APISIX Dashboard supports OpenAPI3 specification, it is actually designed to export from APISIX and then import (even it doesn't do well in this area), it has poor support for importing standard OpenAPI3 documents, and we need to improve this.
The following problems exist in the current implementation:
- Cannot import if `operationId` field does not exist
This is our original implementation of the field added to the "export", in line with the OAS specification, but other documents sometimes do not set this value, resulting in the inability to generate route name, the schema will check failure.
- Uri generation error
When a single path has more than one `{xxx}` variable defined, wildcards are truncated directly after the first uri, and the remaining parts of the uri are ignored
```text
Incorrent:
/test/{root}/test/{sub} => /test/*
Corrent:
/test/{root}/test/{sub} => /test/*/test/*
```
- Different methods of the same path are generated as multiple routes
When there are multiple HTTP Methods on the same path, multiple routes will be generated. (Current improvements may have been made, but they do not fully meet the requirements.)
- OAS's `Servers` field is ignored
Currently only focus on custom configurations like `x-apisix-upstream`, ignoring the `servers` field in the OAS specification.
- Over-coupled code
The current import logic is heavily coupled with the handler and cannot be separated, and is not decoupled using the currently defined `Loader` interface.
Therefore, this **Proposal** will fix the issues.
### Scheme
- Cannot import if `operationId` field does not exist
No longer relies on `operationId` to generate route id and name.
Perhaps we can use uri as a material to generate id and name. For example, the following rules:
```text
/test/{user_id}/info => ID: test-user_id-info
```
No better ideas at the moment, hope to get good methods.
- Uri generation error
Correction generation algorithm
- Different methods of the same path are generated as multiple routes
Provide the option for the user to specify whether to merge or not (merging will result in loss when different methods have different security definitions, so it needs to be allowed without merging)
- OAS's `Servers` field is ignored
Prefer to generate upstreams using the Servers field, and generate empty upstreams when servers do not exist.
- Over-coupled code
Using the Loader interface to implement functionality, the original handler implementation will be preserved in this PR and migrated to the Loader implementation in the next PR and marked as deprecated.
### Timetable
- [ ] Implementing new decoder and converter: Ensure that API 101OAS documents converted by Postman can be parsed
- [ ] Migration of existing implementations to the Loader interface
- [ ] Import using the Loader interface is supported on the data loader handler.
- [ ] Change front-end forms to adapt user options for various Loaders.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on issue #2465: Proposal: refactor OpenAPI 3 import/export
Posted by GitBox <gi...@apache.org>.
starsz commented on issue #2465:
URL: https://github.com/apache/apisix-dashboard/issues/2465#issuecomment-1150952230
> Maybe so, but in APISIX we actually allow empty upstream configurations without nodes as well. When importing OpenAPI, there are cases where no server address is defined, but based on the logic that "an API within an API set may be provided by a single service", it is acceptable to provide the creation of an empty upstream as a final guarantee that the user can modify the node information in this upstream to configure the backend for services.
Get it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] bzp2010 commented on issue #2465: Proposal: refactor OpenAPI 3 import/export
Posted by GitBox <gi...@apache.org>.
bzp2010 commented on issue #2465:
URL: https://github.com/apache/apisix-dashboard/issues/2465#issuecomment-1148409921
> Can we use "hash" or base64 instead ?
Yes, I will use it as `id`, and use uri as `name`
> Maybe we shouldn't merge them.
This is a good question, a user once submitted an issue asking why we don't merge methods into a single route, he thought it was a buggy situation and submitted a PR fix for it, which was eventually merged as well.
However, the reality shows that there are indeed scenarios in which merging is not possible, so I think this configurability should be preserved.
> Generate empty upstreams is not a good idea, maybe we should report an error?
Maybe so, but in APISIX we actually allow empty upstream configurations without nodes as well. When importing OpenAPI, there are cases where no server address is defined, but based on the logic that "an API within an API set may be provided by a single service", it is acceptable to provide the creation of an empty upstream as a final guarantee that the user can modify the node information in this upstream to configure the backend for services.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on issue #2465: Proposal: refactor OpenAPI 3 import/export
Posted by GitBox <gi...@apache.org>.
starsz commented on issue #2465:
URL: https://github.com/apache/apisix-dashboard/issues/2465#issuecomment-1147076037
> /test/{user_id}/info => ID: test-user_id-info
Can we use "hash" or base64 instead ?
> Provide the option for the user to specify whether to merge or not (merging will result in loss when different methods have different security definitions, so it needs to be allowed without merging)
Maybe we shouldn't merge them.
> Prefer to generate upstreams using the Servers field, and generate empty upstreams when servers do not exist.
Generate empty upstreams is not a good idea, maybe we should report an error?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org