Replace ambiguous PATCH /deployments/{id} with explicit action endpoints (#74) - #80
Open
DLondonoD wants to merge 6 commits into
Open
Replace ambiguous PATCH /deployments/{id} with explicit action endpoints (#74)#80DLondonoD wants to merge 6 commits into
DLondonoD wants to merge 6 commits into
Conversation
…araproject#73 Per issue camaraproject#73's analysis of both list endpoints against the CAMARA Design Guide (including the pagination guide at Commonalities/documentation/CAMARA-API-Design-Guide.md#41-pagination): Fix (mandatory per issue): - ClusterInfo.provider renamed to edgeCloudProvider, now referencing EdgeCloudProvider instead of the incorrect AppProvider (Application Provider != Edge Cloud Provider running the cluster). - Removed 500/503 from the endpoint, and restricted 400/403 to only INVALID_ARGUMENT/PERMISSION_DENIED via new local Generic400/403 response objects, per the rationale already applied for issue camaraproject#72 (5xx not documented by default; OUT_OF_RANGE/INVALID_TOKEN_CONTEXT irrelevant to this API). Consider (adopted): - Combined GET /clusters into GET /edge-cloud-zones: removed the standalone /clusters path, getClusters operation, its dedicated security scope and the now-unused 'Cluster' tag. EdgeCloudZone gains an optional 'clusters' array (ClusterInfo items, without the redundant edgeCloudZoneId/edgeCloudRegion fields already provided by the enclosing zone), populated when the zone has Kubernetes clusters available. - Added pagination support per the Commonalities pagination guide: page/perPage query params (ref'd from CAMARA_common.yaml), x-total-count/x-total-pages/link response headers, and a new EdgeCloudZoneList response schema (replacing the bare EdgeCloudZones array) wrapping items + the common Pagination object. - Added a new CountryCode schema (ISO 3166-1 alpha-2) and countryCode query parameter, replacing the previous edge-cloud-provider-specific 'region' filter with a standardized, interoperable one. EdgeCloudZone gains a required 'countryCode' property; the existing free-text edgeCloudRegion field is kept as provider-specific descriptive info. - Added an edgeCloudProvider query parameter to filter zones by provider. - Removed the 'default: unknown' from the status query parameter (now defined inline instead of -ing EdgeCloudZoneStatus, whose default remains appropriate for the response object) so omitting the filter returns zones of any status instead of defaulting to 'unknown' only. Updated getEdgeCloudZones.feature accordingly (new schema references, countryCode/edgeCloudProvider/pagination scenarios, removed the region-filter scenario, added a no-default-status scenario and a 400 invalid-page scenario) and deleted getClusters.feature, since the operation no longer exists. Fixes camaraproject#73
Per the Commonalities pagination guide, the 'items' array name in the paginated response schema is API-specific, not mandatory - renamed EdgeCloudZoneList.items to edgeCloudZones for clarity in this API's context. Updated the .feature file's property references accordingly. Also restored the ability to filter by edgeCloudRegion, lost when 'region' was replaced by the new standardized countryCode filter. The EdgeCloudZone schema still exposes edgeCloudRegion as a provider-specific descriptive field, so it makes sense to keep it filterable alongside countryCode: countryCode for provider-independent results, edgeCloudRegion when finer, provider-specific granularity is needed. Added the corresponding query parameter, operation/API description updates, and a getEdgeCloudZones.feature scenario.
…nts (camaraproject#74) The PATCH-based updateAppDeployment operation obscured the actual API design requirement (adding/removing specific Edge Cloud Zones and Kubernetes clusters from an existing deployment) by treating it as a generic database entry update: - Ambiguous behavior around array replacement vs merging for edgeCloudZones/kubernetesClusterRefs, needing an explanatory note that JSON Merge Patch replaces rather than merges arrays - itself a sign PATCH wasn't intuitive here. - Functionality creep: PATCH incidentally allowed updating appDeploymentName, which was never a stated design requirement. Removed the patch: operation on /deployments/{appDeploymentId} and replaced it with four explicit, unambiguous action endpoints, as suggested in the issue: - POST /deployments/{appDeploymentId}/addEdgeCloudZone - POST /deployments/{appDeploymentId}/removeEdgeCloudZone - POST /deployments/{appDeploymentId}/addKubernetesCluster - POST /deployments/{appDeploymentId}/removeKubernetesCluster Each takes a single-field request body (edgeCloudZoneId or kubernetesClusterRef) and returns the updated AppDeploymentInfo on 200, with 409 ALREADY_EXISTS for add operations targeting an already-present zone/cluster and 404 NOT_FOUND for remove operations targeting one not present in the deployment. appDeploymentName is no longer updatable through any operation, removing the functionality creep entirely. Replaced updateAppDeployment.feature with one .feature file per new operation, and updated the API description's Quick Start section. Fixes camaraproject#74
addKubernetesCluster and removeKubernetesCluster returned AppDeploymentInfo without any way to verify their effect in the response, unlike addEdgeCloudZone/removeEdgeCloudZone which can be checked against the edgeCloudZones array. Added an optional kubernetesClusterRefs array property (not required, since a deployment may not use any specific Kubernetes cluster) mirroring the one already accepted by createAppDeployment's request body. Updated addKubernetesCluster.feature and removeKubernetesCluster.feature success scenarios to assert the cluster is present/absent in the response accordingly.
DLondonoD
requested review from
FabrizioMoggio,
JoseMConde,
Kevsy,
gainsley,
maheshc01 and
seralogar
as code owners
August 7, 2026 10:43
…roject#80) - [S-008] x4: the 4 new deployment action paths introduced in a9583a1 used camelCase path segments (addEdgeCloudZone, removeEdgeCloudZone, addKubernetesCluster, removeKubernetesCluster), violating the mandatory kebab-case path convention. Renamed to add-edge-cloud-zone, remove-edge-cloud-zone, add-kubernetes-cluster and remove-kubernetes-cluster respectively, consistent with the rest of the spec's paths (/app-instances, /edge-cloud-zones, etc.). operationIds are unaffected (S-008 only applies to paths). Updated the 4 corresponding .feature files' Background resource to match. - [S-011]: EdgeCloudZoneList.edgeCloudZones (introduced in f0dbe25) was missing a description. Added one. The remaining 10 [S-313] findings (1 warning + 9 notices) are pre-existing free-form/implementation-dependent string fields already covered by the rationale documented for issue camaraproject#46/camaraproject#65 (names, versions, opaque strings); no action needed.
…5351424, PR camaraproject#80) The Generic400/403 response objects introduced locally for camaraproject#72 (restricting the documented error codes, e.g. dropping OUT_OF_RANGE and INVALID_TOKEN_CONTEXT) never made it past the fix/issue-72 branch into fix/issue-73/74/75, which were branched from main instead. Only the redesigned getEdgeCloudZones operation (added in fix/issue-73) ended up referencing the local Generic400/403, while every other operation still $ref'd the common ones - two different-content components sharing the same name in the same bundled document, which the validator flags as a P-040 collision. Consistently applied the camaraproject#72 fix repo-wide instead: - Added a local Generic404 (NOT_FOUND only, dropping IDENTIFIER_NOT_FOUND per the same camaraproject#72 rationale), which was missing. - Replaced all remaining external $ref's to CAMARA_common.yaml's Generic400/403/404 - including the two notification callbacks - with the local ones, so each name now resolves to exactly one component throughout the bundled document. Generic401 is untouched, since no local override exists for it and no collision was ever reported.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
correction
What this PR does / why we need it:
Replaces the ambiguous
PATCH /deployments/{appDeploymentId}(updateAppDeployment) with explicit, unambiguousaction endpoints:
edgeCloudZones/kubernetesClusterRefs, and the need for an explanatory note that JSON Merge Patch replaces ratherthan merges these arrays was itself a sign PATCH wasn't intuitive for this use case.
appDeploymentName, which was never a stated designrequirement.
Removed the
patch:operation and added four explicit endpoints instead:POST /deployments/{appDeploymentId}/addEdgeCloudZonePOST /deployments/{appDeploymentId}/removeEdgeCloudZonePOST /deployments/{appDeploymentId}/addKubernetesClusterPOST /deployments/{appDeploymentId}/removeKubernetesClusterEach takes a single-field request body (
edgeCloudZoneIdorkubernetesClusterRef) and returns the updatedAppDeploymentInfoon200, with409 ALREADY_EXISTSforaddoperations targeting an already-present zone/clusterand
404 NOT_FOUNDforremoveoperations targeting one not present in the deployment.appDeploymentNameis nolonger updatable through any operation.
Also added an optional
kubernetesClusterRefsarray toAppDeploymentInfo(mirroring the one already accepted bycreateAppDeployment's request body), so the new cluster endpoints' effect can be verified in their own response,consistent with how the zone endpoints can be checked against
edgeCloudZones.Which issue(s) this PR fixes:
Fixes #74
Special notes for reviewers:
updateAppDeployment.featurewas replaced by one.featurefile per new operation (addEdgeCloudZone,removeEdgeCloudZone,addKubernetesCluster,removeKubernetesCluster). The API description's Quick Start section wasupdated accordingly. This PR is branched off
fix/issue-73(still pending merge), since both touch thedeployments/edge-cloud-zones area of the spec.
Changelog input
release-note Replace PATCH /deployments/{appDeploymentId} with explicit
addEdgeCloudZone/removeEdgeCloudZone/addKubernetesCluster/removeKubernetesCluster operations, removing ambiguous array-
replacement semantics and unintended appDeploymentName updates.
Additional documentation
This section can be blank.
docs