feat(calm-hub): add GLOBAL_ADMIN-gated delete endpoints for all resource types - #3020
Merged
rocketstack-matt merged 10 commits intoAug 26, 2026
Merged
Conversation
willosborne
requested review from
Thels,
grahampacker-ms,
jpgough-ms,
markscott-ms and
rocketstack-matt
as code owners
August 25, 2026 10:41
- PERMISSIONS.md: GLOBAL admin capability table now lists content-resource deletion; note that it's deliberately GLOBAL-only (namespace/domain admin does not grant it) and that a control requirement with configurations refuses to delete. - ADR 0001: flag that the 'nothing is ever deleted' premise no longer holds now that deleteResource exists. - ADR 0003: correct deleteHeader's rationale, which claimed no delete endpoint existed for any type.
Architecture (Mongo + Nitrite): delete removes the whole resource including
all versions, and repeat/missing-id delete returns 404.
Control (Mongo + Nitrite): a fresh requirement + configuration exercises the
full cascade-refuse-then-succeed sequence — delete refuses with 409 while a
configuration exists, deleting the configuration first then lets the
requirement delete succeed. Also covers 404 for missing control/domain/
configuration.
Fixes a pre-existing Control integration test that assumed GET on
.../configurations/{id} (a path with no GET handler) fell through to 404 —
the new DELETE endpoint now claims that exact path, so JAX-RS correctly
reports 405 (path matched, method didn't) instead.
Architecture, Flow, Interface, Pattern, and Standard delete their resource but never cleaned up the matching resource_mappings entry, so recreating under the same custom-id threw DuplicateMappingException and the custom-id route kept resolving to a numeric ID that no longer existed. Adds ResourceMappingStore.deleteMappingByNumericId (a silent no-op when no mapping exists, since most resources are never given a custom ID) and wires it into all five delete endpoints via a small shared MappingCleanup helper: best-effort, logged rather than failing the response, since the resource is already gone by the time it runs. Found via PR review.
countHeaders then deleteResource isn't atomic, so a configuration created in the gap survives under a deleted requirement. Not fixed, consistent with the rest of the store layer using no transactions or locks anywhere: a Nitrite-only lock wouldn't extend the guarantee to Mongo, and the failure mode is a data-hygiene issue, not a correctness or security one. Raised in PR review.
NamespaceNotFoundException was the only caught exception, so a driver/DB failure from the mapping store (MongoException, a Nitrite lock/store error) would surface as an unhandled 500 for a delete that had already succeeded — contradicting the javadoc's own promise that cleanup failures are logged and swallowed. Found via PR review.
rocketstack-matt
requested changes
Aug 25, 2026
rocketstack-matt
left a comment
rocketstack-matt
left a comment
Member
There was a problem hiding this comment.
Solid, well-tested implementation of the delete pattern across every resource type. One real pre-merge defect: the five custom-id-mapped resource types (Architecture, Flow, Interface, Pattern, Standard) delete the resource but never clean up their ResourceMappingStore entry, so recreating under the same custom-id throws DuplicateMappingException. Also flagging a non-atomic check-then-act race in the control cascade-refuse logic for discussion, plus a non-blocking nit.
rocketstack-matt
left a comment
rocketstack-matt
left a comment
Member
There was a problem hiding this comment.
One edge-case finding below; everything else — the fixes for the earlier review round, the idiom audit, and a Copilot CLI pass — checks out.
rocketstack-matt
approved these changes
Aug 26, 2026
rocketstack-matt
left a comment
rocketstack-matt
left a comment
Member
There was a problem hiding this comment.
All prior review feedback addressed and verified — the mapping-cleanup runtime-exception fix in adde216 is correct and tested. Nothing outstanding.
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
2 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.
Description
Adds a
DELETEendpoint for every CALM Hub resource type that didn't already have one (Architecture, Pattern, Flow, Standard, Interface, Timeline, Adr, Decorator, Control Requirement, Control Configuration), gated behindGLOBAL_ADMIN.Delete semantics: delete removes the whole resource — its header and every version — not just the latest version or the header alone. The shared
MongoVersionDocumentStore/NitriteVersionDocumentStorehelpers gain a newdeleteResource(namespace, id)for this; it's distinct from the existingdeleteHeader, which is internal-only compensation logic used when a first version write fails and deliberately never touches the version collection.Control requirements caveat: a control requirement that still has configurations underneath it refuses to delete (
409 Conflict) rather than cascading. Delete the configurations first, then the requirement. Configurations themselves delete normally.Decorator isn't part of the versioned-document family (one doc per namespace, embedded array), so its delete is a bespoke pull/list-removal rather than a
deleteResourcecall.Audit logging picks up all the new endpoints automatically via the existing generic path-param resolution, except Control's two new methods, which needed one manual addition to
AuditRequestFilter's per-method dispatch (Control is the one resource that doesn't use the generic resolution path).Also updates
PERMISSIONS.md(content-resource deletion is deliberatelyGLOBAL admin-only, distinct from namespace/domain admin) and two ADRs whose "nothing is ever deleted" premise this change reverses, and adds Mongo + Nitrite integration test coverage for Architecture (representative simple case) and Control (the cascade-refuse case).Review notes
This PR is best reviewed commit by commit:
ControlHasConfigurationsException+ audit filter wiring + audit testType of Change
Affected Components
calm-hub/)Testing
Full suite passes:
../mvnw clean verify -Ddependency-check.skip=true— 2835 tests, 0 failures, JaCoCo coverage gate satisfied.../mvnw -P integration verifyalso passes in full (Docker/TestContainers). Also manually verified end-to-end against a running standalone instance: header+versions are genuinely gone from storage (not just the header), repeat delete returns 404, invalid namespace returns 400, audit log entries are recorded, and the control-requirement-with-configurations 409-refuse-then-succeed sequence behaves as intended.Checklist