Skip to content

CLDSRV-1068: DeleteNotificationDestination API - #6369

Open
anurag4DSB wants to merge 2 commits into
improvement/CLDSRV-1067-destination-updatefrom
improvement/CLDSRV-1068-destination-delete
Open

anurag4DSB wants to merge 2 commits into
improvement/CLDSRV-1067-destination-updatefrom
improvement/CLDSRV-1068-destination-delete

Conversation

@anurag4DSB

@anurag4DSB anurag4DSB commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Spec: https://claude.ai/artifact/QhKsCqtGNzaVice35nvip9#bcc30d01-27db.m1yth80jczv.20396

Intent: why does this change exist?

  • An account needs to remove a destination that it no longer uses.

System impact: what's affected, including downstream?

  • With all five handlers in place, the switch no longer answers 501 for a missing handler.

Preserved behavior: what explicitly stays the same?

  • A Delete never removes a record of another account.
  • On file, a record removed by a parallel request still gets 204. The end state is the same.

Intended change: what's different after this PR?

  • Delete answers 204 with no body, or 404 for an unknown name.
  • Each route test now deletes the destinations that it created, in an afterEach, so a run leaves no record.

Verification: how do we know this worked, or how would we know if it didn't?

  • Unit tests run the handler on mem.
  • Functional route tests in s3c-ft-tests and file-ft-tests check Delete through Get and List.

https://claude.ai/code/session_01LSEjuApZSsJxDp6dw1FhVa

@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.32%. Comparing base (40e765b) to head (ebc0ad7).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
lib/api/api.js 93.89% <100.00%> (+0.01%) ⬆️
lib/api/notificationDestinationDelete.js 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@@                              Coverage Diff                               @@
##           improvement/CLDSRV-1067-destination-update    #6369      +/-   ##
==============================================================================
- Coverage                                       88.33%   88.32%   -0.02%     
==============================================================================
  Files                                             216      217       +1     
  Lines                                           14737    14769      +32     
==============================================================================
+ Hits                                            13018    13044      +26     
- Misses                                           1719     1725       +6     
Flag Coverage Δ
checksums-disabled-tests 35.10% <18.18%> (-0.04%) ⬇️
file-ft-tests 71.66% <81.81%> (-0.04%) ⬇️
file-ft-tests-null-compat 72.17% <81.81%> (+0.01%) ⬆️
kmip-ft-tests 27.95% <18.18%> (-0.02%) ⬇️
mongo-v0-ft-tests 70.51% <21.21%> (-0.17%) ⬇️
mongo-v1-ft-tests 70.53% <21.21%> (-0.08%) ⬇️
multiple-backend 35.81% <18.18%> (-0.04%) ⬇️
s3c-ft-tests-v0 66.99% <81.81%> (+0.05%) ⬆️
s3c-ft-tests-v0-null-compat 67.07% <81.81%> (+0.07%) ⬆️
s3c-ft-tests-v1 66.97% <81.81%> (+0.03%) ⬆️
sur-tests 36.35% <18.18%> (-0.91%) ⬇️
sur-tests-inflights 39.12% <18.18%> (-0.08%) ⬇️
unit 76.40% <100.00%> (+0.05%) ⬆️
utapi-v2-tests 35.05% <18.18%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 8ce9174 to e89e403 Compare November 10, 2026 14:37
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from 075fabd to 8098352 Compare November 10, 2026 14:37
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from e89e403 to cd38b93 Compare November 10, 2026 15:06
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from 8098352 to dd119c9 Compare November 10, 2026 15:06
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from cd38b93 to bdef8c0 Compare November 10, 2026 15:26
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from dd119c9 to 2131eea Compare November 10, 2026 15:28
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from bdef8c0 to b3f97de Compare November 12, 2026 09:52
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from 2131eea to d6975c3 Compare November 12, 2026 09:52
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from b3f97de to 24bbad3 Compare November 12, 2026 09:53
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from d6975c3 to cc238a5 Compare November 12, 2026 09:53
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 24bbad3 to 47625a9 Compare November 12, 2026 12:21
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from cc238a5 to ecda782 Compare November 12, 2026 12:21
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 47625a9 to 4c9d0b3 Compare November 12, 2026 13:06
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from ecda782 to 44a749a Compare November 12, 2026 13:06
@anurag4DSB
anurag4DSB marked this pull request as ready for review November 12, 2026 13:41
@anurag4DSB
anurag4DSB requested review from a team, nicolas2bert and tmacro November 12, 2026 13:41
@anurag4DSB
anurag4DSB marked this pull request as draft November 12, 2026 14:21
@anurag4DSB
anurag4DSB removed request for a team, nicolas2bert and tmacro November 12, 2026 14:21
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 4c9d0b3 to 6509d82 Compare November 12, 2026 14:40
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from 44a749a to efec70c Compare November 12, 2026 14:40
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 6509d82 to 8a14aef Compare November 12, 2026 15:03
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from efec70c to fecf01e Compare November 12, 2026 15:03
@anurag4DSB
anurag4DSB marked this pull request as ready for review November 12, 2026 15:19
@anurag4DSB
anurag4DSB requested a review from a team November 12, 2026 15:20
Add unit tests for the Delete handler on the in-memory backend, on a
stored record. They check:

- the record removed with no body, and Get then answering
  NoSuchDestination
- only the named record removed
- an IAM user deleting for its account
- an unknown name, a second Delete and another account's name: 404
  with no delete call, and the record kept
- a record gone between the read and the delete
- invalid names: 400 before any read
- store read and delete errors, returned unchanged
- no record value in the logs

Add functional tests through Create, Get and List. They check:

- Delete 204 with no body, then Get 404
- an unknown name and a second Delete: 404
- the name created again after a Delete
- List no longer showing a deleted name
- a bad name 400
- anonymous 403, with the record kept
- with a real Vault: an IAM user with no policy gets 403 on Get, List
  and Delete, with the record kept (Create and Update have blocks of
  their own)
- with a real Vault: a user allowed to delete one destination deletes
  it and gets 403 on another

When each test of the route tests ends, it deletes the destinations
that it created, with the credentials of their account. Each IAM test
also deletes its IAM user. Thus a run
leaves no record. The per-run name suffix stays, for runs that share a
server.

The api tests no longer say that the destination handlers do not exist
yet, and drop the case of a destination method with no handler.
notificationDestinationDelete:

- checks the name
- reads the record of the requester's account. NoSuchDestination
  stops the delete.
- deletes the record
- answers with no body, which the router sends as 204
- logs the name and the account only
- reports its call to the Prometheus metrics, as the bucket APIs do

Another request can remove the record between the read and the
delete. On mem and bucketd, the backend reports the missing key, so
the answer is NoSuchDestination. On file, the delete of a missing key
succeeds, so the answer is 204. The end state is the same.

All five destination handlers now exist. Thus callApiHandler no longer
answers NotImplemented to a destination method with no handler.
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 8a14aef to 40e765b Compare November 12, 2026 15:58
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1068-destination-delete branch from fecf01e to ebc0ad7 Compare November 12, 2026 15:58

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant