[Platform API] Add management operations for Deployment Feature#671
[Platform API] Add management operations for Deployment Feature#671
Conversation
WalkthroughReplaces revision-based deployments with immutable deployment artifacts: DB and model schema changes, new deployment DTOs/endpoints, repository methods, DeploymentService and DeploymentHandler (deploy/redeploy/undeploy/delete/list/content), undeploy broadcasting, config/error additions, gateway token/schema updates, and server wiring. (30 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Handler as DeploymentHandler
participant Service as DeploymentService
participant Repo as APIRepository
participant DB as Database
participant Events as GatewayEvents
Client->>Handler: POST /apis/{apiId}/deploy (DeployAPIRequest)
Handler->>Service: DeployAPI(apiUUID, req, orgUUID)
Service->>Repo: CountDeploymentsByAPIAndGateway(api,gateway,org)
Repo->>DB: SELECT COUNT(...)
DB-->>Repo: count
Repo-->>Service: count
alt count >= MaxPerAPIGateway
Service->>Repo: GetOldestUndeployedDeploymentByGateway(...)
Repo->>DB: SELECT oldest undeployed
DB-->>Repo: deployment
Repo-->>Service: oldest deployment
Service->>Repo: DeleteDeployment(oldDeploymentID,...)
Repo->>DB: DELETE ...
DB-->>Repo: deleted
end
Service->>Repo: CreateDeployment(newDeployment)
Repo->>DB: INSERT deployment (deployment_id, content, metadata, status)
DB-->>Repo: inserted
Repo-->>Service: success
Service->>Repo: GetActiveDeploymentByGateway(api,gateway,org)
Repo->>DB: SELECT WHERE status='DEPLOYED' LIMIT 1
DB-->>Repo: previous deployment?
Repo-->>Service: previous deployment or nil
alt previous exists
Service->>Repo: UpdateDeploymentStatus(prevID,'UNDEPLOYED')
Repo->>DB: UPDATE status
DB-->>Repo: updated
Service->>Events: BroadcastUndeploymentEvent(gateway, payload)
Events->>Events: serialize & broadcast
end
Service->>Events: BroadcastDeploymentEvent(gateway, payload)
Events->>Events: serialize & broadcast
Service-->>Handler: DeploymentResponse
Handler-->>Client: 201 Created {deploymentId, status, ...}
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/src/internal/service/gateway_internal.go (1)
273-283: Set deployment Content and Status before insertThe
api_deploymentstable requirescontentasNOT NULLandstatusmust have a valid value. The current code creates anAPIDeploymentwith onlyApiID,GatewayID,OrganizationID, andCreatedAt—leavingContentandStatusunset. TheCreateDeploymentmethod will attempt to insert these empty/null values, causing the insert to fail. PopulateContentwith the API definition and setStatusto a valid value (e.g., "DEPLOYED") before callingCreateDeployment.
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/database/schema.sql`:
- Around line 220-236: The CREATE TABLE for api_deployments has a missing comma
between the table constraints UNIQUE(api_uuid, gateway_uuid, deployment_id,
organization_uuid) and CHECK (status IN (...)), which causes a SQL syntax error;
add a comma after the UNIQUE(...) constraint in the api_deployments table
definition so the UNIQUE constraint and the CHECK constraint are properly
separated and the table (functionally api_deployments) can be created.
In `@platform-api/src/internal/handler/deployment.go`:
- Around line 188-230: The DeleteDeployment handler currently maps service
errors to 500; update error handling after calling
deploymentService.DeleteDeploymentByHandle(apiId, deploymentId, orgId) to map
the service error that indicates an active deployment (e.g.,
constants.ErrDeploymentActive or the specific error returned by your service) to
HTTP 409 with an appropriate utils.NewErrorResponse(409, "Conflict", "<human
message>"), leaving other errors as-is; also return no body for success by using
c.Status(http.StatusNoContent) (or c.JSON(http.StatusNoContent, nil)
consistently) instead of sending a body with 204.
- Around line 256-271: The 500 response currently returns "Deployment not found"
which masks internal errors; in the error branch after calling
h.deploymentService.GetDeploymentByHandle (and after checking
constants.ErrAPINotFound and constants.ErrDeploymentNotFound) change the c.JSON
internal server error body to include the actual error details (e.g.,
err.Error() or a message like "Failed to retrieve deployment") instead of
"Deployment not found" so the response accurately reflects the internal failure
while keeping the existing log.Printf call.
- Around line 314-355: In GetDeploymentContent, the handler currently falls
through after sending error JSON (causing a second response) and sends the wrong
media type; add a return immediately after each c.JSON error response in the
error branch of h.deploymentService.GetDeploymentContentByHandle so the function
exits on error, and change the final c.Data content type from "application/json"
to the correct YAML media type (e.g., "application/x-yaml" or "text/yaml") to
match GenerateAPIDeploymentYAML-produced artifacts.
- Around line 144-186: Handler UndeployDeployment currently sends a 204 with a
JSON null (c.JSON(http.StatusNoContent, nil)) which mismatches the API contract;
update the handler to either return the spec'd 200 with a DeploymentResponse
body (populate and return the deployment via the
deploymentService.UndeployDeploymentByHandle result) or, if you choose to keep
204, replace c.JSON(http.StatusNoContent, nil) with
c.Status(http.StatusNoContent) and ensure no body is sent. Also add explicit
error handling for a "deployment not currently active" error from
UndeployDeploymentByHandle (e.g., constants.ErrDeploymentNotActive or the
service-specific error) and map it to a 400 or 409 response instead of 500; keep
the existing mappings for constants.ErrAPINotFound and
constants.ErrDeploymentNotFound.
In `@platform-api/src/internal/repository/api.go`:
- Around line 1108-1123: The GetDeploymentContent function currently returns a
generic error when the deployment row is missing; change it to return the
package-level sentinel ErrDeploymentNotFound instead of errors.New("deployment
not found") so callers can map to 404s; locate APIRepo.GetDeploymentContent (and
the similar methods around GetDeploymentLogs/GetDeploymentMeta in the same file)
and replace the sql.ErrNoRows branch to return ErrDeploymentNotFound, keeping
other error returns unchanged and ensuring the sentinel is imported/defined in
the package.
- Around line 1327-1332: The LEFT JOIN to api_deployments (alias ad) can match
non-active or older deployments causing duplicates and incorrect is_deployed
flags; change the join to only include active deployments by adding a filter
ad.status = 'DEPLOYED' (and to avoid multiple rows per gateway select the latest
deployment—e.g., join against a subquery that for each gateway_uuid and api_uuid
returns the deployment with max(created_at) or max(deployment_id) before
joining). Update the JOIN condition on ad in the query that references g, aa, ad
(use the existing ON g.uuid = ad.gateway_uuid AND ad.api_uuid = ? plus the
status filter or replace ad with a deduplicated subquery) so is_deployed and
deployed_at reflect the latest deployed artifact only.
- Around line 964-989: In CreateDeployment, json.Marshal(deployment.Metadata)
currently ignores errors and can silently drop invalid metadata; capture the
error when marshaling deployment.Metadata (the code handling metadataJSON inside
CreateDeployment) and return or wrap that error instead of proceeding, ensuring
the INSERT does not run with missing/invalid metadata; update the logic that
sets metadataJSON (and any callers/returns) to handle the marshal error
appropriately and propagate it back to the caller.
In `@platform-api/src/internal/service/deployment.go`:
- Around line 239-244: Replace ad-hoc string errors for missing deployments with
the sentinel error constants.ErrDeploymentNotFound so callers using
errors.Is(...) will map to 404s: locate the branches that check the deployment
variable for nil (e.g., the block that currently returns errors.New("deployment
not found")) and change the return to constants.ErrDeploymentNotFound; do the
same for every other "not found" branch in this file (search for
errors.New("deployment not found") or similar) so all not-found returns
consistently use the ErrDeploymentNotFound sentinel.
- Around line 96-169: The deployment limit check in DeployAPI runs before
undeploying the current active deployment and only deletes one undeployed
artifact, which can exceed s.cfg.Deployments.MaxPerAPIGateway; also failures
from s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, ...,
"UNDEPLOYED", ...) are ignored—make them fatal. Fix by first attempting to
undeploy any existing active deployment via GetActiveDeploymentByGateway and
calling UpdateDeploymentStatus, returning an error if that call fails; then call
CountDeploymentsByAPIAndGateway and, while apiDeploymentCount >=
s.cfg.Deployments.MaxPerAPIGateway, fetch and delete the oldest undeployed
deployment using GetOldestUndeployedDeploymentByGateway and DeleteDeployment
(loop until under the limit), returning errors on any repo failures; apply the
same changes in RedeployDeployment.
In `@platform-api/src/resources/openapi.yaml`:
- Around line 946-973: The OpenAPI operation UndeployDeployment currently
declares a 200 response with schema DeploymentResponse but the implementation
returns 204 No Content; update the spec to match the implementation by replacing
the '200' response under operationId UndeployDeployment with a '204' response
that has description like "Deployment undeployed successfully" and no response
body (remove or stop referencing components/schemas/DeploymentResponse), or
alternatively change the handler to return 200 with the DeploymentResponse
payload—pick one approach and make the operationId UndeployDeployment, its
responses block, and any references to components/schemas/DeploymentResponse
consistent.
- Around line 910-945: The OpenAPI spec defines a required requestBody using the
RedeployRequest schema for the RedeployDeployment operation, but the
handler/service function redeploy currently expects no body; fix by either
making the requestBody optional in the spec (remove required: true or delete the
requestBody block for /apis/{apiId}/deployments/{deploymentId}/redeploy) or by
updating the redeploy implementation to accept and validate RedeployRequest
(consume gatewayId/metadata) and forward it to the redeploy service; ensure the
handler name (RedeployDeployment) and the service method redeploy accept the
same payload shape and update the components/schemas/RedeployRequest usage
accordingly, plus add input validation and tests for both paths.
- Around line 975-996: The OpenAPI response for operationId GetDeploymentContent
(path /apis/{apiId}/deployments/{deploymentId}/content) is incorrect: the
implementation returns raw YAML bytes but the spec declares application/json
with an object schema; update the spec to match the implementation by changing
the 200 response content type to application/x-yaml (or
application/octet-stream) and set the schema to a binary/string payload (e.g.,
type: string, format: binary or type: string with description "raw YAML bytes"),
so clients correctly interpret the raw YAML bytes instead of expecting JSON.
🧹 Nitpick comments (4)
platform-api/src/internal/service/gateway_events.go (1)
138-209: Extract shared broadcast helper to prevent driftBroadcastUndeploymentEvent duplicates the deployment broadcast flow (marshal, size check, connection loop, stats/logging). Consider a shared helper that takes event type/payload so future changes stay consistent across both paths.
platform-api/src/internal/dto/api.go (1)
165-188: Validate DeployAPIRequest.Base values
Baseis free-form but only"current"or a deployment ID are valid. Consider enforcing this via constants/validation to fail fast on bad requests and reduce downstream ambiguity.platform-api/src/internal/service/gateway_internal.go (1)
286-289: Avoid returning legacy deploymentId=0With deployment IDs now strings, returning
DeploymentId: 0can mislead clients. Consider deprecating this field or updating the response type to expose the new deployment ID.platform-api/src/internal/repository/api.go (1)
996-1065: Avoid fetching content blobs in list queries.
GetDeploymentsByAPIUUIDselectscontentfor every row, but the list response doesn’t expose it. This can be a large, unnecessary payload. Consider excludingcontent(or making it opt-in).♻️ Example change (apply to all query branches)
-SELECT deployment_id, api_uuid, organization_uuid, gateway_uuid, status, base_deployment_id, content, metadata, created_at +SELECT deployment_id, api_uuid, organization_uuid, gateway_uuid, status, base_deployment_id, metadata, created_at ... -err := rows.Scan(&deployment.DeploymentID, &deployment.ApiID, &deployment.OrganizationID, - &deployment.GatewayID, &deployment.Status, &baseDeploymentID, &deployment.Content, &metadataJSON, &deployment.CreatedAt) +err := rows.Scan(&deployment.DeploymentID, &deployment.ApiID, &deployment.OrganizationID, + &deployment.GatewayID, &deployment.Status, &baseDeploymentID, &metadataJSON, &deployment.CreatedAt)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
platform-api/src/config/config.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/dto/api.goplatform-api/src/internal/handler/api.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/model/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/api.goplatform-api/src/internal/service/deployment.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/utils/api.goplatform-api/src/resources/openapi.yaml
💤 Files with no reviewable changes (3)
- platform-api/src/internal/handler/api.go
- platform-api/src/internal/utils/api.go
- platform-api/src/internal/service/api.go
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/constants/error.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/config/config.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/server/server.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/repository/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/resources/openapi.yamlplatform-api/src/internal/service/deployment.go
📚 Learning: 2025-12-06T11:57:08.456Z
Learnt from: BLasan
Repo: wso2/api-platform PR: 140
File: gateway/gateway-controller/pkg/utils/api_deployment.go:88-105
Timestamp: 2025-12-06T11:57:08.456Z
Learning: In the gateway-controller Go codebase (file: gateway/gateway-controller/pkg/utils/api_deployment.go), only the async/websub API kind is currently supported. async/websocket and async/sse API kinds are not yet implemented.
Applied to files:
platform-api/src/internal/constants/error.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/server/server.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-15T11:14:32.797Z
Learnt from: VirajSalaka
Repo: wso2/api-platform PR: 686
File: platform-api/src/internal/service/api.go:920-922
Timestamp: 2026-01-15T11:14:32.797Z
Learning: In the platform-api service (file: platform-api/src/internal/service/api.go), API handles (the ID field) are immutable identifiers used as references and should not be updated after API creation. They serve as stable, user-facing identifiers for the API lifecycle.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-09T05:58:05.318Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/memory.go:484-503
Timestamp: 2026-01-09T05:58:05.318Z
Learning: In gateway/gateway-controller/pkg/storage/memory.go, during API key rotation via the StoreAPIKey method, the API key ID is never modified. When an existing API key is found by name (existingKeyID != ""), the incoming apiKey.ID is guaranteed to be the same as existingKeyID, so no ID mismatch can occur during updates.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/service/gateway_internal.go
📚 Learning: 2026-01-09T05:57:36.635Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/apikey_store.go:219-222
Timestamp: 2026-01-09T05:57:36.635Z
Learning: In gateway/gateway-controller/pkg/storage/apikey_store.go, during API key rotation, the API key ID and name remain constant—only the API key value changes. Therefore, when addToAPIMapping finds an existing key by name, the existingKeyID will always equal the incoming apiKey.ID.
Applied to files:
platform-api/src/internal/handler/gateway_internal.go
📚 Learning: 2025-12-18T08:33:51.340Z
Learnt from: O-sura
Repo: wso2/api-platform PR: 485
File: gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go:21-90
Timestamp: 2025-12-18T08:33:51.340Z
Learning: In gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go, the JSON tag casing differences between DefaultFaultEvent (correlationID, applicationID, organizationID, environmentID, regionID with uppercase 'ID') and DefaultResponseEvent (correlationId, applicationId, organizationId, environmentId, regionId with lowercase 'Id') are intentional to maintain consistency with respective event type conventions and config file property standards.
Applied to files:
platform-api/src/internal/model/gateway_event.go
🧬 Code graph analysis (8)
platform-api/src/internal/handler/gateway_internal.go (2)
platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
platform-api/src/internal/service/gateway_events.go (2)
platform-api/src/internal/model/gateway_event.go (1)
APIUndeploymentEvent(66-75)platform-api/src/internal/dto/gateway_event.go (1)
GatewayEventDTO(21-33)
platform-api/src/internal/server/server.go (2)
platform-api/src/internal/service/deployment.go (1)
NewDeploymentService(48-66)platform-api/src/internal/handler/deployment.go (1)
NewDeploymentHandler(38-42)
platform-api/src/internal/repository/api.go (1)
platform-api/src/internal/model/api.go (2)
APIDeployment(243-253)APIDeployment(256-258)
platform-api/src/internal/dto/api.go (1)
platform-api/src/internal/dto/common.go (1)
Pagination(23-27)
platform-api/src/internal/handler/deployment.go (4)
platform-api/src/internal/middleware/auth.go (1)
GetOrganizationFromContext(169-176)platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/dto/api.go (1)
DeployAPIRequest(166-170)platform-api/src/internal/constants/error.go (3)
ErrAPINotFound(47-47)ErrGatewayNotFound(62-62)ErrDeploymentNotFound(68-68)
platform-api/src/internal/repository/interfaces.go (1)
platform-api/src/internal/model/api.go (2)
APIDeployment(243-253)APIDeployment(256-258)
platform-api/src/internal/service/gateway_internal.go (1)
platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
🔇 Additional comments (24)
platform-api/src/internal/constants/error.go (1)
67-70: LGTM — deployment-specific errors are clearly defined.
Names/messages fit the existing error taxonomy and support new deployment flows.platform-api/src/config/config.go (2)
48-49: Deployment configuration is wired into the server config.
111-114: Default per-API gateway limit looks reasonable.platform-api/src/internal/model/gateway_event.go (1)
48-62: Deployment event payload aligns with the new deployment model.platform-api/src/internal/handler/gateway_internal.go (2)
20-32: Import update looks fine.
124-139: Active-deployment lookup and 404 mapping are appropriate.platform-api/src/internal/server/server.go (1)
104-105: Deployment service/handler wiring and route registration look correct.Also applies to: 115-116, 146-146
platform-api/src/internal/service/gateway_internal.go (1)
98-116: LGTM: active deployment lookupReturning
ErrDeploymentNotActiveon nil and using stored content keeps gateway retrieval clear and consistent.platform-api/src/internal/model/api.go (1)
242-252: Metadata persistence is already handled correctlyThe repository layer in
platform-api/src/internal/repository/api.goexplicitly marshalsMetadatato JSON string before insert and unmarshals it back to the map on retrieval. No runtime scan/insert failures will occur since JSON handling is implemented in the persistence layer, not relying ondatabase/sqlautomatic scanning.Likely an incorrect or invalid review comment.
platform-api/src/internal/handler/deployment.go (5)
34-42: Constructor wiring is clean.
Straightforward dependency injection.
44-98: Deploy handler validation and error mapping look solid.
277-312: List handler looks good.
357-368: Route registration is consistent.
100-142: Align redeploy handler with the published contract.
OpenAPI defines a requiredRedeployRequest(gatewayId/metadata), but this handler ignores the body and the service signature doesn't accept it; clients will send required data that gets dropped. Either remove/make the requestBody optional in the spec or bind and plumb it through. Also map the “deployment is already active” state to 400/409 instead of returning 500.⛔ Skipped due to learnings
Learnt from: thivindu Repo: wso2/api-platform PR: 110 File: platform-api/src/internal/service/api.go:432-476 Timestamp: 2025-11-08T13:06:22.133Z Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.platform-api/src/internal/repository/interfaces.go (1)
57-67: Deployment repository surface looks coherent.platform-api/src/internal/repository/api.go (4)
64-75: Column list updates are consistent across CRUD queries.Also applies to: 123-135, 179-200, 229-265, 365-374
1070-1106: GetDeploymentByID mapping and null handling look good.
1171-1248: Active/oldest deployment lookups are straightforward.
1408-1422: Deployment count helper looks good.platform-api/src/resources/openapi.yaml (3)
792-909: Deployment endpoints for create/list/get/delete are well documented.
3675-3693: Deployment request/response schemas look consistent.Also applies to: 3708-3768
3971-4010: New deployment parameters and tag are clear.platform-api/src/internal/service/deployment.go (2)
36-66: Service wiring looks good.
360-400: List/association helpers and handle-based variants look good.Also applies to: 454-480, 482-583
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/src/internal/service/gateway_internal.go (1)
273-281: Add Status and Content fields to deployment record before insert.The
APIDeploymentcreation at lines 273–281 omitsStatusandContent, both of which areNOT NULLin the schema with no repository-side defaults. Thecontentcolumn has no default value, so the INSERT will fail. Even if it succeeded,GetActiveDeploymentByGatewayexpects to readdeployment.Contentand would return empty data.Compare with the proper pattern in
deployment.go(lines 187–195), which setsStatus: "DEPLOYED"and populatesContentbefore callingCreateDeployment. Follow that same approach here:Suggested approach
// Create deployment record deployment := &model.APIDeployment{ + Status: "DEPLOYED", + Content: []byte(notification.Configuration), // or serialize as needed ApiID: apiUUID, GatewayID: gatewayID, OrganizationID: orgID, CreatedAt: now, }
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/dto/api.go`:
- Around line 172-188: Update the DTOs to match the OpenAPI contract: add an
OrganizationID string field to DeploymentResponse (json:"organizationId"
yaml:"organizationId"), and change DeploymentListResponse to expose Deployments
[]*DeploymentResponse `json:"deployments" yaml:"deployments"` instead of
Count/List/Pagination (i.e., remove Count and Pagination and rename List to
Deployments). Also update any code that constructs or consumes
DeploymentResponse and DeploymentListResponse (handlers/serializers) to populate
OrganizationID and to produce the Deployments slice (and stop expecting
Count/Pagination).
In `@platform-api/src/internal/service/deployment.go`:
- Around line 201-204: The code currently swallows failures from
s.ensureAPIGatewayAssociation(apiUUID, req.GatewayID, orgUUID) by logging a
warning; instead treat association failures as fatal: propagate the error (or
wrap it with context) and return it from the enclosing function so the
deployment is aborted/rolled back rather than reported as successful. Update the
surrounding control flow to return the error from the caller (or trigger the
existing rollback path) and include contextual information (apiUUID,
req.GatewayID, orgUUID) in the returned/wrapped error to aid debugging.
- Around line 575-583: GetDeploymentContentByHandle builds apiUUID via
getAPIUUIDByHandle but then calls GetDeploymentContent with the arguments
swapped; change the call in GetDeploymentContentByHandle to pass deploymentID,
apiUUID, orgUUID (instead of apiUUID, deploymentID, orgUUID) so
GetDeploymentContent receives (deploymentID, apiUUID, orgUUID) in the correct
order.
♻️ Duplicate comments (13)
platform-api/src/internal/database/schema.sql (1)
234-235: Add missing comma between UNIQUE and CHECK constraints.SQLite requires comma-separated table constraints. This will cause a syntax error when creating the table.
Fix
- UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid) - CHECK (status IN ('DEPLOYED', 'UNDEPLOYED')) + UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid), + CHECK (status IN ('DEPLOYED', 'UNDEPLOYED'))platform-api/src/internal/handler/deployment.go (4)
168-185: Align undeploy response and error mapping with the contract.
Still returns 204 with a JSON body and doesn’t map “not active” to a 4xx/409.
212-229: Return conflict for active deployments and avoid body on 204.
Delete should map “active deployment” errors to 409 and usec.Status(204)if keeping No Content.
256-271: Don’t mask internal errors as “not found.”
The 500 path currently returns “Deployment not found,” which is misleading.
338-355: Return after errors and use the correct content type.
The handler falls through after error JSON and serves YAML as JSON.platform-api/src/resources/openapi.yaml (3)
910-930: Redeploy request body doesn’t match implementation.
Spec requiresRedeployRequest, but redeploy currently accepts no body. Make the requestBody optional/remove it or update the handler/service to consume it.
946-966: Undeploy response mismatch.
Spec declares 200 +DeploymentResponse, but the implementation returns 204 No Content. Align the contract.
975-995: Deployment content media type/schema mismatch.
Implementation returns raw YAML bytes but spec declares JSON object. Update toapplication/x-yaml(or binary) and a string/binary schema.platform-api/src/internal/repository/api.go (3)
984-987: Handle metadata marshal failures.
json.Marshalerrors are ignored, so invalid metadata is silently dropped. Return the error to avoid storing corrupted/empty metadata.💡 Suggested fix
var metadataJSON string if deployment.Metadata != nil { - metadataBytes, _ := json.Marshal(deployment.Metadata) - metadataJSON = string(metadataBytes) + metadataBytes, mErr := json.Marshal(deployment.Metadata) + if mErr != nil { + return fmt.Errorf("failed to marshal deployment metadata: %w", mErr) + } + metadataJSON = string(metadataBytes) }
1114-1120: UseErrDeploymentNotFoundfor not-found cases.
String errors prevent callers from mapping to 404. Return the sentinel consistently.Also applies to: 1138-1145, 1159-1166
1327-1332: Filter deployment join to active deployments.
Joining all deployments can duplicate rows and markis_deployedtrue even when only undeployed artifacts exist. Filter bystatus='DEPLOYED'(and ideally select latest).platform-api/src/internal/service/deployment.go (2)
96-113: Enforce deployment limits after undeploying and fail fast on undeploy errors.
Current flow checks limits before undeploy and logs undeploy failures; this can exceedMaxPerAPIGatewayor leave multiple active deployments. Make undeploy failures fatal and trim until under the limit after the undeploy step.Also applies to: 164-168, 252-256
239-241: UseErrDeploymentNotFoundfor missing deployments.
Handlers map this sentinel to 404; string errors will surface as 500s.Also applies to: 304-306, 345-347, 418-420, 441-443
🧹 Nitpick comments (2)
platform-api/src/internal/service/gateway_events.go (1)
137-209: Consider extracting common broadcast logic to reduce duplication.The
BroadcastUndeploymentEventmethod duplicates nearly all ofBroadcastDeploymentEvent(correlation ID generation, serialization, size validation, connection lookup, broadcast loop, stats tracking). Consider extracting a generic helper:func (s *GatewayEventsService) broadcastEvent(gatewayID string, eventType string, payload interface{}) errorThis would reduce maintenance burden and ensure consistent behavior across event types.
platform-api/src/internal/database/schema.sql (1)
220-236: Consider adding indexes for common deployment query patterns.The
api_deploymentstable lacks indexes for common queries. Based on the deployment service operations mentioned in the AI summary, consider adding:CREATE INDEX IF NOT EXISTS idx_api_deployments_api ON api_deployments(api_uuid); CREATE INDEX IF NOT EXISTS idx_api_deployments_gateway_status ON api_deployments(gateway_uuid, status); CREATE INDEX IF NOT EXISTS idx_api_deployments_org ON api_deployments(organization_uuid);This would optimize queries for fetching deployments by API, finding active deployments by gateway, and listing deployments by organization.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
platform-api/src/config/config.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/dto/api.goplatform-api/src/internal/handler/api.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/model/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/api.goplatform-api/src/internal/service/deployment.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/utils/api.goplatform-api/src/resources/openapi.yaml
💤 Files with no reviewable changes (3)
- platform-api/src/internal/service/api.go
- platform-api/src/internal/utils/api.go
- platform-api/src/internal/handler/api.go
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/service/gateway_events.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/config/config.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/server/server.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/constants/error.goplatform-api/src/resources/openapi.yamlplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2025-12-06T11:57:08.456Z
Learnt from: BLasan
Repo: wso2/api-platform PR: 140
File: gateway/gateway-controller/pkg/utils/api_deployment.go:88-105
Timestamp: 2025-12-06T11:57:08.456Z
Learning: In the gateway-controller Go codebase (file: gateway/gateway-controller/pkg/utils/api_deployment.go), only the async/websub API kind is currently supported. async/websocket and async/sse API kinds are not yet implemented.
Applied to files:
platform-api/src/internal/service/gateway_events.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-15T11:14:32.797Z
Learnt from: VirajSalaka
Repo: wso2/api-platform PR: 686
File: platform-api/src/internal/service/api.go:920-922
Timestamp: 2026-01-15T11:14:32.797Z
Learning: In the platform-api service (file: platform-api/src/internal/service/api.go), API handles (the ID field) are immutable identifiers used as references and should not be updated after API creation. They serve as stable, user-facing identifiers for the API lifecycle.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-09T05:58:05.318Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/memory.go:484-503
Timestamp: 2026-01-09T05:58:05.318Z
Learning: In gateway/gateway-controller/pkg/storage/memory.go, during API key rotation via the StoreAPIKey method, the API key ID is never modified. When an existing API key is found by name (existingKeyID != ""), the incoming apiKey.ID is guaranteed to be the same as existingKeyID, so no ID mismatch can occur during updates.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/service/gateway_internal.go
📚 Learning: 2026-01-09T05:57:36.635Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/apikey_store.go:219-222
Timestamp: 2026-01-09T05:57:36.635Z
Learning: In gateway/gateway-controller/pkg/storage/apikey_store.go, during API key rotation, the API key ID and name remain constant—only the API key value changes. Therefore, when addToAPIMapping finds an existing key by name, the existingKeyID will always equal the incoming apiKey.ID.
Applied to files:
platform-api/src/internal/handler/gateway_internal.go
📚 Learning: 2025-12-18T08:33:51.340Z
Learnt from: O-sura
Repo: wso2/api-platform PR: 485
File: gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go:21-90
Timestamp: 2025-12-18T08:33:51.340Z
Learning: In gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go, the JSON tag casing differences between DefaultFaultEvent (correlationID, applicationID, organizationID, environmentID, regionID with uppercase 'ID') and DefaultResponseEvent (correlationId, applicationId, organizationId, environmentId, regionId with lowercase 'Id') are intentional to maintain consistency with respective event type conventions and config file property standards.
Applied to files:
platform-api/src/internal/model/gateway_event.go
📚 Learning: 2026-01-07T17:41:01.083Z
Learnt from: VirajSalaka
Repo: wso2/api-platform PR: 642
File: gateway/gateway-controller/pkg/utils/api_deployment.go:570-589
Timestamp: 2026-01-07T17:41:01.083Z
Learning: In gateway/gateway-controller/pkg/utils/api_deployment.go, when enableReplicaSync is true (multi-replica mode), the in-memory store (s.store) is intentionally not updated immediately during DeployAPIConfiguration. Instead, all replicas (including the originating replica) update their in-memory stores through the event-driven path via the EventListener to ensure consistent state synchronization across the cluster. This means immediate reads may not find the config until the EventListener processes the event, which is expected behavior.
Applied to files:
platform-api/src/internal/service/gateway_internal.go
📚 Learning: 2025-11-19T06:49:01.751Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 162
File: platform-api/src/resources/openapi.yaml:299-343
Timestamp: 2025-11-19T06:49:01.751Z
Learning: In OpenAPI 3.1 specification, the `contentType` field within the `encoding` object (used for multipart/form-data) can be a comma-separated list of media types. This is valid per the OpenAPI 3.1 Encoding Object specification and should not be flagged as an error.
Applied to files:
platform-api/src/resources/openapi.yaml
🧬 Code graph analysis (7)
platform-api/src/internal/service/gateway_events.go (4)
platform-api/src/internal/model/gateway_event.go (1)
APIUndeploymentEvent(66-75)platform-api/src/internal/dto/gateway_event.go (1)
GatewayEventDTO(21-33)gateway/gateway-controller/pkg/api/generated/generated.go (1)
Payload(95-95)platform-api/src/internal/websocket/stats.go (1)
DeliveryStats(31-51)
platform-api/src/internal/handler/gateway_internal.go (2)
platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
platform-api/src/internal/handler/deployment.go (6)
platform-api/src/internal/service/deployment.go (1)
DeploymentService(37-45)platform-api/src/internal/middleware/auth.go (1)
GetOrganizationFromContext(169-176)platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/dto/api.go (1)
DeployAPIRequest(166-170)platform-api/src/internal/constants/error.go (3)
ErrAPINotFound(47-47)ErrGatewayNotFound(62-62)ErrDeploymentNotFound(68-68)gateway/gateway-controller/pkg/api/generated/generated.go (3)
POST(227-227)GET(225-225)DELETE(224-224)
platform-api/src/internal/server/server.go (2)
platform-api/src/internal/service/deployment.go (1)
NewDeploymentService(48-66)platform-api/src/internal/handler/deployment.go (1)
NewDeploymentHandler(38-42)
platform-api/src/internal/service/gateway_internal.go (1)
platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
platform-api/src/internal/dto/api.go (1)
platform-api/src/internal/dto/common.go (1)
Pagination(23-27)
platform-api/src/internal/repository/api.go (1)
platform-api/src/internal/model/api.go (2)
APIDeployment(243-253)APIDeployment(256-258)
🔇 Additional comments (23)
platform-api/src/config/config.go (2)
48-49: LGTM!The new
Deploymentsfield follows the established pattern for nested configuration structs in this file, with proper envconfig tagging.
111-114: LGTM!The
Deploymentsstruct is well-defined with a sensible default value. The envconfig tag follows the established naming convention.platform-api/src/internal/model/gateway_event.go (1)
48-62: LGTM!The
APIDeploymentEventstruct changes align well with the PR's transition from revision-based to deployment-based architecture. TheDeploymentIDfield with itsdeploymentIdJSON tag maintains consistency with the existing naming conventions in this file.platform-api/src/internal/constants/error.go (1)
67-70: LGTM!The new deployment error constants follow the established naming convention and provide clear, descriptive error messages. Grouping them in a separate
varblock maintains consistency with other domain-specific error groups in this file.platform-api/src/internal/database/schema.sql (1)
222-234: Verify intent: UNIQUE constraint may be redundant with PRIMARY KEY.Since
deployment_idis already the PRIMARY KEY (globally unique), theUNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid)constraint provides no additional uniqueness enforcement—any combination includingdeployment_idwill already be unique.If the intent is to create a composite index for query performance, consider using an explicit index instead. If the intent is to enforce that each
(api_uuid, gateway_uuid)pair can only have one deployment, the constraint should excludedeployment_id:UNIQUE(api_uuid, gateway_uuid, organization_uuid)platform-api/src/internal/handler/gateway_internal.go (1)
124-138: Active‑deployment lookup path looks good.
The gateway-scoped lookup and explicit 404 for “no active deployment” are clean and consistent.platform-api/src/internal/server/server.go (1)
104-146: Deployment wiring is clean.
Service construction and handler registration align with the new deployment API surface.platform-api/src/internal/model/api.go (1)
242-252: > Likely an incorrect or invalid review comment.platform-api/src/resources/openapi.yaml (6)
792-826: Deploy endpoint definition looks consistent.
Request/response structure aligns with the new deployment flow.
828-855: Deployment list endpoint looks good.
856-908: Deployment get/delete endpoints look consistent.
3675-3768: Deployment schema additions look fine.
3971-3999: New deployment parameters are clear.
4009-4010: Tag update looks good.platform-api/src/internal/repository/interfaces.go (1)
57-67: APIRepository deployment interface additions look good.platform-api/src/internal/repository/api.go (2)
64-75: Expanded API queries and deployment count helper look consistent.Also applies to: 124-135, 180-200, 229-266, 366-374, 1408-1422
996-1068: Deployment retrieval helpers look good.Also applies to: 1070-1106, 1171-1208, 1210-1247
platform-api/src/internal/service/deployment.go (6)
36-65: DeploymentService wiring looks good.
68-95: Core deploy flow (validation/content generation/event broadcast) looks good.Also applies to: 114-153, 155-199, 206-229
232-238: Redeploy/undeploy/delete/read flows look consistent.Also applies to: 242-251, 259-295, 297-303, 307-335, 338-344, 348-358, 402-417, 421-432, 434-440, 444-451
360-399: Deployment listing DTO construction looks good.
454-479: Association helper logic looks fine.
482-573: Handle-based wrappers look consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Pull request overview
This pull request implements a new deployment feature that replaces the previous revision-based deployment system with an immutable deployment artifact architecture. The change enables better deployment lifecycle management with versioned, immutable snapshots of API configurations.
Changes:
- Introduces immutable deployment artifacts with lifecycle states (DEPLOYED/UNDEPLOYED)
- Removes API revision fields and endpoints in favor of deployment-centric operations
- Adds comprehensive deployment management endpoints (deploy, redeploy, undeploy, delete, retrieve content)
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| platform-api/src/resources/openapi.yaml | Replaces revision-based deployment endpoints with new artifact-based deployment API; adds deployment schemas and removes revision schemas |
| platform-api/src/internal/service/deployment.go | New service implementing complete deployment lifecycle with artifact creation, status management, and gateway event broadcasting |
| platform-api/src/internal/handler/deployment.go | New handler exposing deployment REST endpoints with validation and error handling |
| platform-api/src/internal/repository/api.go | Updates deployment repository methods to support artifact storage, filtering, and status management |
| platform-api/src/internal/repository/interfaces.go | Extends APIRepository interface with comprehensive deployment artifact methods |
| platform-api/src/internal/service/api.go | Removes revision deployment logic and related request structures |
| platform-api/src/internal/service/gateway_internal.go | Adds active deployment retrieval by gateway for runtime consumption |
| platform-api/src/internal/service/gateway_events.go | Adds undeployment event broadcasting capability |
| platform-api/src/internal/handler/api.go | Removes revision deployment endpoint handler |
| platform-api/src/internal/handler/gateway_internal.go | Updates GetAPI to fetch active deployment instead of current API state |
| platform-api/src/internal/utils/api.go | Removes revision-related fields from DTO/Model conversions |
| platform-api/src/internal/model/api.go | Updates API and APIDeployment models to remove revision fields and add artifact fields |
| platform-api/src/internal/model/gateway_event.go | Changes deployment event to reference deploymentId instead of revisionId |
| platform-api/src/internal/dto/api.go | Replaces APIRevisionDeployment with new deployment request/response DTOs |
| platform-api/src/internal/database/schema.sql | Removes revision columns from apis table; transforms api_deployments into artifact storage table |
| platform-api/src/internal/constants/error.go | Adds deployment-specific error constants |
| platform-api/src/internal/server/server.go | Registers new deployment service and handler |
| platform-api/src/config/config.go | Adds deployment configuration for artifact limits per API-gateway pair |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/service/deployment.go`:
- Around line 171-182: Only broadcast an undeployment event if there was an
active deployment for this API on that gateway: before constructing
APIUndeploymentEvent and calling
s.gatewayEventsService.BroadcastUndeploymentEvent(req.GatewayID, ...), query the
store for an active deployment for apiUUID and req.GatewayID (e.g. via an
existing method like s.deploymentStore.GetActiveDeployment or
s.store.GetActiveDeployment / HasActiveDeployment) and only proceed to create
the APIUndeploymentEvent and call BroadcastUndeploymentEvent when that query
confirms an active deployment exists.
♻️ Duplicate comments (9)
platform-api/src/internal/service/deployment.go (9)
96-113: Reorder limit enforcement and fail fast on undeploy errors.
Limit checks happen before undeploying the current deployment and delete at most one artifact; plus UpdateDeploymentStatus failures are only logged. This can exceedMaxPerAPIGatewayor leave multiple active deployments.🔧 Suggested tightening
- if apiDeploymentCount >= s.cfg.Deployments.MaxPerAPIGateway { - // Delete oldest deployment in UNDEPLOYED state to make room - oldestDeployment, err := s.apiRepo.GetOldestUndeployedDeploymentByGateway(apiUUID, req.GatewayID, orgUUID) - if err != nil { - return nil, fmt.Errorf("failed to get oldest undeployed deployment: %w", err) - } - if oldestDeployment != nil { - if err := s.apiRepo.DeleteDeployment(oldestDeployment.DeploymentID, apiUUID, orgUUID); err != nil { - return nil, fmt.Errorf("failed to delete oldest undeployed deployment: %w", err) - } - log.Printf("[INFO] Deleted oldest undeployed deployment %s to make room for new deployment", oldestDeployment.DeploymentID) - } - } + for apiDeploymentCount >= s.cfg.Deployments.MaxPerAPIGateway { + oldestDeployment, err := s.apiRepo.GetOldestUndeployedDeploymentByGateway(apiUUID, req.GatewayID, orgUUID) + if err != nil { + return nil, fmt.Errorf("failed to get oldest undeployed deployment: %w", err) + } + if oldestDeployment == nil { + return nil, fmt.Errorf("deployment limit reached for api %s on gateway %s", apiUUID, req.GatewayID) + } + if err := s.apiRepo.DeleteDeployment(oldestDeployment.DeploymentID, apiUUID, orgUUID); err != nil { + return nil, fmt.Errorf("failed to delete oldest undeployed deployment: %w", err) + } + apiDeploymentCount-- + } @@ - if existingDeployment != nil { - if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil { - log.Printf("[WARN] Failed to undeploy existing deployment %s: %v", existingDeployment.DeploymentID, err) - } - } + if existingDeployment != nil { + if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil { + return nil, fmt.Errorf("failed to undeploy existing deployment %s: %w", existingDeployment.DeploymentID, err) + } + }Also applies to: 164-168
142-148: Use ErrDeploymentNotFound for missing base deployments.
This keeps handler error mapping consistent.✅ Suggested fix
- if baseDeployment == nil { - return nil, errors.New("base deployment not found") - } + if baseDeployment == nil { + return nil, constants.ErrDeploymentNotFound + }
201-204: Treat API–gateway association failure as fatal.
Logging and continuing can leave a deployed artifact without an association.Based on learnings, deployments are expected to auto-create API–gateway associations.✅ Suggested fix
- if err := s.ensureAPIGatewayAssociation(apiUUID, req.GatewayID, orgUUID); err != nil { - log.Printf("[WARN] Failed to ensure API-gateway association: %v", err) - } + if err := s.ensureAPIGatewayAssociation(apiUUID, req.GatewayID, orgUUID); err != nil { + return nil, fmt.Errorf("failed to ensure API-gateway association: %w", err) + }
239-241: Return ErrDeploymentNotFound for missing deployments.✅ Suggested fix
- if deployment == nil { - return nil, errors.New("deployment not found") - } + if deployment == nil { + return nil, constants.ErrDeploymentNotFound + }
252-256: Undeploy failures should abort redeploy.
Ignoring this can leave multiple deployments marked active.✅ Suggested fix
- if existingDeployment != nil { - if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil { - log.Printf("[WARN] Failed to undeploy existing deployment %s: %v", existingDeployment.DeploymentID, err) - } - } + if existingDeployment != nil { + if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil { + return nil, fmt.Errorf("failed to undeploy existing deployment %s: %w", existingDeployment.DeploymentID, err) + } + }
304-309: Use ErrDeploymentNotFound / ErrDeploymentNotActive for consistent status mapping.
This aligns with handlers expecting sentinel errors.✅ Suggested fix
- if deployment == nil { - return errors.New("deployment not found") - } - if deployment.Status != "DEPLOYED" { - return errors.New("deployment is not currently active") - } + if deployment == nil { + return constants.ErrDeploymentNotFound + } + if deployment.Status != "DEPLOYED" { + return constants.ErrDeploymentNotActive + }
418-420: Use ErrDeploymentNotFound in GetDeployment.✅ Suggested fix
- if deployment == nil { - return nil, errors.New("deployment not found") - } + if deployment == nil { + return nil, constants.ErrDeploymentNotFound + }
441-443: Use ErrDeploymentNotFound in GetDeploymentContent.✅ Suggested fix
- if deployment == nil { - return nil, errors.New("deployment not found") - } + if deployment == nil { + return nil, constants.ErrDeploymentNotFound + }
575-584: Fix argument order in GetDeploymentContentByHandle.
The call currently swapsapiUUIDanddeploymentID, which will query the wrong content.✅ Suggested fix
- return s.GetDeploymentContent(deploymentID, apiUUID, orgUUID) + return s.GetDeploymentContent(apiUUID, deploymentID, orgUUID)
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/service/deployment.go`:
- Around line 265-270: The code is discarding the error returned from
s.gatewayRepo.GetByUUID(deployment.GatewayID); update the block in the function
handling deployment events so you capture the returned error (e.g., gateway, err
:= s.gatewayRepo.GetByUUID(...)) and handle it instead of ignoring it: if err !=
nil, log or return the error (or emit a gatewayEventsService error event) and
avoid proceeding with an empty vhost; only set vhost = gateway.Vhost when
gateway != nil and err == nil. Ensure you reference s.gatewayEventsService,
s.gatewayRepo.GetByUUID, and deployment.GatewayID when making the change.
- Around line 146-148: Replace the string error returned when baseDeployment is
nil with the shared constant to ensure consistent error handling: change the
nil-check return in the baseDeployment branch (the "baseDeployment == nil" case)
to return constants.ErrDeploymentNotFound (ensure the constants package is
imported if not already) so handlers map this to the correct 404 behavior
instead of a generic 500.
- Around line 317-322: The code currently discards the error from
s.gatewayRepo.GetByUUID which can hide failures and produce events with an empty
vhost; change the call to capture and handle the error (e.g., gateway, err :=
s.gatewayRepo.GetByUUID(deployment.GatewayID)), and if err != nil either return
the error from the current function or log it with context (including
deployment.GatewayID) before continuing, ensuring you only use gateway.Vhost
when err == nil and gateway != nil; mirror the handling used in
RedeployDeployment for consistency and ensure s.gatewayEventsService receives a
valid vhost or a clear error is propagated.
♻️ Duplicate comments (6)
platform-api/src/internal/service/deployment.go (6)
165-169: Treat undeploy failures as fatal to maintain deployment consistency.Ignoring the undeploy failure can leave multiple active deployments on the same gateway, violating the invariant of one active deployment per API per gateway.
💡 Suggested fix
if existingDeployment != nil { if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil { - log.Printf("[WARN] Failed to undeploy existing deployment %s: %v", existingDeployment.DeploymentID, err) + return nil, fmt.Errorf("failed to undeploy existing deployment %s: %w", existingDeployment.DeploymentID, err) } }
171-182: Guard undeployment event broadcast withexistingDeployment != nilcheck.Currently, an undeployment event is broadcast even when there's no active deployment to undeploy, causing spurious events to be sent to gateways.
💡 Suggested fix
- if s.gatewayEventsService != nil { + if existingDeployment != nil && s.gatewayEventsService != nil {
201-204: Treat API-gateway association failures as fatal or implement rollback.If the association creation fails after the deployment is persisted, the system ends up with a deployed artifact without a proper gateway association. Based on learnings, deployments are expected to auto-create associations, so failure here should abort the operation.
💡 Suggested fix
// Ensure API-Gateway association exists if err := s.ensureAPIGatewayAssociation(apiUUID, req.GatewayID, orgUUID); err != nil { - log.Printf("[WARN] Failed to ensure API-gateway association: %v", err) + // Rollback: delete the deployment we just created + if deleteErr := s.apiRepo.DeleteDeployment(deploymentID, apiUUID, orgUUID); deleteErr != nil { + log.Printf("[ERROR] Failed to rollback deployment %s after association failure: %v", deploymentID, deleteErr) + } + return nil, fmt.Errorf("failed to ensure API-gateway association: %w", err) }
253-256: Treat undeploy failures as fatal inRedeployDeployment.Same issue as in
DeployAPI- ignoring this failure can leave multiple active deployments.
239-244: Use sentinel errors for consistent HTTP status mapping.Both error cases use string errors. The "deployment not found" case should use
constants.ErrDeploymentNotFoundfor 404 mapping. Consider definingconstants.ErrDeploymentAlreadyActivefor the second case.💡 Suggested fix
if deployment == nil { - return nil, errors.New("deployment not found") + return nil, constants.ErrDeploymentNotFound } if deployment.Status == "DEPLOYED" { - return nil, errors.New("deployment is already active") + return nil, constants.ErrDeploymentAlreadyActive }
304-309: Use sentinel errors for consistent HTTP status mapping.Both error cases use string errors. Use
constants.ErrDeploymentNotFoundfor the nil check and consider definingconstants.ErrDeploymentNotActivefor the status check.💡 Suggested fix
if deployment == nil { - return errors.New("deployment not found") + return constants.ErrDeploymentNotFound } if deployment.Status != "DEPLOYED" { - return errors.New("deployment is not currently active") + return constants.ErrDeploymentNotActive }
🧹 Nitpick comments (1)
platform-api/src/internal/service/deployment.go (1)
391-399: Consider adding pagination support for scalability.The pagination values are hardcoded to return all results. For APIs with many deployments, this could become a performance concern. Consider accepting pagination parameters (offset, limit) and passing them to the repository.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/service/deployment.go`:
- Around line 242-244: Replace the ad-hoc errors.New calls used for deployment
state checks with the shared constants so handlers can map HTTP status codes via
errors.Is; specifically, in the branch that checks deployment.Status ==
"DEPLOYED" (the activation path) replace errors.New("deployment is already
active") with constants.ErrDeploymentIsDeployed, and in the branch that checks
for an inactive deployment (the deactivation/path around deployment.Status !=
"DEPLOYED") replace errors.New("deployment is not currently active") with
constants.ErrDeploymentNotActive; update the return statements in the functions
containing these checks to return those constant errors instead of new string
errors.
♻️ Duplicate comments (4)
platform-api/src/internal/service/deployment.go (4)
96-169: Enforce deployment limits after undeploy & fail fast on undeploy failures.The limit check runs before undeploying and only deletes one undeployed artifact; this can exceed
MaxPerAPIGateway. Also,UpdateDeploymentStatus(..., "UNDEPLOYED", ...)failures are logged and ignored, which can leave multiple active deployments.Also applies to: 246-257
171-182: Only emit undeploy events when an active deployment existed.Broadcasting an undeploy event without a prior active deployment can produce spurious gateway events.
201-204: Do not ignore API–gateway association failures.Swallowing association errors can leave a deployed artifact without a gateway association. Treat this as fatal (or rollback) so deployment and association stay consistent. Based on learnings, deployments are expected to auto-create missing associations.
265-270: Handle gateway lookup errors for events.The error from
GetByUUIDis discarded, which can emit events with an empty/incorrect vhost and mask underlying issues.Also applies to: 317-322
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/src/internal/repository/api.go (1)
1379-1418: Fix column scan order for deployment_id vs deployed_at.
The SELECT returnsad.deployment_idbeforead.created_at, but the Scan assignsdeployedAtfirst. This can cause scan type errors or swapped data.💡 Suggested fix
err := rows.Scan( &gateway.ID, &gateway.OrganizationID, &gateway.Name, &gateway.DisplayName, &gateway.Description, &gateway.Vhost, &gateway.IsCritical, &gateway.FunctionalityType, &gateway.IsActive, &gateway.CreatedAt, &gateway.UpdatedAt, &gateway.AssociatedAt, &gateway.AssociationUpdatedAt, &gateway.IsDeployed, - &deployedAt, - &gateway.DeploymentId, + &gateway.DeploymentId, + &deployedAt, )
♻️ Duplicate comments (2)
platform-api/src/internal/repository/api.go (2)
982-1013: Default deployment.Status to avoid CHECK violations.
If callers omitStatus, this INSERT bypasses the DB default and can fail the CHECK constraint. Consider guarding with a sensible default before the insert (e.g., DEPLOYED).💡 Suggested fix
// Generate UUID for deployment if not already set if deployment.DeploymentID == "" { deployment.DeploymentID = uuid.New().String() } + if deployment.Status == "" { + deployment.Status = model.DeploymentStatusDeployed + } deployment.CreatedAt = time.Now()
1211-1216: Make active deployment selection deterministic.
Multiple DEPLOYED rows can exist per (api_uuid, gateway_uuid). Without ordering,LIMIT 1is arbitrary.💡 Suggested fix
query := ` SELECT deployment_id, api_uuid, organization_uuid, gateway_uuid, status, base_deployment_id, content, metadata, created_at FROM api_deployments WHERE api_uuid = ? AND gateway_uuid = ? AND organization_uuid = ? AND status = ? + ORDER BY created_at DESC LIMIT 1 `
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/handler/deployment.go`:
- Around line 304-318: Add a guard before calling
h.deploymentService.GetDeploymentsByHandle to validate the status query
parameter: when status is non-empty, check it against the canonical list of
allowed deployment statuses (use the existing constant/enum set if available or
a small local allowed set) and if it is not one of the allowed values return a
400 response via c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad
Request", "Invalid status query parameter")) and return; place this check
immediately after reading gatewayId and status and before invoking
GetDeploymentsByHandle (referencing variables status, gatewayId, apiId, orgId
and the call h.deploymentService.GetDeploymentsByHandle).
♻️ Duplicate comments (1)
platform-api/src/internal/handler/deployment.go (1)
239-239: Avoid sending a body with 204 responses.
c.JSON(http.StatusNoContent, nil)emits anullbody; usec.Status(204)to keep the response truly empty.🔧 Suggested fix
- c.JSON(http.StatusNoContent, nil) + c.Status(http.StatusNoContent)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platform-api/src/resources/openapi.yaml`:
- Around line 843-855: The OpenAPI spec for the GetDeployments endpoint is
missing the 404 response that the GetDeployments handler returns when an API
handle doesn’t exist; update the endpoint's responses block in openapi.yaml to
add a '404' response (preferably referencing an existing
'#/components/responses/NotFound' or define one under components/responses if
missing) so the spec matches handler behavior and clients can rely on the
contract.
♻️ Duplicate comments (1)
platform-api/src/internal/handler/deployment.go (1)
309-313: Validatestatusquery param before calling the service.Spec already constrains
statustoDEPLOYED|UNDEPLOYED, but the handler forwards any value, which can yield a 500 for client mistakes. Add a small guard here to return 400 for invalid values.🔧 Suggested guard
// Get optional query parameters gatewayId := c.Query("gatewayId") status := c.Query("status") + if status != "" && status != "DEPLOYED" && status != "UNDEPLOYED" { + c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", + "Invalid status. Use DEPLOYED or UNDEPLOYED")) + return + } deployments, err := h.deploymentService.GetDeploymentsByHandle(apiId, gatewayId, status, orgId)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/service/deployment.go`:
- Around line 68-219: In DeployAPI, when using an existing deployment base
(req.Base != "current"), guard against GetDeploymentByID returning a nil
baseDeployment before dereferencing baseDeployment.Content: after calling
s.apiRepo.GetDeploymentByID(req.Base, apiUUID, orgUUID) check if baseDeployment
== nil and return constants.ErrBaseDeploymentNotFound (or wrap as appropriate)
instead of proceeding to read baseDeployment.Content; keep existing error
handling for non-nil err and set baseDeploymentID = &req.Base and contentBytes =
baseDeployment.Content only after the nil check.
dushaniw
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/repository/api.go`:
- Around line 998-999: The code unconditionally dereferences
deployment.BaseDeploymentID into baseDeploymentID which will panic if
BaseDeploymentID is nil; update the handling in api.go (the block setting
baseDeploymentID) to check if deployment.BaseDeploymentID != nil and only assign
*deployment.BaseDeploymentID in that case, otherwise leave baseDeploymentID as
nil (or assign a nil interface{}), so downstream code receives a safe nil
instead of causing a nil pointer dereference.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/database/schema.sqlite.sql`:
- Around line 349-351: Add a partial UNIQUE index on api_deployments to ensure
only one active deployment exists per (api_uuid, gateway_uuid); specifically
create a UNIQUE index (e.g., idx_api_deployments_unique_active) on columns
api_uuid, gateway_uuid with a WHERE clause limiting rows to status = 'DEPLOYED'
so concurrent deploy/redeploy cannot produce multiple DEPLOYED rows. Add this
SQL alongside the other index statements for api_deployments (keeping the
existing non-unique indexes if still needed) and ensure the index name and WHERE
predicate reference the api_deployments table and the status value 'DEPLOYED'.
In `@platform-api/src/internal/repository/api.go`:
- Around line 1161-1172: UpdateDeploymentStatus currently accepts any status
string which can cause cryptic DB errors; add application-layer validation of
the status parameter against the same allowed enum values used by
GetDeploymentsByAPIUUID before executing the UPDATE. In UpdateDeploymentStatus
validate the incoming status (e.g., compare to the set/list of allowed
deployment statuses used by GetDeploymentsByAPIUUID or centralize into a shared
helper/constant), and if invalid return a clear error (e.g., "invalid deployment
status: <status>") instead of attempting the DB Exec; keep the rest of the
function logic unchanged.
🧹 Nitpick comments (1)
platform-api/src/internal/repository/api.go (1)
1030-1065: Consider extracting duplicated SELECT columns.The SELECT column list is repeated 4 times across the query branches. This creates maintenance overhead when columns change.
♻️ Suggested refactor
+const deploymentSelectColumns = `deployment_id, api_uuid, organization_uuid, gateway_uuid, status, base_deployment_id, content, metadata, created_at` + func (r *APIRepo) GetDeploymentsByAPIUUID(apiUUID, orgUUID string, gatewayID, status *string) ([]*model.APIDeployment, error) { // ... validation ... - var query string + baseQuery := fmt.Sprintf(`SELECT %s FROM api_deployments WHERE api_uuid = ? AND organization_uuid = ?`, deploymentSelectColumns) var args []interface{} + args = append(args, apiUUID, orgUUID) - if gatewayID != nil && status != nil { - query = ` - SELECT deployment_id, api_uuid, organization_uuid, gateway_uuid, status, base_deployment_id, content, metadata, created_at - FROM api_deployments - WHERE api_uuid = ? AND organization_uuid = ? AND gateway_uuid = ? AND status = ? - ORDER BY created_at DESC - ` - args = []interface{}{apiUUID, orgUUID, *gatewayID, *status} - } else if gatewayID != nil { - // ... similar duplication ... + if gatewayID != nil { + baseQuery += ` AND gateway_uuid = ?` + args = append(args, *gatewayID) + } + if status != nil { + baseQuery += ` AND status = ?` + args = append(args, *status) } + baseQuery += ` ORDER BY created_at DESC`
3 participants
Purpose
This pull request implements a new deployment feature that replaces the old revision-based deployment system with an immutable deployment artifact model. The changes introduce deployment lifecycle management (deploy, redeploy, undeploy, delete) with per-gateway deployment limits and real-time event broadcasting to gateways.
Goals
The design is centered around a new core concept: the Deployment. This PR address the first milestone by adding management operations for deployments in platform-api side.
A Deployment is an immutable, self-contained artifact that represents a specific snapshot of an API, bundled with the gateway-specific configurations it was deployed with. Each time an API is deployed to a gateway, a new Deployment artifact is created.
Key Principles:
Changes:
User stories
Documentation
Has Doc Impact
Automation tests
In progress
In Progress
Security checks
Samples
N/A
Related PRs
N/A
Test environment
Postgresql
Summary by CodeRabbit
New Features
Enhancements
Removed
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.