-
Notifications
You must be signed in to change notification settings - Fork 54
Add automated OpenAPI specification for mgmt_api
#309
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
base: main
Are you sure you want to change the base?
Conversation
bd43733 to
24a03c6
Compare
0094c82 to
94ef57e
Compare
a71724a to
8c6f729
Compare
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.
Pull Request Overview
This PR introduces automated OpenAPI 3.0 specification generation for the Drasi Management API, enabling automatic documentation and API contract validation. The implementation uses the utoipa crate to generate OpenAPI specs directly from Rust code annotations, ensuring the documentation stays synchronized with the implementation.
Key Changes:
- Replaced macro-based route generation with explicit handler modules annotated with
#[utoipa::path]for OpenAPI documentation - Added automated CI/CD verification to prevent API specification drift and breaking changes
- Integrated Swagger UI for interactive API documentation
Reviewed Changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| control-planes/mgmt_api/src/main.rs | Updated routing configuration to use new explicit handler modules and added Swagger UI service |
| control-planes/mgmt_api/src/lib.rs | New library file exposing API modules for the spec generation binary |
| control-planes/mgmt_api/src/api/v1/*.rs | New explicit handler modules replacing macro-generated routes with utoipa annotations |
| control-planes/mgmt_api/src/api/v1/openapi.rs | Central OpenAPI documentation configuration |
| control-planes/mgmt_api/src/api/v1/models/*.rs | Added ToSchema derives for OpenAPI schema generation |
| control-planes/mgmt_api/openapi.yaml | Generated OpenAPI 3.0 specification |
| control-planes/mgmt_api/Makefile | Added OpenAPI generation, validation, and serving targets |
| control-planes/mgmt_api/Dockerfile.* | Updated to specify mgmt_api binary explicitly |
| control-planes/mgmt_api/Cargo.toml | Added utoipa dependencies and spec generation binary |
| .github/workflows/build-test.yml | Added CI jobs for OpenAPI spec verification and breaking change detection |
| .githooks/pre-commit | Added pre-commit hook to ensure API spec stays current |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
8c6f729 to
c81c4dc
Compare
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.
Pull Request Overview
Copilot reviewed 32 out of 34 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
c81c4dc to
3d73a50
Compare
This commit introduces a robust, automated system for generating and maintaining an OpenAPI 3.0 specification for the management API.
This provides a version-controlled API contract that is always in sync with the implementation.
The previous macro-based routes have been refactored into explicit Actix handlers.
Generic DTOs in the API layer were replaced with concrete structs (e.g., SourceDto)
to resolve a utoipa limitation and produce a valid spec.
Key changes include:
- Integrated the utoipa crate to generate the spec from Rust code.
- Aligns the mgmt_api crate version to 1.0.0 to match the /v1 API path.
- Adds a pre-commit hook to enforce local spec updates.
- Adds CI jobs to verify spec sync and detect breaking changes.
- verify-openapi-spec: Fails the build if the committed spec is out of date.
- detect-breaking-api-changes: Uses openapi-diff to prevent merging backward-incompatible changes.
Signed-off-by: Aman Singh <aman.singh.original@gmail.com>
3d73a50 to
c1ecc91
Compare
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.
Great contribution! 👍 👍
| ;; | ||
| esac | ||
|
|
||
| echo "Running pre-commit hook: Checking for API spec changes..." |
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.
In my experience, pre-commit hooks are not reliable, as devs often don't set execution permissions or there are cross platform issues.
| ), | ||
| request_body = QuerySpecDto, | ||
| responses( | ||
| (status = 200, description = "Query created or updated", body = ContinuousQueryDto), |
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.
We don't support updating queries
| (status = 500, description = "Internal server error") | ||
| ) | ||
| )] | ||
| pub async fn watch(service: web::Data<DebugService>, id: web::Path<String>) -> impl Responder { |
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.
I don't think we have any of the E2E tests that hit the watch or debug endpoints. We'll need to test these.
|
|
||
| #[derive(Serialize, ToSchema)] | ||
| #[serde(tag = "kind")] | ||
| pub enum ResultEventDto { |
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.
Sine the ResultEvent originates from the QueryHost, would it make sense for the QueryHost to be the source of truth for that schema and publish an OpenAPI spec that the Management API consumes?
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.
EDIT: we have a TypeSpec definition here: https://github.com/drasi-project/drasi-platform/blob/main/typespec/query-output/main.tsp
| ), | ||
| info( | ||
| title = "Drasi Management API", | ||
| version = "1.0.0", |
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.
Should we harmonize thus version with our release versions?
| ) -> impl Responder { | ||
| log::debug!("readywait_resource: {:?}", id); | ||
|
|
||
| if params.timeout > 300 { |
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.
I see this timeout in multiple places, should we manage it from a config file or constant?
Description
This pull request introduces an utomated system for generating and maintaining an OpenAPI 3.0 specification for the Drasi Management API.
The goal is to have a version-controlled API contract that is always in sync with the implementation. This will improve developer experience and documentation, and will enable automated tooling for API validation and client generation.
Key Changes
1. Refactoring for a Valid OpenAPI Generation
Refactoring of the
mgmt_apiservice to make it compatible with OpenAPI generation tools.utoipaIntegration: Theutoipacrate has been integrated into themgmt_apiservice to generate the OpenAPI spec directly from the Rust code.Macro Removal & Handler Refactoring: The previous route-generation macros (
v1_crud_api!) have been completely removed. They are replaced with explicit Actix handler functions (upsert,get,list, etc.) in new, dedicated modules for each resource (e.g.,sources.rs,reactions.rs).Introduction of Concrete DTOs: To solve a fundamental limitation where the
utoipatool could not correctly generate schemas for generic Rust types (e.g.,ResourceDto<TSpec, TStatus>), new concrete DTOs have been introduced for each resource (e.g.,SourceDto,ReactionDto). The handlers have been updated to use these explicit types, which makes the API contract unambiguous and allowsutoipato generate a valid spec.Generated Spec: The resulting valid
openapi.yamlis now checked into the repository atcontrol-planes/mgmt_api/openapi.yaml.2. Automation and CI/CD Integration
Pre-Commit Hook: A new Git pre-commit hook (
.githooks/pre-commit) has been added. It automatically regenerates the OpenAPI spec before each commit and fails if the generated spec has changed, forcing the developer to include the updated spec.CI Verification: The
build-test.ymlGitHub Actions workflow has been updated with two new jobs:verify-openapi-spec: Ensures the committed spec is never out of sync with the code.detect-breaking-api-changes: Usesopenapi-diffto prevent backward-incompatible changes from being merged intomain, thus enforcing thev1API contract.3. API Versioning Alignment
Package Version: The
mgmt_apicrate version inCargo.tomlhas been updated from0.1.0to1.0.0. This aligns the package's major version with the/v1API path.Spec Version: The version in the generated
openapi.yamlis set to1.0.0.4. Tooling and Refinements
Makefile Targets: New
openapi-*targets have been added to thecontrol-planes/mgmt_api/Makefilefor developers to easily generate, validate, and serve the spec locally.Dead Code Removal: The now-unused route-generation macros (
v1_crud_api!) have been removed, cleaning up the codebase.WebSocket Documentation: A placeholder has been added to the OpenAPI spec to document the
/v1/debugWebSocket endpoint.Reasons for refactoring to concrete DTOs
Type of change
This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Drasi.