feat: add middleware management API specs#196
Conversation
enAPI spec with 7 endpoints and FOCA integration
Reviewer's GuideAdds a new OpenAPI 3.0 specification for runtime middleware management and wires it into FOCA/Connexion configuration and project docs, without yet implementing controller logic. Sequence diagram for adding a new middleware via the OpenAPI specsequenceDiagram
actor Admin
participant Client as API_client
participant Connexion as FOCA_Connexion
participant Controller as MiddlewaresController
participant DB as MongoDB_middlewares
Admin->>Client: Prepare MiddlewareCreate_payload
Client->>Connexion: POST /ga4gh/tes/v1/middlewares
Connexion->>Connexion: Validate_request_against_OpenAPI
Connexion->>Controller: addMiddleware(MiddlewareCreate)
Controller->>DB: Query existing_middlewares_for_order
DB-->>Controller: Existing_middlewares_with_orders
Controller->>Controller: Compute_final_order_and_shift_stack
Controller->>DB: Insert new_middleware_document
DB-->>Controller: Insert_result_with_id
Controller-->>Connexion: 201 MiddlewareCreateResponse
Connexion-->>Client: HTTP_201_with_JSON_body
Client-->>Admin: Show_created_middleware_id_and_order
Class diagram for middleware management OpenAPI schemasclassDiagram
class MiddlewareConfig {
string _id
string name
string class_path_string
string[] class_path_group
int order
string source
string github_url
object config
boolean enabled
string created_at
string updated_at
}
class MiddlewareCreate {
string name
string class_path_string
string[] class_path_group
int order
string github_url
object config
boolean enabled
}
class MiddlewareUpdate {
string name
int order
object config
boolean enabled
}
class MiddlewareList {
MiddlewareConfig[] middlewares
int total
int limit
int offset
}
class MiddlewareCreateResponse {
string _id
int order
string message
}
class MiddlewareOrder {
string[] ordered_ids
}
class ValidationRequest {
string class_path
string github_url
string code
}
class ValidationErrorItem {
int line
int column
string message
string severity
}
class ValidationWarningItem {
int line
string message
string severity
}
class ValidationResponse {
boolean valid
string message
ValidationErrorItem[] errors
ValidationWarningItem[] warnings
}
class ErrorResponse {
string error
string message
object details
string timestamp
}
MiddlewareList "1" --> "*" MiddlewareConfig : contains
MiddlewareCreateResponse --> MiddlewareConfig : _id_and_order_align_with
MiddlewareOrder "1" --> "*" MiddlewareConfig : orders
ValidationResponse "1" --> "*" ValidationErrorItem : has
ValidationResponse "1" --> "*" ValidationWarningItem : has
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
MiddlewareCreateResponseschema only exposes_id,order, andmessage, but the docs indocs/middleware.mdstate that it returns the full middleware object; consider either expanding the response schema (e.g., embedMiddlewareConfig) or updating the docs to reflect the actual payload to avoid confusion for API consumers. - In
ValidationRequest, theclass_pathfield is described as "Class path or code to validate" while a separatecodefield is also defined; it would be clearer to tighten the descriptions and possibly add explicit rules (e.g., mutual exclusivity or precedence) so clients know exactly how these fields should be used together.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `MiddlewareCreateResponse` schema only exposes `_id`, `order`, and `message`, but the docs in `docs/middleware.md` state that it returns the full middleware object; consider either expanding the response schema (e.g., embed `MiddlewareConfig`) or updating the docs to reflect the actual payload to avoid confusion for API consumers.
- In `ValidationRequest`, the `class_path` field is described as "Class path or code to validate" while a separate `code` field is also defined; it would be clearer to tighten the descriptions and possibly add explicit rules (e.g., mutual exclusivity or precedence) so clients know exactly how these fields should be used together.
## Individual Comments
### Comment 1
<location> `pro_tes/config.yaml:82` </location>
<code_context>
+ - api/middleware_management.yaml
+ add_operation_fields:
+ x-openapi-router-controller: pro_tes.api.middlewares.controllers
+ disable_auth: True
+ connexion:
+ strict_validation: True
</code_context>
<issue_to_address>
**🚨 issue (security):** Exposing middleware management without auth looks risky for production use.
Since this endpoint can change the middleware stack at runtime, keeping `disable_auth: True` lets anyone with network access reorder/enable/disable/add middlewares. Unless this is strictly limited to a trusted/debug environment, this should require authz (or be configurable per deployment) to avoid a straightforward privilege escalation path.
</issue_to_address>
### Comment 2
<location> `pro_tes/api/middleware_management.yaml:572-358` </location>
<code_context>
+ - "507f1f77bcf86cd799439012"
+ - "507f1f77bcf86cd799439013"
+
+ ValidationRequest:
+ type: object
+ description: Request body for validating middleware code
+ required:
+ - class_path
+ properties:
</code_context>
<issue_to_address>
**issue (bug_risk):** ValidationRequest requires class_path even when validating raw code, which conflicts with the description.
The schema contradicts the docs: `code` is documented as an alternative to `class_path`, but `class_path` is required. This blocks use cases where clients submit only raw code or only a `github_url`. Please adjust the schema (e.g., with `oneOf`/`anyOf` to require at least one of `class_path`, `github_url`, or `code`, or by making `class_path` optional and clarifying the validation rules) so the spec matches the intended behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive OpenAPI 3.0 specification for middleware management functionality in proTES, enabling dynamic management of middleware components at runtime. This is the first subtask in a multi-part implementation following a design-first approach, focusing solely on the API specification and documentation without implementing the actual controller logic.
Changes:
- Added complete OpenAPI 3.0 specification defining 7 REST endpoints for middleware CRUD operations, reordering, and validation
- Configured FOCA integration with new middleware management API routes and MongoDB collection
- Added comprehensive documentation describing the API design, endpoints, data models, and implementation plan
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| pro_tes/api/middleware_management.yaml | New OpenAPI 3.0 specification defining middleware management API with 7 endpoints and 9 schemas |
| pro_tes/config.yaml | Added FOCA integration for middleware API, database collection configuration, and routing setup |
| docs/middleware.md | Comprehensive documentation covering API design, implementation details, security considerations, and future work |
Comments suppressed due to low confidence (1)
docs/middleware.md:210
- The changelog date "2026-01-24" is in the future. This should be corrected to use the actual date when this work was completed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b3118e8 to
5025c12
Compare
uniqueg
left a comment
There was a problem hiding this comment.
I didn't have a chance to review everything yet, but please go ahead and address the existing comments.
uniqueg
left a comment
There was a problem hiding this comment.
Still some ways to go, but we're getting there! :)
uniqueg
left a comment
There was a problem hiding this comment.
Okay, this is starting to look much better now. There are four issues left, 2 previously existing ones, and 2 new ones. With those addressed, I think we are good to go.
Previously existing issues/comments:
- Completely remove
enabledfrom all models, examples & descriptions etc.; we don't want to disable/enable middlewares for now - Ensure there is a way how middlewares within a fallback group can be reordered and deleted
New issues/comments:
- The
MiddlewareCreatemodel is redundant (usingreadOnlywas also addressed previously, but ignored) MiddlewareSourceneeds some improvements
After you have addressed an issue, please leave a message in the conversation where you briefly confirm what you did. This makes it much easier for reviewing. But don't resolve the issues, because that risks that issues are overlooked - leave resolving issues up to the people who have raised them.
uniqueg
left a comment
There was a problem hiding this comment.
This looks good, thanks a lot. Please address the other 4 comments I have sent in the chat. Then we're ready to merge.
uniqueg
left a comment
There was a problem hiding this comment.
Great job, thanks a lot, @keshxvdayal
Detailed Implementation Plan
Create OpenAPI Specification Document
data/specs/middleware_management.yaml/ga4gh/tes/v1/middlewares)GET /middlewares- List all configured middlewaresPOST /middlewares- Add new middleware (local or from GitHub)GET /middlewares/{middleware_id}- Get middleware detailsPUT /middlewares/{middleware_id}- Update middleware configurationDELETE /middlewares/{middleware_id}- Remove middlewarePUT /middlewares/reorder- Reorder middleware stackPOST /middlewares/validate- Validate middleware code without addingMiddlewareConfig- Full middleware configuration objectMiddlewareCreate- Request body for creating middlewareMiddlewareUpdate- Request body for updating middlewareMiddlewareList- Array response with pagination supportMiddlewareOrder- Order configuration for reorderingValidationRequest- Code validation requestErrorResponse- Standard error formatIntegrate with FOCA Configuration
pro_tes/config.yamlto reference the new specWrite API Documentation
docs/api/middleware_management.mdValidation and Review
openapi-generator validateSummary by Sourcery
Add an OpenAPI specification and configuration wiring for a new runtime middleware management API in proTES.
New Features:
Enhancements:
Documentation: