From 63214060087fd46c4ee31ea07c896b3d44d7843e Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 3 May 2025 09:07:54 -0700 Subject: [PATCH 1/4] fix: scoutfs missing ListObjects() response fields This fixes some tests that were fialing due to missing response fields in ListObjects(). --- backend/scoutfs/scoutfs.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/scoutfs/scoutfs.go b/backend/scoutfs/scoutfs.go index 08b0020c..1cb5f6e3 100644 --- a/backend/scoutfs/scoutfs.go +++ b/backend/scoutfs/scoutfs.go @@ -769,13 +769,13 @@ func (s *ScoutFS) ListObjects(ctx context.Context, input *s3.ListObjectsInput) ( return s3response.ListObjectsResult{ CommonPrefixes: results.CommonPrefixes, Contents: results.Objects, - Delimiter: &delim, + Delimiter: backend.GetPtrFromString(delim), + Marker: backend.GetPtrFromString(marker), + NextMarker: backend.GetPtrFromString(results.NextMarker), + Prefix: backend.GetPtrFromString(prefix), IsTruncated: &results.Truncated, - Marker: &marker, MaxKeys: &maxkeys, Name: &bucket, - NextMarker: &results.NextMarker, - Prefix: &prefix, }, nil } From a29f7b18391f82519705a103699e43c471fde656 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 3 May 2025 09:12:01 -0700 Subject: [PATCH 2/4] fix: scoutfs missing ListObjectsV2() start after This brings ListObjectsV2 for scoutfs in sync with posix to handle the start after and continuation token ases. --- backend/posix/posix.go | 6 +----- backend/scoutfs/scoutfs.go | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 1c3bbaa1..41e77ad2 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -4312,11 +4312,7 @@ func (p *Posix) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Input) marker := "" if input.ContinuationToken != nil { if input.StartAfter != nil { - if *input.StartAfter > *input.ContinuationToken { - marker = *input.StartAfter - } else { - marker = *input.ContinuationToken - } + marker = max(*input.StartAfter, *input.ContinuationToken) } else { marker = *input.ContinuationToken } diff --git a/backend/scoutfs/scoutfs.go b/backend/scoutfs/scoutfs.go index 1cb5f6e3..a0930914 100644 --- a/backend/scoutfs/scoutfs.go +++ b/backend/scoutfs/scoutfs.go @@ -790,7 +790,11 @@ func (s *ScoutFS) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Inpu } marker := "" if input.ContinuationToken != nil { - marker = *input.ContinuationToken + if input.StartAfter != nil { + marker = max(*input.StartAfter, *input.ContinuationToken) + } else { + marker = *input.ContinuationToken + } } delim := "" if input.Delimiter != nil { @@ -816,16 +820,20 @@ func (s *ScoutFS) ListObjectsV2(ctx context.Context, input *s3.ListObjectsV2Inpu return s3response.ListObjectsV2Result{}, fmt.Errorf("walk %v: %w", bucket, err) } + count := int32(len(results.Objects)) + return s3response.ListObjectsV2Result{ CommonPrefixes: results.CommonPrefixes, Contents: results.Objects, - Delimiter: &delim, IsTruncated: &results.Truncated, - ContinuationToken: &marker, MaxKeys: &maxkeys, Name: &bucket, - NextContinuationToken: &results.NextMarker, - Prefix: &prefix, + KeyCount: &count, + Delimiter: backend.GetPtrFromString(delim), + ContinuationToken: backend.GetPtrFromString(marker), + NextContinuationToken: backend.GetPtrFromString(results.NextMarker), + Prefix: backend.GetPtrFromString(prefix), + StartAfter: backend.GetPtrFromString(*input.StartAfter), }, nil } From a60d6a7faa86e17de5e9e708602fa73a1fc8850e Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 3 May 2025 09:30:45 -0700 Subject: [PATCH 3/4] fix: scoutfs racing mutlipart uploads internal error When multiple uploads with the same object key are racing, we can end up with an EEXIST when trying to link the final object into the namespace. When this happens, we should just remove the existing file and try again since the semantics are that the last upload should win. --- backend/scoutfs/scoutfs_compat.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/backend/scoutfs/scoutfs_compat.go b/backend/scoutfs/scoutfs_compat.go index 044acc72..0a590bac 100644 --- a/backend/scoutfs/scoutfs_compat.go +++ b/backend/scoutfs/scoutfs_compat.go @@ -155,10 +155,20 @@ func (tmp *tmpfile) link() error { } defer dirf.Close() - err = unix.Linkat(int(procdir.Fd()), filepath.Base(tmp.f.Name()), - int(dirf.Fd()), filepath.Base(objPath), unix.AT_SYMLINK_FOLLOW) - if err != nil { - return fmt.Errorf("link tmpfile: %w", err) + for { + err = unix.Linkat(int(procdir.Fd()), filepath.Base(tmp.f.Name()), + int(dirf.Fd()), filepath.Base(objPath), unix.AT_SYMLINK_FOLLOW) + if errors.Is(err, fs.ErrExist) { + err := os.Remove(objPath) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove stale path: %w", err) + } + continue + } + if err != nil { + return fmt.Errorf("link tmpfile: %w", err) + } + break } err = tmp.f.Close() From e9286f7a23636d7107e2a1a7762452ba7650f053 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 3 May 2025 12:04:47 -0700 Subject: [PATCH 4/4] feat: add scoutfs group tests to integration --- cmd/versitygw/test.go | 5 ++ tests/integration/group-tests.go | 93 ++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/cmd/versitygw/test.go b/cmd/versitygw/test.go index dd74f9c4..a4f9e753 100644 --- a/cmd/versitygw/test.go +++ b/cmd/versitygw/test.go @@ -124,6 +124,11 @@ func initTestCommands() []*cli.Command { }, }, }, + { + Name: "scoutfs", + Usage: "Tests scoutfs full flow", + Action: getAction(integration.TestScoutfs), + }, { Name: "iam", Usage: "Tests iam service", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 73888b15..a7eb42bb 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -659,6 +659,99 @@ func TestPosix(s *S3Conf) { } } +func TestScoutfs(s *S3Conf) { + TestAuthentication(s) + TestPresignedAuthentication(s) + TestCreateBucket(s) + TestHeadBucket(s) + TestListBuckets(s) + TestDeleteBucket(s) + TestPutBucketOwnershipControls(s) + TestGetBucketOwnershipControls(s) + TestDeleteBucketOwnershipControls(s) + TestPutBucketTagging(s) + TestGetBucketTagging(s) + TestDeleteBucketTagging(s) + TestPutObject(s) + TestHeadObject(s) + TestGetObjectAttributes(s) + TestGetObject(s) + TestListObjects(s) + TestListObjectsV2(s) + TestListObjectVersions_VD(s) + TestDeleteObject(s) + TestDeleteObjects(s) + TestCopyObject(s) + TestPutObjectTagging(s) + TestDeleteObjectTagging(s) + TestUploadPart(s) + TestUploadPartCopy(s) + TestListParts(s) + TestListMultipartUploads(s) + TestAbortMultipartUpload(s) + TestPutBucketAcl(s) + TestGetBucketAcl(s) + TestPutBucketPolicy(s) + TestGetBucketPolicy(s) + TestDeleteBucketPolicy(s) + TestPutObjectLockConfiguration(s) + TestGetObjectLockConfiguration(s) + TestPutObjectRetention(s) + TestGetObjectRetention(s) + TestPutObjectLegalHold(s) + TestGetObjectLegalHold(s) + TestWORMProtection(s) + TestAccessControl(s) + + CreateMultipartUpload_non_existing_bucket(s) + CreateMultipartUpload_with_tagging(s) + CreateMultipartUpload_with_object_lock(s) + CreateMultipartUpload_with_object_lock_not_enabled(s) + CreateMultipartUpload_with_object_lock_invalid_retention(s) + CreateMultipartUpload_past_retain_until_date(s) + CreateMultipartUpload_invalid_legal_hold(s) + CreateMultipartUpload_invalid_object_lock_mode(s) + CreateMultipartUpload_invalid_checksum_algorithm(s) + CreateMultipartUpload_empty_checksum_algorithm_with_checksum_type(s) + CreateMultipartUpload_invalid_checksum_type(s) + CreateMultipartUpload_valid_checksum_algorithm(s) + CreateMultipartUpload_success(s) + + CompletedMultipartUpload_non_existing_bucket(s) + CompleteMultipartUpload_incorrect_part_number(s) + CompleteMultipartUpload_invalid_part_number(s) + CompleteMultipartUpload_invalid_ETag(s) + CompleteMultipartUpload_small_upload_size(s) + CompleteMultipartUpload_empty_parts(s) + CompleteMultipartUpload_incorrect_parts_order(s) + CompleteMultipartUpload_mpu_object_size(s) + CompleteMultipartUpload_invalid_checksum_type(s) + CompleteMultipartUpload_invalid_checksum_part(s) + CompleteMultipartUpload_multiple_checksum_part(s) + CompleteMultipartUpload_incorrect_checksum_part(s) + CompleteMultipartUpload_different_checksum_part(s) + CompleteMultipartUpload_missing_part_checksum(s) + CompleteMultipartUpload_multiple_final_checksums(s) + CompleteMultipartUpload_invalid_final_checksums(s) + CompleteMultipartUpload_checksum_type_mismatch(s) + CompleteMultipartUpload_should_ignore_the_final_checksum(s) + CompleteMultipartUpload_success(s) + CompleteMultipartUpload_racey_success(s) + + // posix/scoutfs specific tests + PutObject_overwrite_dir_obj(s) + PutObject_overwrite_file_obj(s) + PutObject_overwrite_file_obj_with_nested_obj(s) + PutObject_dir_obj_with_data(s) + CreateMultipartUpload_dir_obj(s) + PutObject_name_too_long(s) + HeadObject_name_too_long(s) + DeleteObject_name_too_long(s) + CopyObject_overwrite_same_dir_object(s) + CopyObject_overwrite_same_file_object(s) + DeleteObject_directory_not_empty(s) +} + func TestIAM(s *S3Conf) { IAM_user_access_denied(s) IAM_userplus_access_denied(s)