Skip to content

fix(event): normalize shorthand notification event names#66

Closed
overtrue wants to merge 1 commit intomainfrom
fix/issue-65-event-put
Closed

fix(event): normalize shorthand notification event names#66
overtrue wants to merge 1 commit intomainfrom
fix/issue-65-event-put

Conversation

@overtrue
Copy link
Contributor

Summary

Fixes rc event add --event put storing shorthand names literally instead of normalizing to S3 event patterns.

Closes #65.

Problem

parse_event_list() passed --event values through unchanged. As a result, shorthand values such as put were persisted as-is in bucket notification configuration and never matched incoming RustFS events like s3:ObjectCreated:Put.

Root Cause

Event normalization for shorthand names was missing in crates/cli/src/commands/event.rs.

Solution

  • Added normalize_event_name() in the event command parser.
  • Normalized shorthand values (case-insensitive):
    • put -> s3:ObjectCreated:*
    • get -> s3:ObjectAccessed:*
    • delete -> s3:ObjectRemoved:*
    • replica -> s3:Replication:*
    • ilm -> s3:ObjectTransition:*
  • Kept non-shorthand values unchanged.
  • Added unit test test_parse_event_list_normalizes_shorthand_events to lock behavior and dedup semantics.

Validation

Docker Reproduction (before fix)

  • Started rustfs/rustfs:latest in Docker.
  • Ran:
rc alias set local http://localhost:19000 accesskey secretkey
rc mb local/test-bucket
rc event add local/test-bucket arn:rustfs:sqs::primary:mqtt --event put
rc event list local/test-bucket
  • Observed (before fix):
[queue] arn:rustfs:sqs::primary:mqtt -> put

Docker Verification (after fix)

  • Ran same command sequence against rustfs/rustfs:latest.
  • Observed (after fix):
[queue] arn:rustfs:sqs::primary:mqtt -> s3:ObjectCreated:*

Quality Gates

cargo fmt --all --check
cargo clippy --workspace -- -D warnings
cargo test --workspace

All passed.

Copilot AI review requested due to automatic review settings March 23, 2026 19:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes rc event add --event <shorthand> persisting literal shorthand values by normalizing supported shorthands into S3 notification event patterns, ensuring saved rules match incoming RustFS events (closes #65).

Changes:

  • Normalize shorthand event names (put/get/delete/replica/ilm, case-insensitive) in parse_event_list().
  • Add a focused unit test to lock in shorthand normalization + dedup/sort behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +362 to +368
match value.to_ascii_lowercase().as_str() {
"put" => "s3:ObjectCreated:*".to_string(),
"get" => "s3:ObjectAccessed:*".to_string(),
"delete" => "s3:ObjectRemoved:*".to_string(),
"replica" => "s3:Replication:*".to_string(),
"ilm" => "s3:ObjectTransition:*".to_string(),
_ => value.to_string(),
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalize_event_name() allocates a lowercase String for matching (to_ascii_lowercase) and then allocates again for the returned String (even in the non-shorthand _ case). This does unnecessary work for a CLI parse path; consider avoiding the temporary allocation by matching with eq_ignore_ascii_case (or matching on ASCII bytes) and only allocating the output once.

Suggested change
match value.to_ascii_lowercase().as_str() {
"put" => "s3:ObjectCreated:*".to_string(),
"get" => "s3:ObjectAccessed:*".to_string(),
"delete" => "s3:ObjectRemoved:*".to_string(),
"replica" => "s3:Replication:*".to_string(),
"ilm" => "s3:ObjectTransition:*".to_string(),
_ => value.to_string(),
if value.eq_ignore_ascii_case("put") {
"s3:ObjectCreated:*".to_string()
} else if value.eq_ignore_ascii_case("get") {
"s3:ObjectAccessed:*".to_string()
} else if value.eq_ignore_ascii_case("delete") {
"s3:ObjectRemoved:*".to_string()
} else if value.eq_ignore_ascii_case("replica") {
"s3:Replication:*".to_string()
} else if value.eq_ignore_ascii_case("ilm") {
"s3:ObjectTransition:*".to_string()
} else {
value.to_string()

Copilot uses AI. Check for mistakes.
@overtrue
Copy link
Contributor Author

Closing as duplicate of #67, which contains the same fix with slightly broader test coverage.

@overtrue overtrue closed this Mar 23, 2026
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.

rc event add --event put registers subscription but events never fire

2 participants