Repository navigation
fix(events): use notification subsystem for subscriptions - #259
Merged
Merged
Conversation
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.
Pull Request
Description
Expanding an event destination passed its service name (for example,
webhook) to an admin endpoint that expects the notification subsystem (notify_webhook), causing the subscription list to fail. Build the canonical subsystem path at the request boundary while preserving URL encoding, response data, and retry behavior.Type of Change
Testing
Validated commit
8802229. Request-level tests cover all nine built-in services, encoded destination names with a base path, and a failed request followed by a successful retry. All 635 tests passed on Node 22.22.0.Browser validation used the real Console page with a local mock API: the unmodified base sent
/target/webhook/primary/subscriptionsand displayed the loading error; this commit sent/target/notify_webhook/primary/subscriptionsand displayed the bucket, subscription ID, events, prefix, and suffix. Loading, empty results, an HTTP 500 followed by collapse/re-expand recovery, and a 390 × 844 viewport were also checked. These fixtures are UI evidence, not a live RustFS integration test.Checklist
Independent correctness/simplicity and tester/UX reviews found no defects in this diff.
Related Issues
Related to rustfs/rustfs#8356.
Screenshots (if applicable)
The following captures use the same local mock API fixtures, not a live RustFS server. Desktop captures use a 1280 × 720 viewport. Empty results, HTTP 500 handling, and collapse/re-expand recovery were also checked.
Mobile viewport (390 × 844), with the existing horizontally scrollable subscription table:
Additional Notes
Deploy the companion RustFS handler fix (rustfs/rustfs#8386) first: it accepts legacy service names and canonical subsystem names, and matches subscriptions using the service name in the ARN. This Console change alone cannot fix the ARN mismatch in the 1.0.1 server. Rollback is a revert of this commit; the companion backend continues to accept legacy Console requests.