Skip to content

fix: correct ARN parsing for notification targets #1010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
houseme merged 5 commits into from
Dec 6, 2025
Merged

fix: correct ARN parsing for notification targets #1010

houseme merged 5 commits into from
Dec 6, 2025

Conversation

@tennisleng
Copy link
Contributor

@tennisleng tennisleng commented Dec 5, 2025

Summary

This PR fixes an issue where bucket event notifications fail with a duplicated ARN prefix error.

Fixes #1007

Problem

When using minio mc to set bucket event notifications, the ARN validation fails with:

ARN not found:arn:rustfs:sqs:eu-central:arn:rustfs:sqs:eu-central:primary:webhook

Root Cause

In ecfs.rs and main.rs, TargetID::from_str was used to parse full ARN strings like arn:rustfs:sqs:eu-central:primary:webhook. However, TargetID::from_str expects the format ID:Name (e.g., primary:webhook).

This caused the TargetID to be incorrectly constructed with:

  • id = "arn"
  • name = "rustfs:sqs:eu-central:primary:webhook"

When the config was validated, calling target_id.to_arn() produced a duplicated ARN prefix.

Fix

Changed the parser to use ARN::parse() to properly parse the full ARN string and extract the correct TargetID with:

  • id = "primary"
  • name = "webhook"

Changes

  • rustfs/src/storage/ecfs.rs: Updated ARN parsing in put_bucket_notification_configuration
  • rustfs/src/main.rs: Updated ARN parsing in add_bucket_notification_configuration

@CLAassistant
Copy link

CLAassistant commented Dec 5, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@loverustfs
Copy link
Contributor

loverustfs commented Dec 6, 2025 •
edited
Loading

Hey @tennisleng ,

Thank you for your contribution! CLA signing has been added to the merge, without it, automated CI testing cannot be initiated.

@loverustfs
fix: correct ARN parsing for notification targets
98de3f1 
Previously TargetID::from_str was used to parse full ARN strings like
'arn:rustfs:sqs:eu-central:primary:webhook'. This caused the TargetID to
be incorrectly constructed with id='arn' and name='rustfs:sqs:...'.

When the config was validated, calling target_id.to_arn() produced a
duplicated ARN prefix:
'arn:rustfs:sqs:eu-central:arn:rustfs:sqs:eu-central:primary:webhook'

This fix uses ARN::parse() to properly parse the full ARN string and
extract the correct TargetID with id='primary' and name='webhook'.

Fixes rustfs#1007
@loverustfs loverustfs force-pushed the fix/notification-target-arn-parsing branch from 4d65931 to 98de3f1 Compare December 6, 2025 07:18
@loverustfs
Copy link
Contributor

loverustfs commented Dec 6, 2025 •
edited
Loading

Hey @tennisleng ,

Thank you for your contribution, but CI failed.
https://github.com/rustfs/rustfs/actions/runs/19985181187/job/57319734273?pr=1010

Run cargo fmt --all --check
Diff in /home/runner/work/rustfs/rustfs/rustfs/src/main.rs:516:
                     "Bucket '{}' has existing notification configuration: {:?}", bucket, cfg);
 
                 let mut event_rules = Vec::new();
-                process_queue_configurations(&mut event_rules, cfg.queue_configurations.clone(), |arn_str| ARN::parse(arn_str).map(|arn| arn.target_id).map_err(|e| TargetIDError::InvalidFormat(e.to_string())));
-                process_topic_configurations(&mut event_rules, cfg.topic_configurations.clone(), |arn_str| ARN::parse(arn_str).map(|arn| arn.target_id).map_err(|e| TargetIDError::InvalidFormat(e.to_string())));
-                process_lambda_configurations(&mut event_rules, cfg.lambda_function_configurations.clone(), |arn_str| ARN::parse(arn_str).map(|arn| arn.target_id).map_err(|e| TargetIDError::InvalidFormat(e.to_string())));
+                process_queue_configurations(&mut event_rules, cfg.queue_configurations.clone(), |arn_str| {
+                    ARN::parse(arn_str)
+                        .map(|arn| arn.target_id)
+                        .map_err(|e| TargetIDError::InvalidFormat(e.to_string()))
+                });
+                process_topic_configurations(&mut event_rules, cfg.topic_configurations.clone(), |arn_str| {
+                    ARN::parse(arn_str)
+                        .map(|arn| arn.target_id)
+                        .map_err(|e| TargetIDError::InvalidFormat(e.to_string()))
+                });
+                process_lambda_configurations(&mut event_rules, cfg.lambda_function_configurations.clone(), |arn_str| {
+                    ARN::parse(arn_str)
+                        .map(|arn| arn.target_id)
+                        .map_err(|e| TargetIDError::InvalidFormat(e.to_string()))
+                });
 
                 if let Err(e) = notifier_global::add_event_specific_rules(bucket, region, &event_rules)
                     .await
Diff in /home/runner/work/rustfs/rustfs/rustfs/src/storage/ecfs.rs:4890:
         let parse_rules = async {
             let mut event_rules = Vec::new();
 
-            process_queue_configurations(
-                &mut event_rules,
-                notification_configuration.queue_configurations.clone(),
-                |arn_str| ARN::parse(arn_str).map(|arn| arn.target_id).map_err(|e| TargetIDError::InvalidFormat(e.to_string())),
-            );
-            process_topic_configurations(
-                &mut event_rules,
-                notification_configuration.topic_configurations.clone(),
-                |arn_str| ARN::parse(arn_str).map(|arn| arn.target_id).map_err(|e| TargetIDError::InvalidFormat(e.to_string())),
-            );
+            process_queue_configurations(&mut event_rules, notification_configuration.queue_configurations.clone(), |arn_str| {
+                ARN::parse(arn_str)
+                    .map(|arn| arn.target_id)
+                    .map_err(|e| TargetIDError::InvalidFormat(e.to_string()))
+            });
+            process_topic_configurations(&mut event_rules, notification_configuration.topic_configurations.clone(), |arn_str| {
+                ARN::parse(arn_str)
+                    .map(|arn| arn.target_id)
+                    .map_err(|e| TargetIDError::InvalidFormat(e.to_string()))
+            });
             process_lambda_configurations(
                 &mut event_rules,
                 notification_configuration.lambda_function_configurations.clone(),
Diff in /home/runner/work/rustfs/rustfs/rustfs/src/storage/ecfs.rs:4906:
-                |arn_str| ARN::parse(arn_str).map(|arn| arn.target_id).map_err(|e| TargetIDError::InvalidFormat(e.to_string())),
+                |arn_str| {
+                    ARN::parse(arn_str)
+                        .map(|arn| arn.target_id)
+                        .map_err(|e| TargetIDError::InvalidFormat(e.to_string()))
+                },
             );
 
             event_rules
Error: Process completed with exit code 1.

@houseme houseme merged commit 5f25624 into rustfs:main Dec 6, 2025
12 checks passed
@loverustfs
Copy link
Contributor

loverustfs commented Dec 7, 2025

Hey @tennisleng ,

Thank you so much for your contribution! We have merged the code.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

wrong arn for notification target using minio mc

4 participants

@tennisleng @CLAassistant @loverustfs @houseme