Skip to content

Commit 23a3c24

Browse files
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 <testsabirweb@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> * Address review comments: fix changelog filename and import grouping Signed-off-by: Sabir Ali <testsabirweb@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> * 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 <sabir.ali@spectrocloud.com> Co-authored-by: Cursor <cursoragent@cursor.com> * 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 <sabir.ali@spectrocloud.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Signed-off-by: Sabir Ali <testsabirweb@gmail.com> Signed-off-by: Sabir Ali <sabir.ali@spectrocloud.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b7bc16f commit 23a3c24

3 files changed

Lines changed: 21 additions & 1 deletion

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Issue #9544: Add test coverage for S3 bucket name in MRAP ARN notation and fix bucket validation to accept ARN format

pkg/persistence/object_store.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ func (b *objectBackupStoreGetter) Get(location *velerov1api.BackupStorageLocatio
149149
// if there are any slashes in the middle of 'bucket', the user
150150
// probably put <bucket>/<prefix> in the bucket field, which we
151151
// don't support.
152-
if strings.Contains(bucket, "/") {
152+
// Exception: MRAP ARNs (arn:aws:s3::...) legitimately contain slashes.
153+
if strings.Contains(bucket, "/") && !strings.HasPrefix(bucket, "arn:aws:s3:") {
153154
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)
154155
}
155156

pkg/persistence/object_store_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,24 @@ func TestNewObjectBackupStoreGetter(t *testing.T) {
943943
wantBucket: "bucket",
944944
wantPrefix: "prefix/",
945945
},
946+
{
947+
name: "when the Bucket field is an MRAP ARN, it should be valid",
948+
location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap").Result(),
949+
objectStoreGetter: objectStoreGetter{
950+
"provider-1": newInMemoryObjectStore("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap"),
951+
},
952+
credFileStore: velerotest.NewFakeCredentialsFileStore("", nil),
953+
wantBucket: "arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap",
954+
},
955+
{
956+
name: "when the Bucket field is an MRAP ARN with trailing slash, it should be valid and trimmed",
957+
location: builder.ForBackupStorageLocation("", "").Provider("provider-1").Bucket("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap/").Result(),
958+
objectStoreGetter: objectStoreGetter{
959+
"provider-1": newInMemoryObjectStore("arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap"),
960+
},
961+
credFileStore: velerotest.NewFakeCredentialsFileStore("", nil),
962+
wantBucket: "arn:aws:s3::123456789012:accesspoint/abcdef0123456.mrap",
963+
},
946964
}
947965

948966
for _, tc := range tests {

0 commit comments

Comments
 (0)