From 23a3c242fa652a08d42bf70316b6eaa959eb4b65 Mon Sep 17 00:00:00 2001 From: testsabirweb <53115363+testsabirweb@users.noreply.github.com> Date: Wed, 4 Mar 2026 20:41:01 +0530 Subject: [PATCH] Add test coverage and fix validation for MRAP ARN bucket names (#9554) * Issue #9544: Add test coverage and fix validation for MRAP ARN bucket names S3 Multi-Region Access Point (MRAP) ARNs have the format: arn:aws:s3::{account-id}:accesspoint/{mrap-alias}.mrap These ARNs contain a '/' as part of the ARN path, which caused Velero's BSL bucket validation to reject them with an error asking the user to put the value in the Prefix field instead. Fix the bucket name validation in objectBackupStoreGetter.Get() to exempt ARNs (identified by the "arn:" prefix) from the slash check, since slashes are a valid and required part of ARN syntax. Add unit tests in object_store_mrap_test.go covering: - A plain MRAP ARN as bucket name succeeds - A MRAP ARN with a trailing slash is trimmed and accepted Signed-off-by: Sabir Ali Co-authored-by: Cursor * Address review comments: fix changelog filename and import grouping Signed-off-by: Sabir Ali Co-authored-by: Cursor * Restrict MRAP ARN bucket validation to arn:aws:s3: prefix Per review, use HasPrefix(bucket, "arn:aws:s3:") instead of HasPrefix(bucket, "arn:") so only S3 ARNs (e.g. MRAP) are exempt from the slash check, not any ARN from other AWS services. Signed-off-by: Sabir Ali Co-authored-by: Cursor * Move MRAP bucket tests into TestNewObjectBackupStoreGetter Consolidate MRAP ARN test cases into the existing table in object_store_test.go and remove object_store_mrap_test.go. Signed-off-by: Sabir Ali Co-authored-by: Cursor --------- Signed-off-by: Sabir Ali Signed-off-by: Sabir Ali Co-authored-by: Cursor --- changelogs/unreleased/9554-testsabirweb | 1 + pkg/persistence/object_store.go | 3 ++- pkg/persistence/object_store_test.go | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/9554-testsabirweb diff --git a/changelogs/unreleased/9554-testsabirweb b/changelogs/unreleased/9554-testsabirweb new file mode 100644 index 000000000..18b925797 --- /dev/null +++ b/changelogs/unreleased/9554-testsabirweb @@ -0,0 +1 @@ +Issue #9544: Add test coverage for S3 bucket name in MRAP ARN notation and fix bucket validation to accept ARN format \ No newline at end of file diff --git a/pkg/persistence/object_store.go b/pkg/persistence/object_store.go index eaf983819..fd441e943 100644 --- a/pkg/persistence/object_store.go +++ b/pkg/persistence/object_store.go @@ -149,7 +149,8 @@ func (b *objectBackupStoreGetter) Get(location *velerov1api.BackupStorageLocatio // if there are any slashes in the middle of 'bucket', the user // probably put / in the bucket field, which we // don't support. - if strings.Contains(bucket, "/") { + // Exception: MRAP ARNs (arn:aws:s3::...) legitimately contain slashes. + if strings.Contains(bucket, "/") && !strings.HasPrefix(bucket, "arn:aws:s3:") { return nil, errors.Errorf("backup storage location's bucket name %q must not contain a '/' (if using a prefix, put it in the 'Prefix' field instead)", location.Spec.ObjectStorage.Bucket) } diff --git a/pkg/persistence/object_store_test.go b/pkg/persistence/object_store_test.go index fac2f8d97..e9a3bde36 100644 --- a/pkg/persistence/object_store_test.go +++ b/pkg/persistence/object_store_test.go @@ -943,6 +943,24 @@ func TestNewObjectBackupStoreGetter(t *testing.T) { wantBucket: "bucket", wantPrefix: "prefix/", }, + { + name: "when the Bucket field is an MRAP ARN, it should be valid", + location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap").Result(), + objectStoreGetter: objectStoreGetter{ + "provider-1": newInMemoryObjectStore("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap"), + }, + credFileStore: velerotest.NewFakeCredentialsFileStore("", nil), + wantBucket: "arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap", + }, + { + name: "when the Bucket field is an MRAP ARN with trailing slash, it should be valid and trimmed", + location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap/").Result(), + objectStoreGetter: objectStoreGetter{ + "provider-1": newInMemoryObjectStore("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap"), + }, + credFileStore: velerotest.NewFakeCredentialsFileStore("", nil), + wantBucket: "arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap", + }, } for _, tc := range tests {