Proposal: refactor OpenAPI 3 import/export

[bzp2010]

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

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:

/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.

[starsz]

/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?

[bzp2010]

Hi, @starsz

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.

Is there also an option here to allow users to customize the switch for this feature?

[starsz]

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.