Replace ambiguous PATCH /deployments/{id} with explicit action endpoints (#74) - #80
Merged
Merged
Conversation
…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.
Resolves the conflicts reported by PR camaraproject#80 after camaraproject#73 (PR camaraproject#79) was merged into main, which already carries camaraproject#72's fix (500/503 removal and local Generic400/403/404 responses) applied repo-wide. Resolved 9 conflicts in edge-application-management.yaml: - 7 identical 500/503-removal conflicts on operations unrelated to camaraproject#74 (submitApp, getApps, getApp, deleteApp, createAppInstance, createAppDeployment, deleteAppDeployment): kept main's side (500/503 already removed there). - deleteAppDeployment's trailing 500/503 vs the start of the new add-edge-cloud-zone path: removed the 500/503, kept the new path. - The large conflict spanning the four new deployment action endpoints (addEdgeCloudZone, removeEdgeCloudZone, addKubernetesCluster, removeKubernetesCluster) vs main's now-stale PATCH-based updateAppDeployment operation (which camaraproject#74 already replaced, but main still had since camaraproject#73/camaraproject#79 predates camaraproject#74's merge): kept HEAD's four endpoints, stripping their own 500/503 blocks for consistency with camaraproject#72's fix already present in main. The two .feature files that merged cleanly (getAppDeployments, getApps) carry over camaraproject#72's removal of phantom 404 scenarios on collection-list operations.
DLondonoD
added a commit
to DLondonoD/EdgeApplicationManagement
that referenced
this pull request
Sep 2, 2026
Brings in the resolution of PR camaraproject#80's conflicts (fix/issue-74 merged with upstream/main, which already carries camaraproject#72/camaraproject#73's fixes repo-wide). Kept fix/issue-75's 409 INCOMPATIBLE_STATE examples on the four deployment action endpoints, since fix/issue-74's freshly-merged version had them without the 500/503 blocks removed for camaraproject#72 consistency - identical to what was already resolved for camaraproject#79/camaraproject#80.
seralogar
reviewed
Sep 3, 2026
Co-authored-by: Sergi <sergialonsogarcia@gmail.com>
DLondonoD
added a commit
to DLondonoD/EdgeApplicationManagement
that referenced
this pull request
Sep 4, 2026
PR camaraproject#80 (issue camaraproject#74) merged to main with an additional commit applying @seralogar's review suggestion to simplify removeEdgeCloudZone's 404 response to $ref: Generic404, which conflicted with fix/issue-75's addition of the 409 INCOMPATIBLE_STATE response right after it. Resolved by keeping main's Generic404 simplification alongside fix/issue-75's 409 block. Also applied the same simplification to removeKubernetesCluster's 404, which had the same custom-message pattern but wasn't touched by the original suggestion, for consistency with the rest of the spec (addEdgeCloudZone/addKubernetesCluster already used Generic404).
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
3 participants
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