Repository navigation
ARSN-671: route the notification-destination requests - #2744
Merged
bert-e merged 3 commits intoNov 4, 2026
Merged
Conversation
The bucket notification destination calls are served at the service root with the notification-destination query. With the routes option off or unset, every such request keeps today's answer. With it on, the five calls reach their API methods, and a bucket, a key, a website host, a missing name or OPTIONS get the answers of the plan. The tests of the new route fail on the base, because it does not exist yet.
Add the optional notificationDestinationRoutes field to the routes params. Only when a caller sets it to true, routes() sends a service-root request with the notification-destination query, on a REST endpoint and with any method but OPTIONS, to the new routeNotificationDestination. The existing route files do not change. The new route rejects a bucket or a key with MethodNotAllowed, so a misaddressed call never reaches a bucket call, and a missing name with InvalidArgument before authorization, so that the wildcard resource is only ever authorized for the list.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development/8.7 #2744 +/- ##
===================================================
+ Coverage 75.50% 75.59% +0.09%
===================================================
Files 226 227 +1
Lines 18641 18678 +37
Branches 3897 3909 +12
===================================================
+ Hits 14074 14120 +46
+ Misses 4562 4553 -9
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
anurag4DSB
marked this pull request as ready for review
November 3, 2026 12:03
tmacro
approved these changes
Nov 3, 2026
leif-scality
approved these changes
Nov 4, 2026
D1 Arsenal work ships as the new 8.7 minor line, so the package version on development/8.7 moves from 8.6.1 to 8.7.0.
Contributor
Author
|
Just added a package.json bump so not asking for re-review |
Contributor
Author
|
/approve |
Base automatically changed from
improvement/ARSN-668-bucketnotif-iam-actions
to
development/8.7
November 4, 2026 10:14
Contributor
Hello anurag4dsb,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
This pull request does not target the following hotfix branch(es) so they
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
Contributor
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue ARSN-671. Goodbye anurag4dsb. |
bert-e
deleted the
improvement/ARSN-671-route-notification-destination
branch
November 4, 2026 10:26
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
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.
Teaches Arsenal's request router about the five new tenant destination calls, so that CloudServer can serve them later.
Intent: why does this change exist?
CloudServer serves the five tenant destination calls at the service root, addressed by a
namequery parameter, and Arsenal's routing does not know them: today a service-root PUT, POST or DELETE gets 405, andGET /?notification-destinationlists buckets.System impact: what's affected, including downstream?
No behavior changes unless the caller of
routes()sets the new optionalnotificationDestinationRoutes: true, and nothing sets it outside the D1 epic branches. CloudServer will set it from its D1 flag, in the change that defines the five handlers, because itscallApiMethodcalls the handler with no existence check. The existing route files are untouched; the new behavior lives in its ownrouteNotificationDestination.ts.Preserved behavior: what explicitly stays the same?
With the field unset or false, every request keeps today's answer, which the tests check for the service root and for a bucket request with the query. With it on, OPTIONS keeps its CORS answer, website endpoints keep their own route, and a bare
GET /still lists the buckets.Intended change: what's different after this PR?
With the field on,
PUT|POST|GET|DELETE /?notification-destination&name=<name>reachnotificationDestinationCreate|Update|Get|Delete(200, 200, 200 with XML, 204), and a GET with no name reachesnotificationDestinationList. A request that also names a bucket or a key gets MethodNotAllowed, so a misaddressed call never reachesbucketDeleteor another bucket call; this holds while the decision on bucket-style host names stays open, and accepting them later breaks no client. A missing or empty name gets InvalidArgument before authorization, so the wildcard resource built without a name is authorized only for the list.Verification: how do we know this worked, or how would we know if it didn't?
New route tests fail on the test commit and pass with the code, including a production-mode test that a non-boolean value stays off; the full unit suite passes, as do tsc, lint and the prettier check. As on the base,
yarn testexits 1 after the open-handle warning ofrequestUtils.spec.js.