Skip to content

CLDSRV-1067: UpdateNotificationDestination API - #6368

Open
anurag4DSB wants to merge 2 commits into
improvement/CLDSRV-1066-destination-listfrom
improvement/CLDSRV-1067-destination-update
Open

anurag4DSB wants to merge 2 commits into
improvement/CLDSRV-1066-destination-listfrom
improvement/CLDSRV-1067-destination-update

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 change the brokers, topic or auth of a destination without a delete.

System impact: what's affected, including downstream?

  • Update uses the body checker of Create, so both apply the same rules.

Preserved behavior: what explicitly stays the same?

  • Name, AccountId, Arn and CreatedAt keep their stored values.
  • A record of a newer version stays untouched, because this server would drop its new fields.

Intended change: what's different after this PR?

  • Update replaces the whole record from the body and sets a new UpdatedAt.

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 the change through Get.

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.33%. Comparing base (19b6121) to head (40e765b).
✅ 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.87% <100.00%> (+0.01%) ⬆️
lib/api/notificationDestinationUpdate.js 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@@                             Coverage Diff                              @@
##           improvement/CLDSRV-1066-destination-list    #6368      +/-   ##
============================================================================
+ Coverage                                     88.28%   88.33%   +0.05%     
============================================================================
  Files                                           215      216       +1     
  Lines                                         14689    14737      +48     
============================================================================
+ Hits                                          12968    13018      +50     
+ Misses                                         1721     1719       -2     
Flag Coverage Δ
checksums-disabled-tests 35.13% <18.75%> (-0.06%) ⬇️
file-ft-tests 71.69% <79.16%> (+0.03%) ⬆️
file-ft-tests-null-compat 72.15% <79.16%> (+0.41%) ⬆️
kmip-ft-tests 27.97% <18.75%> (-0.04%) ⬇️
mongo-v0-ft-tests 70.67% <18.75%> (-0.13%) ⬇️
mongo-v1-ft-tests 70.61% <18.75%> (-0.17%) ⬇️
multiple-backend 35.85% <18.75%> (-0.06%) ⬇️
s3c-ft-tests-v0 66.94% <79.16%> (+0.03%) ⬆️
s3c-ft-tests-v0-null-compat 67.00% <79.16%> (+0.02%) ⬆️
s3c-ft-tests-v1 66.94% <79.16%> (+0.05%) ⬆️
sur-tests 37.25% <18.75%> (+0.80%) ⬆️
sur-tests-inflights 39.20% <18.75%> (-0.04%) ⬇️
unit 76.35% <100.00%> (+0.07%) ⬆️
utapi-v2-tests 35.08% <18.75%> (-0.06%) ⬇️

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-1066-destination-list branch from f7de0cf to 1251db1 Compare November 10, 2026 15:06
@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-1066-destination-list branch from 1251db1 to db126da Compare November 10, 2026 15:24
@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-1066-destination-list branch from db126da to 7d0c72c Compare November 12, 2026 09:52
@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-1066-destination-list branch from 7d0c72c to 052353d Compare November 12, 2026 09:53
@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-1066-destination-list branch from 052353d to 59ea255 Compare November 12, 2026 12:21
@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-1066-destination-list branch from 59ea255 to 15c33ed Compare November 12, 2026 13:06
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 47625a9 to 4c9d0b3 Compare November 12, 2026 13:06
@anurag4DSB
anurag4DSB marked this pull request as ready for review November 12, 2026 13:40
@anurag4DSB
anurag4DSB requested review from a team, nicolas2bert and tmacro November 12, 2026 13:40
@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-1066-destination-list branch from 15c33ed to 9839ab7 Compare November 12, 2026 14:40
@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-1066-destination-list branch from 9839ab7 to 1aa7df3 Compare November 12, 2026 15:03
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 6509d82 to 8a14aef Compare November 12, 2026 15:03
@anurag4DSB
anurag4DSB marked this pull request as ready for review November 12, 2026 15:19
@anurag4DSB
anurag4DSB requested review from a team, nicolas2bert and tmacro November 12, 2026 15:20
Add unit tests for the Update handler on the in-memory backend, on a
record stored with an old CreatedAt. They check:

- the body replaces the whole record: new brokers and topic, defaults
  applied, and the old Sasl block gone when the body switches to mtls
- identity fields and CreatedAt kept as stored, and a new UpdatedAt
- a version 1 record written again as version 1
- a record of a newer version refused with InvalidDestination, and
  left untouched
- an IAM user
- an unknown name and another account's name: 404 with no write
- a bad name, checked before the body
- MalformedXML and InvalidDestination (GSSAPI included), with the
  record kept
- the body, checked before the read
- store errors
- no record value in the logs

Add functional tests through Create and Get. They check:

- Update 200, then Get shows the new brokers and topic, the old
  CreatedAt and a later UpdatedAt
- an unknown name 404
- an invalid body 400, with the record unchanged
- a bad name 400
- anonymous 403
- with a real Vault: an IAM user with no policy gets 403, while a user
  allowed on one destination updates it and gets 403 on another

Each IAM test makes its own IAM user and destinations, and removes the
user when it ends.
notificationDestinationUpdate:

- checks the name, then checks the body with the checker that Create
  uses
- reads the record of the requester's account. NoSuchDestination
  stops the update.
- writes the whole record again: Type, Connection and Auth from the
  body, Name, AccountId, Arn and CreatedAt as stored, UpdatedAt now,
  and the record version that this server writes
- leaves a record of a newer version untouched, because it may hold
  fields that this server would drop. That record gets
  InvalidDestination, which says to update it from a server that runs
  the newer version.
- answers 200 with no body, and logs the name and the account only
- reports its call to the Prometheus metrics, as the bucket APIs do

Update writes the record version of the shared request helpers, as
Create does.
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1066-destination-list branch from 1aa7df3 to 19b6121 Compare November 12, 2026 15:58
@anurag4DSB
anurag4DSB force-pushed the improvement/CLDSRV-1067-destination-update branch from 8a14aef to 40e765b 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