Skip to content

fix(events): use notification subsystem for subscriptions - #259

Merged
cxymds merged 1 commit into
mainfrom
cxymds/fix-event-destination-subscriptions
Oct 7, 2026
Merged

cxymds merged 1 commit into
mainfrom
cxymds/fix-event-destination-subscriptions

Conversation

@cxymds

@cxymds cxymds commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test improvements
  • Security fix

Testing

  • Unit tests added/updated
  • Manual testing completed

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.

nvm use v22
pnpm install --frozen-lockfile
pnpm type-check
pnpm lint
pnpm format:check
pnpm test:run
git diff --check

Browser validation used the real Console page with a local mock API: the unmodified base sent /target/webhook/primary/subscriptions and displayed the loading error; this commit sent /target/notify_webhook/primary/subscriptions and 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

  • Code follows the project's style guidelines
  • Self-review completed
  • TypeScript types are properly defined
  • All commit messages are in English (Conventional Commits)
  • All existing tests pass
  • No new dependencies added, or they are justified

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.

Before: legacy service path fails After: canonical subsystem path loads subscriptions
Before: event destination subscription loading error After: bucket, subscription, event, and filter fields loaded

Mobile viewport (390 × 844), with the existing horizontally scrollable subscription table:

Mobile event destination subscriptions

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.

@cxymds
cxymds merged commit cf11789 into main Oct 7, 2026
10 checks passed
@cxymds
cxymds deleted the cxymds/fix-event-destination-subscriptions branch October 7, 2026 07:59
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