-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Draft] A plugin that dynamically generates an OpenAPI spec #11843
Conversation
❌ Gradle check result for f717c19: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Compatibility status:Checks if related components are compatible with change 5e7b87e Incompatible componentsIncompatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/k-nn.git] Skipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/performance-analyzer.git] |
Passing ./gradlew plugins:api:yamlRestTest. Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
❌ Gradle check result for 3504590: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: dblock <[email protected]>
❌ Gradle check result for 5cae02a: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for c70d02f: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for f44b777: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: dblock <[email protected]>
❌ Gradle check result for 205de2e: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
❕ Gradle check result for de6c3f7: UNSTABLE
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Is there a question-behind-the-question here? Architecturally, I like this as a plugin/module because it doesn't need deep integration into parts of the core. Are you questioning whether this should be a module (aka a plugin that is always installed in the min distribution)? I would guess most users would not use the OpenAPI spec generated by this plugin, but also this plugin is quite lean so including it by default is probably fine.
Given the size and complexity of core my default position is to create it as a separate repo. As it is, this plugin is quite lean so including in the core repo is probably fine. But if we have any expectation that we might want to add functionality or add additional OpenAPI-related tooling to this plugin, then that would push me even stronger towards a separate repo.
I lean towards RestController having a public method that describes all REST methods it provides, versus exposing the more detailed and functional getHandlers()/RestHandler. That is exactly what |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is amazing! A few thoughts:
- If this could also generate the params for the APIs.
- If this completely generates the specs for the server in the future, would this not go against the generation of server if we want to do that some day?
Yes, I'll try to be clearer. Assuming it can be complete, do we want dynamic OpenAPI spec a core feature enabled by default in OpenSearch? That would likely imply building facilities to describe the API in annotations. That has advantages, such as clients being able to rely on the fact that each different deployment of OpenSearch can dynamically express what its API is at runtime, and therefore we can use generic clients and frameworks that are OpenAPI-compatible.
I am looking at that next.
You are correctly pointing out that we can do either spec -> server or server -> spec, but we don't need to do both. |
With #11876 re-extracted this code into a separate plugin, https://github.com/dblock/opensearch-api (dblock/opensearch-api@5adec70). |
Description
This POC generates a partial OpenAPI spec from the existing registered REST handlers. It can be used, for example, by assembling a distribution of OpenSearch with all its plugins, and then generating the OpenAPI spec from that instance at runtime like in opensearch-project/opensearch-api-specification#179 where a report is being generated about what APIs are and aren't documented in the OpenAPI spec.
Looking for feedback.
RestController#toXContent
or makegetHandlers()
andRestHandler
public?Currently it returns something like this:
curl http://localhost:9200/_plugins/api | jq
In its complete form, this plugin could offers a few capabilities.
2.x, https://github.com/dblock/OpenSearch/tree/2.x-api-plugin
Related Issues
Other
Running this plugin on 2.11.
Build OpenSearch 2.11 + plugin
Create a
Dockerfile
Build and Run Docker Image
Curl
Compare
The spec repo has
OpenSearch.openapi.json
. We createdOpenSearch.auto.openapi.json
above.Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.