diff --git a/auth/object_lock.go b/auth/object_lock.go index 5ff0432..0fc60f9 100644 --- a/auth/object_lock.go +++ b/auth/object_lock.go @@ -278,6 +278,11 @@ func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects [ if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchKey)) { continue } + // the object is a delete marker, if a `MethodNotAllowed` error is returned + // no object lock check is needed + if errors.Is(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)) { + continue + } if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration)) { checkRetention = false } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index dc7a372..edcbe46 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -871,6 +871,30 @@ func getBoolPtr(b bool) *bool { return &b } +// ensureNotDeleteMarker return a `MethodNotAllowd` error +// if the provided object(version) is a delete marker +func (p *Posix) ensureNotDeleteMarker(bucket, object, versionId string) error { + if !p.versioningEnabled() { + return nil + } + + _, err := p.meta.RetrieveAttribute(nil, bucket, object, deleteMarkerKey) + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if versionId != "" { + return s3err.GetAPIError(s3err.ErrNoSuchVersion) + } + return s3err.GetAPIError(s3err.ErrNoSuchKey) + } + if errors.Is(err, meta.ErrNoSuchKey) { + return nil + } + if err != nil { + return fmt.Errorf("get delete marker attr: %w", err) + } + + return s3err.GetAPIError(s3err.ErrMethodNotAllowed) +} + // Check if the given object is a delete marker func (p *Posix) isObjDeleteMarker(bucket, object string) (bool, error) { _, err := p.meta.RetrieveAttribute(nil, bucket, object, deleteMarkerKey) @@ -4890,6 +4914,11 @@ func (p *Posix) GetObjectTagging(_ context.Context, bucket, object, versionId st } } + err = p.ensureNotDeleteMarker(bucket, object, versionId) + if err != nil { + return nil, err + } + return p.getAttrTags(bucket, object, versionId) } @@ -4952,6 +4981,11 @@ func (p *Posix) PutObjectTagging(_ context.Context, bucket, object, versionId st } } + err = p.ensureNotDeleteMarker(bucket, object, versionId) + if err != nil { + return err + } + if tags == nil { err = p.meta.DeleteAttribute(bucket, object, tagHdr) if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { @@ -5243,6 +5277,11 @@ func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId } } + err = p.ensureNotDeleteMarker(bucket, object, versionId) + if err != nil { + return err + } + err = p.meta.StoreAttribute(nil, bucket, object, objectLegalHoldKey, statusData) if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { if versionId != "" { @@ -5289,6 +5328,11 @@ func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId } } + err = p.ensureNotDeleteMarker(bucket, object, versionId) + if err != nil { + return nil, err + } + data, err := p.meta.RetrieveAttribute(nil, bucket, object, objectLegalHoldKey) if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { if versionId != "" { @@ -5340,6 +5384,11 @@ func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId } } + err = p.ensureNotDeleteMarker(bucket, object, versionId) + if err != nil { + return err + } + err = p.meta.StoreAttribute(nil, bucket, object, objectRetentionKey, retention) if err != nil { return fmt.Errorf("set object lock config: %w", err) @@ -5380,6 +5429,11 @@ func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId } } + err = p.ensureNotDeleteMarker(bucket, object, versionId) + if err != nil { + return nil, err + } + data, err := p.meta.RetrieveAttribute(nil, bucket, object, objectRetentionKey) if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { if versionId != "" { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 6f4b73e..3d6c0ba 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1024,6 +1024,7 @@ func TestVersioning(ts *TestState) { // object tagging actions ts.Run(Versioning_PutObjectTagging_invalid_versionId) ts.Run(Versioning_PutObjectTagging_non_existing_object_version) + ts.Run(Versioning_PutGetDeleteObjectTagging_delete_marker) ts.Run(Versioning_GetObjectTagging_invalid_versionId) ts.Run(Versioning_GetObjectTagging_non_existing_object_version) ts.Run(Versioning_DeleteObjectTagging_invalid_versionId) @@ -1068,12 +1069,14 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_PutObjectRetention_non_existing_object_version) ts.Run(Versioning_GetObjectRetention_invalid_versionId) ts.Run(Versioning_GetObjectRetention_non_existing_object_version) + ts.Run(Versioning_Put_GetObjectRetention_delete_marker) ts.Run(Versioning_Put_GetObjectRetention_success) // Object-Lock Legal hold ts.Run(Versioning_PutObjectLegalHold_invalid_versionId) ts.Run(Versioning_PutObjectLegalHold_non_existing_object_version) ts.Run(Versioning_GetObjectLegalHold_invalid_versionId) ts.Run(Versioning_GetObjectLegalHold_non_existing_object_version) + ts.Run(Versioning_PutGetObjectLegalHold_delete_marker) ts.Run(Versioning_Put_GetObjectLegalHold_success) // WORM protection ts.Run(Versioning_WORM_obj_version_locked_with_legal_hold) @@ -1085,6 +1088,7 @@ func TestVersioning(ts *TestState) { ts.Run(Versioning_WORM_PutObject_overwrite_locked_object) ts.Run(Versioning_WORM_CopyObject_overwrite_locked_object) ts.Run(Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object) + ts.Run(Versioning_WORM_remove_delete_marker_under_bucket_default_retention) // Concurrent requests // Versioninig_concurrent_upload_object ts.Run(Versioning_AccessControl_GetObjectVersion) @@ -1736,6 +1740,7 @@ func GetIntTests() IntTests { "Versioning_GetObject_null_versionId_obj": Versioning_GetObject_null_versionId_obj, "Versioning_PutObjectTagging_invalid_versionId": Versioning_PutObjectTagging_invalid_versionId, "Versioning_PutObjectTagging_non_existing_object_version": Versioning_PutObjectTagging_non_existing_object_version, + "Versioning_PutGetDeleteObjectTagging_delete_marker": Versioning_PutGetDeleteObjectTagging_delete_marker, "Versioning_GetObjectTagging_invalid_versionId": Versioning_GetObjectTagging_invalid_versionId, "Versioning_GetObjectTagging_non_existing_object_version": Versioning_GetObjectTagging_non_existing_object_version, "Versioning_DeleteObjectTagging_invalid_versionId": Versioning_DeleteObjectTagging_invalid_versionId, @@ -1774,11 +1779,13 @@ func GetIntTests() IntTests { "Versioning_PutObjectRetention_non_existing_object_version": Versioning_PutObjectRetention_non_existing_object_version, "Versioning_GetObjectRetention_invalid_versionId": Versioning_GetObjectRetention_invalid_versionId, "Versioning_GetObjectRetention_non_existing_object_version": Versioning_GetObjectRetention_non_existing_object_version, + "Versioning_Put_GetObjectRetention_delete_marker": Versioning_Put_GetObjectRetention_delete_marker, "Versioning_Put_GetObjectRetention_success": Versioning_Put_GetObjectRetention_success, "Versioning_PutObjectLegalHold_invalid_versionId": Versioning_PutObjectLegalHold_invalid_versionId, "Versioning_PutObjectLegalHold_non_existing_object_version": Versioning_PutObjectLegalHold_non_existing_object_version, "Versioning_GetObjectLegalHold_invalid_versionId": Versioning_GetObjectLegalHold_invalid_versionId, "Versioning_GetObjectLegalHold_non_existing_object_version": Versioning_GetObjectLegalHold_non_existing_object_version, + "Versioning_PutGetObjectLegalHold_delete_marker": Versioning_PutGetObjectLegalHold_delete_marker, "Versioning_Put_GetObjectLegalHold_success": Versioning_Put_GetObjectLegalHold_success, "Versioning_WORM_obj_version_locked_with_legal_hold": Versioning_WORM_obj_version_locked_with_legal_hold, "Versioning_WORM_obj_version_locked_with_governance_retention": Versioning_WORM_obj_version_locked_with_governance_retention, @@ -1789,6 +1796,7 @@ func GetIntTests() IntTests { "Versioning_WORM_PutObject_overwrite_locked_object": Versioning_WORM_PutObject_overwrite_locked_object, "Versioning_WORM_CopyObject_overwrite_locked_object": Versioning_WORM_CopyObject_overwrite_locked_object, "Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object": Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object, + "Versioning_WORM_remove_delete_marker_under_bucket_default_retention": Versioning_WORM_remove_delete_marker_under_bucket_default_retention, "Versioning_AccessControl_GetObjectVersion": Versioning_AccessControl_GetObjectVersion, "Versioning_AccessControl_HeadObjectVersion": Versioning_AccessControl_HeadObjectVersion, "Versioning_AccessControl_object_tagging_policy": Versioning_AccessControl_object_tagging_policy, diff --git a/tests/integration/utils.go b/tests/integration/utils.go index e4d49e2..a3d2796 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -118,17 +118,25 @@ func teardown(s *S3Conf, bucket string) error { s3client := s.GetClient() deleteObject := func(bucket, key, versionId *string) error { - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ - Bucket: bucket, - Key: key, - VersionId: versionId, - }) - cancel() - if err != nil { - return fmt.Errorf("failed to delete object %v: %w", *key, err) + var attempts int + var err error + for attempts < maxRetryAttempts { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: bucket, + Key: key, + VersionId: versionId, + }) + cancel() + if err == nil { + return nil + } + + attempts++ + time.Sleep(time.Second) } - return nil + + return fmt.Errorf("delete object %s: %w", *key, err) } if s.versioningEnabled { @@ -1915,6 +1923,7 @@ func cleanupLockedObjects(client *s3.Client, bucket string, objs []objToDelete) if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchKey)) { return nil } + if err != nil { return err } @@ -1922,29 +1931,8 @@ func cleanupLockedObjects(client *s3.Client, bucket string, objs []objToDelete) // Wait until retention lock expires before attempting delete time.Sleep(lockWaitTime) - // Attempt deletion with retries - attempts := 0 - for attempts != maxRetryAttempts { - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err = client.DeleteObject(ctx, &s3.DeleteObjectInput{ - Bucket: &bucket, - Key: &obj.key, - VersionId: getPtr(obj.versionId), - }) - cancel() - if err != nil { - // Retry after a short delay if delete fails - time.Sleep(time.Second) - attempts++ - continue - } - - // Success, no more retries needed - return nil - } - // Return last error if all retries failed - return err + return nil }) } diff --git a/tests/integration/versioning.go b/tests/integration/versioning.go index c6402fe..80c1bf8 100644 --- a/tests/integration/versioning.go +++ b/tests/integration/versioning.go @@ -2176,6 +2176,57 @@ func Versioning_GetObjectRetention_non_existing_object_version(s *S3Conf) error }, withLock(), withVersioning(types.BucketVersioningStatusEnabled)) } +func Versioning_Put_GetObjectRetention_delete_marker(s *S3Conf) error { + testName := "Versioning_Put_GetObjectRetention_delete_marker" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-object" + _, err := putObjectWithData(10, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + // PutObjectRetention + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + Retention: &types.ObjectLockRetention{ + Mode: types.ObjectLockRetentionModeCompliance, + RetainUntilDate: getPtr(time.Now().AddDate(1, 0, 0)), + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)); err != nil { + return err + } + + // GetObjectRetention + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectRetention(ctx, &s3.GetObjectRetentionInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)) + }, withLock()) +} + func Versioning_Put_GetObjectRetention_success(s *S3Conf) error { testName := "Versioning_Put_GetObjectRetention_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -2316,6 +2367,56 @@ func Versioning_GetObjectLegalHold_non_existing_object_version(s *S3Conf) error }, withLock(), withVersioning(types.BucketVersioningStatusEnabled)) } +func Versioning_PutGetObjectLegalHold_delete_marker(s *S3Conf) error { + testName := "Versioning_PutGetObjectLegalHold_delete_marker" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-object" + _, err := putObjectWithData(10, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + // PutObjectLegalHold + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + LegalHold: &types.ObjectLockLegalHold{ + Status: types.ObjectLockLegalHoldStatusOn, + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)); err != nil { + return err + } + + // GetObjectLegalHold + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectLegalHold(ctx, &s3.GetObjectLegalHoldInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)) + }, withLock()) +} + func Versioning_Put_GetObjectLegalHold_success(s *S3Conf) error { testName := "Versioning_Put_GetObjectLegalHold_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -2957,6 +3058,85 @@ func Versioning_WORM_CompleteMultipartUpload_overwrite_locked_object(s *S3Conf) }, withLock()) } +func Versioning_WORM_remove_delete_marker_under_bucket_default_retention(s *S3Conf) error { + testName := "Versioning_WORM_remove_delete_marker_under_bucket_default_retention" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutObjectLockConfiguration(ctx, &s3.PutObjectLockConfigurationInput{ + Bucket: &bucket, + ObjectLockConfiguration: &types.ObjectLockConfiguration{ + ObjectLockEnabled: types.ObjectLockEnabledEnabled, + Rule: &types.ObjectLockRule{ + DefaultRetention: &types.DefaultRetention{ + Mode: types.ObjectLockRetentionModeGovernance, + Days: getPtr(int32(5)), + }, + }, + }, + }) + cancel() + if err != nil { + return err + } + + obj := "my-object" + versions, err := createObjVersions(s3client, bucket, obj, 3) + if err != nil { + return err + } + + // Create a delete marker + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + // Delete the delete marker + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + resp, err := s3client.ListObjectVersions(ctx, &s3.ListObjectVersionsInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + + if !compareVersions(versions, resp.Versions) { + return fmt.Errorf("expected the object vresions to be %v, instead got %v", versions, resp.Versions) + } + if len(resp.DeleteMarkers) != 0 { + return fmt.Errorf("expected empty delete markers list, instead got %v", resp.DeleteMarkers) + } + + // + lockedVersions := make([]objToDelete, 0, len(versions)) + for _, v := range versions { + lockedVersions = append(lockedVersions, objToDelete{ + key: obj, + versionId: getString(v.VersionId), + isCompliance: false, + }) + } + return cleanupLockedObjects(s3client, bucket, lockedVersions) + }, withLock()) +} + func Versioning_AccessControl_GetObjectVersion(s *S3Conf) error { testName := "Versioning_AccessControl_GetObjectVersion" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { @@ -3516,6 +3696,68 @@ func Versioning_PutObjectTagging_non_existing_object_version(s *S3Conf) error { }, withVersioning(types.BucketVersioningStatusEnabled)) } +func Versioning_PutGetDeleteObjectTagging_delete_marker(s *S3Conf) error { + testName := "Versioning_PutGetDeleteObjectTagging_delete_marker" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-object" + _, err := putObjectWithData(10, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + }, s3client) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } + + // PutObjectTagging + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectTagging(ctx, &s3.PutObjectTaggingInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + Tagging: &types.Tagging{ + TagSet: []types.Tag{{Key: getPtr("key"), Value: getPtr("value")}}, + }, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)); err != nil { + return err + } + + // GetObjectTagging + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.GetObjectTagging(ctx, &s3.GetObjectTaggingInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)); err != nil { + return err + } + + // DeleteObjectTagging + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.DeleteObjectTagging(ctx, &s3.DeleteObjectTaggingInput{ + Bucket: &bucket, + Key: &obj, + VersionId: out.VersionId, + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)) + }, withLock()) +} + func Versioning_PutObjectTagging_invalid_versionId(s *S3Conf) error { testName := "Versioning_PutObjectTagging_invalid_versionId" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {