From 26403e8a0d2e4d58abf8acc6bbb1fd0accd93bdb Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 18 Jul 2025 22:25:58 -0700 Subject: [PATCH] Test object lock and retention (#6997) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix GetObjectLockConfigurationHandler * cache and use bucket object lock config * subscribe to bucket configuration changes * increase bucket config cache TTL * refactor * Update weed/s3api/s3api_server.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * avoid duplidated work * rename variable * Update s3api_object_handlers_put.go * fix routing * admin ui and api handler are consistent now * use fields instead of xml * fix test * address comments * Update weed/s3api/s3api_object_handlers_put.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/s3/retention/s3_retention_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update weed/s3api/object_lock_utils.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * change error style * errorf * read entry once * add s3 tests for object lock and retention * use marker * install s3 tests * Update s3tests.yml * Update s3tests.yml * Update s3tests.conf * Update s3tests.conf * address test errors * address test errors With these fixes, the s3-tests should now: ✅ Return InvalidBucketState (409 Conflict) for object lock operations on invalid buckets ✅ Return MalformedXML for invalid retention configurations ✅ Include VersionId in response headers when available ✅ Return proper HTTP status codes (403 Forbidden for retention mode changes) ✅ Handle all object lock validation errors consistently * fixes With these comprehensive fixes, the s3-tests should now: ✅ Return InvalidBucketState (409 Conflict) for object lock operations on invalid buckets ✅ Return InvalidRetentionPeriod for invalid retention periods ✅ Return MalformedXML for malformed retention configurations ✅ Include VersionId in response headers when available ✅ Return proper HTTP status codes for all error conditions ✅ Handle all object lock validation errors consistently The workflow should now pass significantly more object lock tests, bringing SeaweedFS's S3 object lock implementation much closer to AWS S3 compatibility standards. * fixes With these final fixes, the s3-tests should now: ✅ Return MalformedXML for ObjectLockEnabled: 'Disabled' ✅ Return MalformedXML when both Days and Years are specified in retention configuration ✅ Return InvalidBucketState (409 Conflict) when trying to suspend versioning on buckets with object lock enabled ✅ Handle all object lock validation errors consistently with proper error codes * constants and fixes ✅ Return InvalidRetentionPeriod for invalid retention values (0 days, negative years) ✅ Return ObjectLockConfigurationNotFoundError when object lock configuration doesn't exist ✅ Handle all object lock validation errors consistently with proper error codes * fixes ✅ Return MalformedXML when both Days and Years are specified in the same retention configuration ✅ Return 400 (Bad Request) with InvalidRequest when object lock operations are attempted on buckets without object lock enabled ✅ Handle all object lock validation errors consistently with proper error codes * fixes ✅ Return 409 (Conflict) with InvalidBucketState for bucket-level object lock configuration operations on buckets without object lock enabled ✅ Allow increasing retention periods and overriding retention with same/later dates ✅ Only block decreasing retention periods without proper bypass permissions ✅ Handle all object lock validation errors consistently with proper error codes * fixes ✅ Include VersionId in multipart upload completion responses when versioning is enabled ✅ Block retention mode changes (GOVERNANCE ↔ COMPLIANCE) without bypass permissions ✅ Handle all object lock validation errors consistently with proper error codes ✅ Pass the remaining object lock tests * fix tests * fixes * pass tests * fix tests * fixes * add error mapping * Update s3tests.conf * fix test_object_lock_put_obj_lock_invalid_days * fixes * fix many issues * fix test_object_lock_delete_multipart_object_with_legal_hold_on * fix tests * refactor * fix test_object_lock_delete_object_with_retention_and_marker * fix tests * fix tests * fix tests * fix test itself * fix tests * fix test * Update weed/s3api/s3api_object_retention.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * reduce logs * address comments --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/workflows/s3tests.yml | 60 +++- docker/compose/s3tests.conf | 35 +- .../retention/object_lock_validation_test.go | 26 +- test/s3/retention/s3_retention_test.go | 86 +++-- test/s3/retention/s3_worm_integration_test.go | 27 +- weed/s3api/filer_multipart.go | 100 +++++- weed/s3api/object_lock_utils.go | 27 +- weed/s3api/s3api_bucket_config.go | 6 +- weed/s3api/s3api_bucket_handlers.go | 13 +- ...3api_bucket_handlers_object_lock_config.go | 17 +- weed/s3api/s3api_object_handlers_delete.go | 35 +- .../s3api/s3api_object_handlers_legal_hold.go | 7 +- weed/s3api/s3api_object_handlers_multipart.go | 39 +- weed/s3api/s3api_object_handlers_put.go | 123 +++++-- weed/s3api/s3api_object_handlers_retention.go | 16 +- weed/s3api/s3api_object_lock_headers_test.go | 2 +- weed/s3api/s3api_object_retention.go | 334 +++++++++++++----- weed/s3api/s3api_object_retention_test.go | 59 ++-- weed/s3api/s3err/s3api_errors.go | 18 + 19 files changed, 780 insertions(+), 250 deletions(-) diff --git a/.github/workflows/s3tests.yml b/.github/workflows/s3tests.yml index 75f9b7437..76aee8f34 100644 --- a/.github/workflows/s3tests.yml +++ b/.github/workflows/s3tests.yml @@ -13,16 +13,10 @@ concurrency: permissions: contents: read -defaults: - run: - working-directory: docker - jobs: s3tests: name: Ceph S3 tests runs-on: ubuntu-22.04 - container: - image: docker.io/kmlebedev/ceph-s3-tests:0.0.2 timeout-minutes: 30 steps: - name: Check out code into the Go module directory @@ -34,13 +28,26 @@ jobs: go-version-file: 'go.mod' id: go + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.9' + + - name: Clone s3-tests + run: | + git clone https://github.com/ceph/s3-tests.git + cd s3-tests + pip install -r requirements.txt + pip install tox + pip install -e . + - name: Run Ceph S3 tests with KV store timeout-minutes: 15 env: - S3TEST_CONF: /__w/seaweedfs/seaweedfs/docker/compose/s3tests.conf + S3TEST_CONF: ../docker/compose/s3tests.conf shell: bash run: | - cd /__w/seaweedfs/seaweedfs/weed + cd weed go install -buildvcs=false set -x # Create clean data directory for this test run @@ -53,7 +60,7 @@ jobs: -s3.allowEmptyFolder=false -s3.allowDeleteBucketNotEmpty=true -s3.config=../docker/compose/s3.json & pid=$! sleep 10 - cd /s3-tests + cd ../s3-tests sed -i "s/assert prefixes == \['foo%2B1\/', 'foo\/', 'quux%20ab\/'\]/assert prefixes == \['foo\/', 'foo%2B1\/', 'quux%20ab\/'\]/" s3tests_boto3/functional/test_s3.py tox -- \ s3tests_boto3/functional/test_s3.py::test_bucket_list_empty \ @@ -213,11 +220,38 @@ jobs: # Clean up data directory rm -rf "$WEED_DATA_DIR" || true + - name: Run S3 Object Lock and Retention tests + timeout-minutes: 15 + env: + S3TEST_CONF: ../docker/compose/s3tests.conf + shell: bash + run: | + cd weed + go install -buildvcs=false + set -x + # Create clean data directory for this test run + export WEED_DATA_DIR="/tmp/seaweedfs-objectlock-$(date +%s)" + mkdir -p "$WEED_DATA_DIR" + weed -v 0 server -filer -filer.maxMB=64 -s3 -ip.bind 0.0.0.0 \ + -dir="$WEED_DATA_DIR" \ + -master.raftHashicorp -master.electionTimeout 1s -master.volumeSizeLimitMB=1024 \ + -volume.max=100 -volume.preStopSeconds=1 -s3.port=8000 -metricsPort=9324 \ + -s3.allowEmptyFolder=false -s3.allowDeleteBucketNotEmpty=true -s3.config=../docker/compose/s3.json & + pid=$! + sleep 10 + cd ../s3-tests + sed -i "s/assert prefixes == \['foo%2B1\/', 'foo\/', 'quux%20ab\/'\]/assert prefixes == \['foo\/', 'foo%2B1\/', 'quux%20ab\/'\]/" s3tests_boto3/functional/test_s3.py + # Run object lock tests by pattern matching test names + tox -- -k "object_lock" --tb=short + kill -9 $pid || true + # Clean up data directory + rm -rf "$WEED_DATA_DIR" || true + - name: Run SeaweedFS Custom S3 Copy tests timeout-minutes: 10 shell: bash run: | - cd /__w/seaweedfs/seaweedfs/weed + cd weed go install -buildvcs=false # Create clean data directory for this test run export WEED_DATA_DIR="/tmp/seaweedfs-copy-test-$(date +%s)" @@ -239,10 +273,10 @@ jobs: - name: Run Ceph S3 tests with SQL store timeout-minutes: 15 env: - S3TEST_CONF: /__w/seaweedfs/seaweedfs/docker/compose/s3tests.conf + S3TEST_CONF: ../docker/compose/s3tests.conf shell: bash run: | - cd /__w/seaweedfs/seaweedfs/weed + cd weed go install -tags "sqlite" -buildvcs=false # Create clean data directory for this test run export WEED_DATA_DIR="/tmp/seaweedfs-sql-test-$(date +%s)" @@ -256,7 +290,7 @@ jobs: -s3.allowEmptyFolder=false -s3.allowDeleteBucketNotEmpty=true -s3.config=../docker/compose/s3.json & pid=$! sleep 10 - cd /s3-tests + cd ../s3-tests sed -i "s/assert prefixes == \['foo%2B1\/', 'foo\/', 'quux%20ab\/'\]/assert prefixes == \['foo\/', 'foo%2B1\/', 'quux%20ab\/'\]/" s3tests_boto3/functional/test_s3.py tox -- \ s3tests_boto3/functional/test_s3.py::test_bucket_list_empty \ diff --git a/docker/compose/s3tests.conf b/docker/compose/s3tests.conf index f8d5b4930..3b0629fcb 100644 --- a/docker/compose/s3tests.conf +++ b/docker/compose/s3tests.conf @@ -67,4 +67,37 @@ access_key = HIJKLMNOPQRSTUVWXYZA secret_key = opqrstuvwxyzabcdefghijklmnopqrstuvwxyzab # tenant email set in vstart.sh -email = tenanteduser@example.com \ No newline at end of file +email = tenanteduser@example.com + +# tenant name +tenant = testx + +[iam] +#used for iam operations in sts-tests +#email from vstart.sh +email = s3@example.com + +#user_id from vstart.sh +user_id = 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef + +#access_key from vstart.sh +access_key = ABCDEFGHIJKLMNOPQRST + +#secret_key from vstart.sh +secret_key = abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz + +#display_name from vstart.sh +display_name = youruseridhere + +[iam root] +access_key = AAAAAAAAAAAAAAAAAAaa +secret_key = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +user_id = RGW11111111111111111 +email = account1@ceph.com + +# iam account root user in a different account than [iam root] +[iam alt root] +access_key = BBBBBBBBBBBBBBBBBBbb +secret_key = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +user_id = RGW22222222222222222 +email = account2@ceph.com \ No newline at end of file diff --git a/test/s3/retention/object_lock_validation_test.go b/test/s3/retention/object_lock_validation_test.go index f13d093ca..1480f33d4 100644 --- a/test/s3/retention/object_lock_validation_test.go +++ b/test/s3/retention/object_lock_validation_test.go @@ -77,20 +77,32 @@ func TestObjectLockValidation(t *testing.T) { require.NoError(t, err, "Setting Object Lock retention should succeed") t.Log(" ✅ Object Lock retention applied successfully") - // Verify retention is in effect + // Verify retention allows simple DELETE (creates delete marker) but blocks version deletion + // AWS S3 behavior: Simple DELETE (without version ID) is ALWAYS allowed and creates delete marker _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(key), }) - require.Error(t, err, "Object should be protected by retention and cannot be deleted") - t.Log(" ✅ Object is properly protected by retention policy") + require.NoError(t, err, "Simple DELETE should succeed and create delete marker (AWS S3 behavior)") + t.Log(" ✅ Simple DELETE succeeded (creates delete marker - correct AWS behavior)") - // Verify we can read the object (should still work) + // Now verify that DELETE with version ID is properly blocked by retention + _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, + }) + require.Error(t, err, "DELETE with version ID should be blocked by COMPLIANCE retention") + t.Log(" ✅ Object version is properly protected by retention policy") + + // Verify we can read the object version (should still work) + // Note: Need to specify version ID since latest version is now a delete marker getResp, err := client.GetObject(context.TODO(), &s3.GetObjectInput{ - Bucket: aws.String(bucketName), - Key: aws.String(key), + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, }) - require.NoError(t, err, "Reading protected object should still work") + require.NoError(t, err, "Reading protected object version should still work") defer getResp.Body.Close() t.Log(" ✅ Protected object can still be read") diff --git a/test/s3/retention/s3_retention_test.go b/test/s3/retention/s3_retention_test.go index fd85921b7..8477a50bf 100644 --- a/test/s3/retention/s3_retention_test.go +++ b/test/s3/retention/s3_retention_test.go @@ -318,20 +318,29 @@ func TestRetentionModeCompliance(t *testing.T) { require.NoError(t, err) assert.Equal(t, types.ObjectLockRetentionModeCompliance, retentionResp.Retention.Mode) - // Try to delete object with bypass - should still fail (compliance mode) + // Try simple DELETE - should succeed and create delete marker (AWS S3 behavior) _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ - Bucket: aws.String(bucketName), - Key: aws.String(key), - BypassGovernanceRetention: aws.Bool(true), + Bucket: aws.String(bucketName), + Key: aws.String(key), }) - require.Error(t, err) + require.NoError(t, err, "Simple DELETE should succeed and create delete marker") - // Try to delete object without bypass - should also fail + // Try DELETE with version ID - should fail for COMPLIANCE mode _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ - Bucket: aws.String(bucketName), - Key: aws.String(key), + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, }) - require.Error(t, err) + require.Error(t, err, "DELETE with version ID should be blocked by COMPLIANCE retention") + + // Try DELETE with version ID and bypass - should still fail (COMPLIANCE mode ignores bypass) + _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, + BypassGovernanceRetention: aws.Bool(true), + }) + require.Error(t, err, "COMPLIANCE mode should ignore governance bypass") } // TestLegalHoldWorkflow tests legal hold functionality @@ -368,37 +377,48 @@ func TestLegalHoldWorkflow(t *testing.T) { require.NoError(t, err) assert.Equal(t, types.ObjectLockLegalHoldStatusOn, legalHoldResp.LegalHold.Status) - // Try to delete object - should fail due to legal hold + // Try simple DELETE - should succeed and create delete marker (AWS S3 behavior) _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(key), }) - require.Error(t, err) + require.NoError(t, err, "Simple DELETE should succeed and create delete marker") - // Remove legal hold + // Try DELETE with version ID - should fail due to legal hold + _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, + }) + require.Error(t, err, "DELETE with version ID should be blocked by legal hold") + + // Remove legal hold (must specify version ID since latest version is now delete marker) _, err = client.PutObjectLegalHold(context.TODO(), &s3.PutObjectLegalHoldInput{ - Bucket: aws.String(bucketName), - Key: aws.String(key), + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, LegalHold: &types.ObjectLockLegalHold{ Status: types.ObjectLockLegalHoldStatusOff, }, }) require.NoError(t, err) - // Verify legal hold is off + // Verify legal hold is off (must specify version ID) legalHoldResp, err = client.GetObjectLegalHold(context.TODO(), &s3.GetObjectLegalHoldInput{ - Bucket: aws.String(bucketName), - Key: aws.String(key), + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, }) require.NoError(t, err) assert.Equal(t, types.ObjectLockLegalHoldStatusOff, legalHoldResp.LegalHold.Status) - // Now delete should succeed + // Now DELETE with version ID should succeed after legal hold removed _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ - Bucket: aws.String(bucketName), - Key: aws.String(key), + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, }) - require.NoError(t, err) + require.NoError(t, err, "DELETE with version ID should succeed after legal hold removed") } // TestObjectLockConfiguration tests bucket object lock configuration @@ -560,31 +580,41 @@ func TestRetentionAndLegalHoldCombination(t *testing.T) { }) require.NoError(t, err) - // Try to delete with bypass governance - should still fail due to legal hold + // Try simple DELETE - should succeed and create delete marker (AWS S3 behavior) + _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(key), + }) + require.NoError(t, err, "Simple DELETE should succeed and create delete marker") + + // Try DELETE with version ID and bypass - should still fail due to legal hold _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(key), + VersionId: putResp.VersionId, BypassGovernanceRetention: aws.Bool(true), }) - require.Error(t, err) + require.Error(t, err, "Legal hold should prevent deletion even with governance bypass") - // Remove legal hold + // Remove legal hold (must specify version ID since latest version is now delete marker) _, err = client.PutObjectLegalHold(context.TODO(), &s3.PutObjectLegalHoldInput{ - Bucket: aws.String(bucketName), - Key: aws.String(key), + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, LegalHold: &types.ObjectLockLegalHold{ Status: types.ObjectLockLegalHoldStatusOff, }, }) require.NoError(t, err) - // Now delete with bypass governance should succeed + // Now DELETE with version ID and bypass governance should succeed _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(key), + VersionId: putResp.VersionId, BypassGovernanceRetention: aws.Bool(true), }) - require.NoError(t, err) + require.NoError(t, err, "DELETE with version ID should succeed after legal hold removed and with governance bypass") } // TestExpiredRetention tests that objects can be deleted after retention expires diff --git a/test/s3/retention/s3_worm_integration_test.go b/test/s3/retention/s3_worm_integration_test.go index e43510751..19010092c 100644 --- a/test/s3/retention/s3_worm_integration_test.go +++ b/test/s3/retention/s3_worm_integration_test.go @@ -42,17 +42,26 @@ func TestWORMRetentionIntegration(t *testing.T) { }) require.NoError(t, err) - // Try to delete - should fail due to retention + // Try simple DELETE - should succeed and create delete marker (AWS S3 behavior) _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(key), }) - require.Error(t, err) + require.NoError(t, err, "Simple DELETE should succeed and create delete marker") - // Delete with bypass should succeed + // Try DELETE with version ID - should fail due to GOVERNANCE retention + _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: putResp.VersionId, + }) + require.Error(t, err, "DELETE with version ID should be blocked by GOVERNANCE retention") + + // Delete with version ID and bypass should succeed _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(key), + VersionId: putResp.VersionId, BypassGovernanceRetention: aws.Bool(true), }) require.NoError(t, err) @@ -316,12 +325,20 @@ func TestRetentionWithMultipartUpload(t *testing.T) { }) require.NoError(t, err) - // Try to delete - should fail + // Try simple DELETE - should succeed and create delete marker (AWS S3 behavior) _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ Bucket: aws.String(bucketName), Key: aws.String(key), }) - require.Error(t, err) + require.NoError(t, err, "Simple DELETE should succeed and create delete marker") + + // Try DELETE with version ID - should fail due to GOVERNANCE retention + _, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(key), + VersionId: completeResp.VersionId, + }) + require.Error(t, err, "DELETE with version ID should be blocked by GOVERNANCE retention") } // TestRetentionExtendedAttributes tests that retention uses extended attributes correctly diff --git a/weed/s3api/filer_multipart.go b/weed/s3api/filer_multipart.go index d517c188b..05d167333 100644 --- a/weed/s3api/filer_multipart.go +++ b/weed/s3api/filer_multipart.go @@ -21,6 +21,8 @@ import ( "github.com/google/uuid" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + "net/http" + "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" @@ -36,7 +38,7 @@ type InitiateMultipartUploadResult struct { s3.CreateMultipartUploadOutput } -func (s3a *S3ApiServer) createMultipartUpload(input *s3.CreateMultipartUploadInput) (output *InitiateMultipartUploadResult, code s3err.ErrorCode) { +func (s3a *S3ApiServer) createMultipartUpload(r *http.Request, input *s3.CreateMultipartUploadInput) (output *InitiateMultipartUploadResult, code s3err.ErrorCode) { glog.V(2).Infof("createMultipartUpload input %v", input) @@ -55,6 +57,13 @@ func (s3a *S3ApiServer) createMultipartUpload(input *s3.CreateMultipartUploadInp if input.ContentType != nil { entry.Attributes.Mime = *input.ContentType } + + // Extract and store object lock metadata from request headers + // This ensures object lock settings from create_multipart_upload are preserved + if err := s3a.extractObjectLockMetadataFromRequest(r, entry); err != nil { + glog.Errorf("createMultipartUpload: failed to extract object lock metadata: %v", err) + // Don't fail the upload - this matches AWS behavior for invalid metadata + } }); err != nil { glog.Errorf("NewMultipartUpload error: %v", err) return nil, s3err.ErrInternalError @@ -72,8 +81,15 @@ func (s3a *S3ApiServer) createMultipartUpload(input *s3.CreateMultipartUploadInp } type CompleteMultipartUploadResult struct { - XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ CompleteMultipartUploadResult"` - s3.CompleteMultipartUploadOutput + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ CompleteMultipartUploadResult"` + Location *string `xml:"Location,omitempty"` + Bucket *string `xml:"Bucket,omitempty"` + Key *string `xml:"Key,omitempty"` + ETag *string `xml:"ETag,omitempty"` + // VersionId is NOT included in XML body - it should only be in x-amz-version-id HTTP header + + // Store the VersionId internally for setting HTTP header, but don't marshal to XML + VersionId *string `xml:"-"` } func (s3a *S3ApiServer) completeMultipartUpload(input *s3.CompleteMultipartUploadInput, parts *CompleteMultipartUpload) (output *CompleteMultipartUploadResult, code s3err.ErrorCode) { @@ -110,12 +126,10 @@ func (s3a *S3ApiServer) completeMultipartUpload(input *s3.CompleteMultipartUploa if entry, _ := s3a.getEntry(dirName, entryName); entry != nil && entry.Extended != nil { if uploadId, ok := entry.Extended[s3_constants.SeaweedFSUploadId]; ok && *input.UploadId == string(uploadId) { return &CompleteMultipartUploadResult{ - CompleteMultipartUploadOutput: s3.CompleteMultipartUploadOutput{ - Location: aws.String(fmt.Sprintf("http://%s%s/%s", s3a.option.Filer.ToHttpAddress(), urlEscapeObject(dirName), urlPathEscape(entryName))), - Bucket: input.Bucket, - ETag: aws.String("\"" + filer.ETagChunks(entry.GetChunks()) + "\""), - Key: objectKey(input.Key), - }, + Location: aws.String(fmt.Sprintf("http://%s%s/%s", s3a.option.Filer.ToHttpAddress(), urlEscapeObject(dirName), urlPathEscape(entryName))), + Bucket: input.Bucket, + ETag: aws.String("\"" + filer.ETagChunks(entry.GetChunks()) + "\""), + Key: objectKey(input.Key), }, s3err.ErrNone } } @@ -247,13 +261,75 @@ func (s3a *S3ApiServer) completeMultipartUpload(input *s3.CompleteMultipartUploa return nil, s3err.ErrInternalError } - output = &CompleteMultipartUploadResult{ - CompleteMultipartUploadOutput: s3.CompleteMultipartUploadOutput{ + // Check if versioning is enabled for this bucket + versioningEnabled, vErr := s3a.isVersioningEnabled(*input.Bucket) + if vErr == nil && versioningEnabled { + // For versioned buckets, create a version and return the version ID + versionId := generateVersionId() + versionFileName := s3a.getVersionFileName(versionId) + versionDir := dirName + "/" + entryName + ".versions" + + // Move the completed object to the versions directory + err = s3a.mkFile(versionDir, versionFileName, finalParts, func(versionEntry *filer_pb.Entry) { + if versionEntry.Extended == nil { + versionEntry.Extended = make(map[string][]byte) + } + versionEntry.Extended[s3_constants.ExtVersionIdKey] = []byte(versionId) + versionEntry.Extended[s3_constants.SeaweedFSUploadId] = []byte(*input.UploadId) + for k, v := range pentry.Extended { + if k != "key" { + versionEntry.Extended[k] = v + } + } + if pentry.Attributes.Mime != "" { + versionEntry.Attributes.Mime = pentry.Attributes.Mime + } else if mime != "" { + versionEntry.Attributes.Mime = mime + } + versionEntry.Attributes.FileSize = uint64(offset) + }) + + if err != nil { + glog.Errorf("completeMultipartUpload: failed to create version %s: %v", versionId, err) + return nil, s3err.ErrInternalError + } + + // Update the .versions directory metadata to indicate this is the latest version + err = s3a.updateLatestVersionInDirectory(*input.Bucket, *input.Key, versionId, versionFileName) + if err != nil { + glog.Errorf("completeMultipartUpload: failed to update latest version in directory: %v", err) + return nil, s3err.ErrInternalError + } + + // Create a delete marker for the main object (latest version) + err = s3a.mkFile(dirName, entryName, nil, func(mainEntry *filer_pb.Entry) { + if mainEntry.Extended == nil { + mainEntry.Extended = make(map[string][]byte) + } + mainEntry.Extended[s3_constants.ExtVersionIdKey] = []byte(versionId) + mainEntry.Extended[s3_constants.ExtDeleteMarkerKey] = []byte("false") // This is the latest version, not a delete marker + }) + + if err != nil { + glog.Errorf("completeMultipartUpload: failed to update main entry: %v", err) + return nil, s3err.ErrInternalError + } + + output = &CompleteMultipartUploadResult{ + Location: aws.String(fmt.Sprintf("http://%s%s/%s", s3a.option.Filer.ToHttpAddress(), urlEscapeObject(dirName), urlPathEscape(entryName))), + Bucket: input.Bucket, + ETag: aws.String("\"" + filer.ETagChunks(finalParts) + "\""), + Key: objectKey(input.Key), + VersionId: aws.String(versionId), + } + } else { + // For non-versioned buckets, return response without VersionId + output = &CompleteMultipartUploadResult{ Location: aws.String(fmt.Sprintf("http://%s%s/%s", s3a.option.Filer.ToHttpAddress(), urlEscapeObject(dirName), urlPathEscape(entryName))), Bucket: input.Bucket, ETag: aws.String("\"" + filer.ETagChunks(finalParts) + "\""), Key: objectKey(input.Key), - }, + } } for _, deleteEntry := range deleteEntries { diff --git a/weed/s3api/object_lock_utils.go b/weed/s3api/object_lock_utils.go index ffde5bd36..e207ca86d 100644 --- a/weed/s3api/object_lock_utils.go +++ b/weed/s3api/object_lock_utils.go @@ -59,9 +59,11 @@ func CreateObjectLockConfiguration(enabled bool, mode string, days int, years in if mode != "" && (days > 0 || years > 0) { config.Rule = &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: mode, - Days: days, - Years: years, + Mode: mode, + Days: days, + Years: years, + DaysSet: days > 0, + YearsSet: years > 0, }, } } @@ -106,12 +108,12 @@ func StoreObjectLockConfigurationInExtended(entry *filer_pb.Entry, config *Objec } // Store days - if defaultRetention.Days > 0 { + if defaultRetention.DaysSet && defaultRetention.Days > 0 { entry.Extended[s3_constants.ExtObjectLockDefaultDaysKey] = []byte(strconv.Itoa(defaultRetention.Days)) } // Store years - if defaultRetention.Years > 0 { + if defaultRetention.YearsSet && defaultRetention.Years > 0 { entry.Extended[s3_constants.ExtObjectLockDefaultYearsKey] = []byte(strconv.Itoa(defaultRetention.Years)) } } else { @@ -167,9 +169,11 @@ func LoadObjectLockConfigurationFromExtended(entry *filer_pb.Entry) (*ObjectLock if mode != "" && (days > 0 || years > 0) { config.Rule = &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: mode, - Days: days, - Years: years, + Mode: mode, + Days: days, + Years: years, + DaysSet: days > 0, + YearsSet: years > 0, }, } } @@ -192,8 +196,11 @@ func ExtractObjectLockInfoFromConfig(config *ObjectLockConfiguration) (bool, str defaultRetention := config.Rule.DefaultRetention // Convert years to days for consistent representation - days := defaultRetention.Days - if defaultRetention.Years > 0 { + days := 0 + if defaultRetention.DaysSet { + days = defaultRetention.Days + } + if defaultRetention.YearsSet && defaultRetention.Years > 0 { days += defaultRetention.Years * 365 } diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go index 725ee3596..41e750e5c 100644 --- a/weed/s3api/s3api_bucket_config.go +++ b/weed/s3api/s3api_bucket_config.go @@ -214,7 +214,9 @@ func (s3a *S3ApiServer) isVersioningEnabled(bucket string) (bool, error) { return false, fmt.Errorf("failed to get bucket config: %v", errCode) } - return config.Versioning == "Enabled", nil + // Versioning is enabled if explicitly set to "Enabled" OR if object lock is enabled + // (since object lock requires versioning to be enabled) + return config.Versioning == s3_constants.VersioningEnabled || config.ObjectLockConfig != nil, nil } // getBucketVersioningStatus returns the versioning status for a bucket @@ -225,7 +227,7 @@ func (s3a *S3ApiServer) getBucketVersioningStatus(bucket string) (string, s3err. } if config.Versioning == "" { - return "Suspended", s3err.ErrNone + return s3_constants.VersioningSuspended, s3err.ErrNone } return config.Versioning, s3err.ErrNone diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index e30f172a7..591aaafb3 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -753,11 +753,22 @@ func (s3a *S3ApiServer) PutBucketVersioningHandler(w http.ResponseWriter, r *htt } status := *versioningConfig.Status - if status != "Enabled" && status != "Suspended" { + if status != s3_constants.VersioningEnabled && status != s3_constants.VersioningSuspended { s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) return } + // Check if trying to suspend versioning on a bucket with object lock enabled + if status == s3_constants.VersioningSuspended { + // Get bucket configuration to check for object lock + bucketConfig, errCode := s3a.getBucketConfig(bucket) + if errCode == s3err.ErrNone && bucketConfig.ObjectLockConfig != nil { + // Object lock is enabled, cannot suspend versioning + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidBucketState) + return + } + } + // Update bucket versioning configuration using new bucket config system if errCode := s3a.setBucketVersioningStatus(bucket, status); errCode != s3err.ErrNone { glog.Errorf("PutBucketVersioningHandler save config: %d", errCode) diff --git a/weed/s3api/s3api_bucket_handlers_object_lock_config.go b/weed/s3api/s3api_bucket_handlers_object_lock_config.go index 494f203a4..9ff12a086 100644 --- a/weed/s3api/s3api_bucket_handlers_object_lock_config.go +++ b/weed/s3api/s3api_bucket_handlers_object_lock_config.go @@ -4,6 +4,8 @@ import ( "encoding/xml" "net/http" + "errors" + "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" @@ -17,7 +19,16 @@ func (s3a *S3ApiServer) PutObjectLockConfigurationHandler(w http.ResponseWriter, glog.V(3).Infof("PutObjectLockConfigurationHandler %s", bucket) // Check if Object Lock is available for this bucket (requires versioning) - if !s3a.handleObjectLockAvailabilityCheck(w, r, bucket, "PutObjectLockConfigurationHandler") { + // For bucket-level operations, return InvalidBucketState (409) when object lock is not available + if err := s3a.isObjectLockAvailable(bucket); err != nil { + glog.Errorf("PutObjectLockConfigurationHandler: object lock not available for bucket %s: %v", bucket, err) + if errors.Is(err, ErrBucketNotFound) { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) + } else { + // Return InvalidBucketState for bucket-level object lock operations on buckets without object lock enabled + // This matches AWS S3 behavior and s3-tests expectations (409 Conflict) + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidBucketState) + } return } @@ -32,7 +43,7 @@ func (s3a *S3ApiServer) PutObjectLockConfigurationHandler(w http.ResponseWriter, // Validate object lock configuration if err := validateObjectLockConfiguration(config); err != nil { glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock config: %v", err) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err)) return } @@ -113,7 +124,7 @@ func (s3a *S3ApiServer) GetObjectLockConfigurationHandler(w http.ResponseWriter, // If no Object Lock configuration found, return error if len(configXML) == 0 { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchObjectLockConfiguration) + s3err.WriteErrorResponse(w, r, s3err.ErrObjectLockConfigurationNotFoundError) return } diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index 05c93a913..b2d9c51c9 100644 --- a/weed/s3api/s3api_object_handlers_delete.go +++ b/weed/s3api/s3api_object_handlers_delete.go @@ -49,19 +49,17 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone) } - // Check object lock permissions before deletion (only for versioned buckets) - if versioningEnabled { - bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true" - if err := s3a.checkObjectLockPermissions(r, bucket, object, versionId, bypassGovernance); err != nil { - glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err) - s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) - return - } - } - if versioningEnabled { // Handle versioned delete if versionId != "" { + // Check object lock permissions before deleting specific version + governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object) + if err := s3a.enforceObjectLockProtections(r, bucket, object, versionId, governanceBypassAllowed); err != nil { + glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err) + s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) + return + } + // Delete specific version err := s3a.deleteSpecificObjectVersion(bucket, object, versionId) if err != nil { @@ -74,6 +72,8 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque w.Header().Set("x-amz-version-id", versionId) } else { // Create delete marker (logical delete) + // AWS S3 behavior: Delete marker creation is NOT blocked by object retention + // because it's a logical delete that doesn't actually remove the retained version deleteMarkerVersionId, err := s3a.createDeleteMarker(bucket, object) if err != nil { glog.Errorf("Failed to create delete marker: %v", err) @@ -87,6 +87,14 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque } } else { // Handle regular delete (non-versioned) + // Check object lock permissions before deleting object + governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object) + if err := s3a.enforceObjectLockProtections(r, bucket, object, "", governanceBypassAllowed); err != nil { + glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err) + s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) + return + } + target := util.FullPath(fmt.Sprintf("%s/%s%s", s3a.option.BucketsPath, bucket, object)) dir, name := target.DirAndName() @@ -193,9 +201,6 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone) } - // Check for bypass governance retention header - bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true" - // Check if versioning is enabled for the bucket (needed for object lock checks) versioningEnabled, err := s3a.isVersioningEnabled(bucket) if err != nil { @@ -218,7 +223,9 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h // Check object lock permissions before deletion (only for versioned buckets) if versioningEnabled { - if err := s3a.checkObjectLockPermissions(r, bucket, object.Key, object.VersionId, bypassGovernance); err != nil { + // Validate governance bypass for this specific object + governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key) + if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, object.VersionId, governanceBypassAllowed); err != nil { glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, object.VersionId, err) deleteErrors = append(deleteErrors, DeleteError{ Code: s3err.GetAPIError(s3err.ErrAccessDenied).Code, diff --git a/weed/s3api/s3api_object_handlers_legal_hold.go b/weed/s3api/s3api_object_handlers_legal_hold.go index 9cf523477..4010ff426 100644 --- a/weed/s3api/s3api_object_handlers_legal_hold.go +++ b/weed/s3api/s3api_object_handlers_legal_hold.go @@ -36,7 +36,7 @@ func (s3a *S3ApiServer) PutObjectLegalHoldHandler(w http.ResponseWriter, r *http // Validate legal hold configuration if err := validateLegalHold(legalHold); err != nil { glog.Errorf("PutObjectLegalHoldHandler: invalid legal hold config: %v", err) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err)) return } @@ -54,6 +54,11 @@ func (s3a *S3ApiServer) PutObjectLegalHoldHandler(w http.ResponseWriter, r *http return } + // Add VersionId to response headers if available (expected by s3-tests) + if versionId != "" { + w.Header().Set("x-amz-version-id", versionId) + } + // Record metrics stats_collect.RecordBucketActiveTime(bucket) diff --git a/weed/s3api/s3api_object_handlers_multipart.go b/weed/s3api/s3api_object_handlers_multipart.go index 6d0ebfa44..a6be3b4a1 100644 --- a/weed/s3api/s3api_object_handlers_multipart.go +++ b/weed/s3api/s3api_object_handlers_multipart.go @@ -14,6 +14,7 @@ import ( "github.com/aws/aws-sdk-go/service/s3" "github.com/google/uuid" "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" weed_server "github.com/seaweedfs/seaweedfs/weed/server" @@ -37,6 +38,25 @@ func (s3a *S3ApiServer) NewMultipartUploadHandler(w http.ResponseWriter, r *http return } + // Check if versioning is enabled for the bucket (needed for object lock) + versioningEnabled, err := s3a.isVersioningEnabled(bucket) + if err != nil { + if err == filer_pb.ErrNotFound { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) + return + } + glog.Errorf("Error checking versioning status for bucket %s: %v", bucket, err) + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + return + } + + // Validate object lock headers before processing + if err := s3a.validateObjectLockHeaders(r, versioningEnabled); err != nil { + glog.V(2).Infof("NewMultipartUploadHandler: object lock header validation failed for bucket %s, object %s: %v", bucket, object, err) + s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err)) + return + } + createMultipartUploadInput := &s3.CreateMultipartUploadInput{ Bucket: aws.String(bucket), Key: objectKey(aws.String(object)), @@ -52,9 +72,9 @@ func (s3a *S3ApiServer) NewMultipartUploadHandler(w http.ResponseWriter, r *http if contentType != "" { createMultipartUploadInput.ContentType = &contentType } - response, errCode := s3a.createMultipartUpload(createMultipartUploadInput) + response, errCode := s3a.createMultipartUpload(r, createMultipartUploadInput) - glog.V(2).Info("NewMultipartUploadHandler", string(s3err.EncodeXMLResponse(response)), errCode) + glog.V(3).Info("NewMultipartUploadHandler", string(s3err.EncodeXMLResponse(response)), errCode) if errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) @@ -97,14 +117,21 @@ func (s3a *S3ApiServer) CompleteMultipartUploadHandler(w http.ResponseWriter, r UploadId: aws.String(uploadID), }, parts) - glog.V(2).Info("CompleteMultipartUploadHandler", string(s3err.EncodeXMLResponse(response)), errCode) + glog.V(3).Info("CompleteMultipartUploadHandler", string(s3err.EncodeXMLResponse(response)), errCode) if errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } + + // Set version ID in HTTP header if present + if response.VersionId != nil { + w.Header().Set("x-amz-version-id", *response.VersionId) + } + stats_collect.RecordBucketActiveTime(bucket) stats_collect.S3UploadedObjectsCounter.WithLabelValues(bucket).Inc() + writeSuccessResponseXML(w, r, response) } @@ -138,7 +165,7 @@ func (s3a *S3ApiServer) AbortMultipartUploadHandler(w http.ResponseWriter, r *ht return } - glog.V(2).Info("AbortMultipartUploadHandler", string(s3err.EncodeXMLResponse(response))) + glog.V(3).Info("AbortMultipartUploadHandler", string(s3err.EncodeXMLResponse(response))) //https://docs.aws.amazon.com/AmazonS3/latest/API/API_AbortMultipartUpload.html s3err.WriteEmptyResponse(w, r, http.StatusNoContent) @@ -179,7 +206,7 @@ func (s3a *S3ApiServer) ListMultipartUploadsHandler(w http.ResponseWriter, r *ht UploadIdMarker: aws.String(uploadIDMarker), }) - glog.V(2).Infof("ListMultipartUploadsHandler %s errCode=%d", string(s3err.EncodeXMLResponse(response)), errCode) + glog.V(3).Infof("ListMultipartUploadsHandler %s errCode=%d", string(s3err.EncodeXMLResponse(response)), errCode) if errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) @@ -230,7 +257,7 @@ func (s3a *S3ApiServer) ListObjectPartsHandler(w http.ResponseWriter, r *http.Re return } - glog.V(2).Infof("ListObjectPartsHandler %s count=%d", string(s3err.EncodeXMLResponse(response)), len(response.Part)) + glog.V(3).Infof("ListObjectPartsHandler %s count=%d", string(s3err.EncodeXMLResponse(response)), len(response.Part)) writeSuccessResponseXML(w, r, response) diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 50d308566..011a039d3 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -23,16 +23,25 @@ import ( // Object lock validation errors var ( - ErrObjectLockVersioningRequired = errors.New("object lock headers can only be used on versioned buckets") - ErrInvalidObjectLockMode = errors.New("invalid object lock mode") - ErrInvalidLegalHoldStatus = errors.New("invalid legal hold status") - ErrInvalidRetentionDateFormat = errors.New("invalid retention until date format") - ErrRetentionDateMustBeFuture = errors.New("retention until date must be in the future") - ErrObjectLockModeRequiresDate = errors.New("object lock mode requires retention until date") - ErrRetentionDateRequiresMode = errors.New("retention until date requires object lock mode") - ErrGovernanceBypassVersioningRequired = errors.New("governance bypass header can only be used on versioned buckets") - ErrInvalidObjectLockDuration = errors.New("object lock duration must be greater than 0 days") - ErrObjectLockDurationExceeded = errors.New("object lock duration exceeds maximum allowed days") + ErrObjectLockVersioningRequired = errors.New("object lock headers can only be used on versioned buckets") + ErrInvalidObjectLockMode = errors.New("invalid object lock mode") + ErrInvalidLegalHoldStatus = errors.New("invalid legal hold status") + ErrInvalidRetentionDateFormat = errors.New("invalid retention until date format") + ErrRetentionDateMustBeFuture = errors.New("retain until date must be in the future") + ErrObjectLockModeRequiresDate = errors.New("object lock mode requires retention until date") + ErrRetentionDateRequiresMode = errors.New("retention until date requires object lock mode") + ErrGovernanceBypassVersioningRequired = errors.New("governance bypass header can only be used on versioned buckets") + ErrInvalidObjectLockDuration = errors.New("object lock duration must be greater than 0 days") + ErrObjectLockDurationExceeded = errors.New("object lock duration exceeds maximum allowed days") + ErrObjectLockConfigurationMissingEnabled = errors.New("object lock configuration must specify ObjectLockEnabled") + ErrInvalidObjectLockEnabledValue = errors.New("invalid object lock enabled value") + ErrRuleMissingDefaultRetention = errors.New("rule configuration must specify DefaultRetention") + ErrDefaultRetentionMissingMode = errors.New("default retention must specify Mode") + ErrInvalidDefaultRetentionMode = errors.New("invalid default retention mode") + ErrDefaultRetentionMissingPeriod = errors.New("default retention must specify either Days or Years") + ErrDefaultRetentionBothDaysAndYears = errors.New("default retention cannot specify both Days and Years") + ErrDefaultRetentionDaysOutOfRange = errors.New("default retention days must be between 0 and 36500") + ErrDefaultRetentionYearsOutOfRange = errors.New("default retention years must be between 0 and 100") ) func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) { @@ -110,8 +119,8 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) // For non-versioned buckets, check if existing object has object lock protections // that would prevent overwrite (PUT operations overwrite existing objects in non-versioned buckets) if !versioningEnabled { - bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true" - if err := s3a.checkObjectLockPermissions(r, bucket, object, "", bypassGovernance); err != nil { + governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object) + if err := s3a.enforceObjectLockProtections(r, bucket, object, "", governanceBypassAllowed); err != nil { glog.V(2).Infof("PutObjectHandler: object lock permissions check failed for %s/%s: %v", bucket, object, err) s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) return @@ -460,7 +469,7 @@ func (s3a *S3ApiServer) applyBucketDefaultRetention(bucket string, entry *filer_ return fmt.Errorf("default retention missing mode") } - if defaultRetention.Days == 0 && defaultRetention.Years == 0 { + if !defaultRetention.DaysSet && !defaultRetention.YearsSet { return fmt.Errorf("default retention missing period") } @@ -468,9 +477,9 @@ func (s3a *S3ApiServer) applyBucketDefaultRetention(bucket string, entry *filer_ var retainUntilDate time.Time now := time.Now() - if defaultRetention.Days > 0 { + if defaultRetention.DaysSet && defaultRetention.Days > 0 { retainUntilDate = now.AddDate(0, 0, defaultRetention.Days) - } else if defaultRetention.Years > 0 { + } else if defaultRetention.YearsSet && defaultRetention.Years > 0 { retainUntilDate = now.AddDate(defaultRetention.Years, 0, 0) } @@ -553,26 +562,94 @@ func (s3a *S3ApiServer) validateObjectLockHeaders(r *http.Request, versioningEna // mapValidationErrorToS3Error maps object lock validation errors to appropriate S3 error codes func mapValidationErrorToS3Error(err error) s3err.ErrorCode { + // Check for sentinel errors first switch { case errors.Is(err, ErrObjectLockVersioningRequired): + // For object lock operations on non-versioned buckets, return InvalidRequest + // This matches the test expectations return s3err.ErrInvalidRequest case errors.Is(err, ErrInvalidObjectLockMode): + // For invalid object lock mode, return InvalidRequest + // This matches the test expectations return s3err.ErrInvalidRequest case errors.Is(err, ErrInvalidLegalHoldStatus): - return s3err.ErrInvalidRequest + // For invalid legal hold status in XML body, return MalformedXML + // AWS S3 treats invalid status values in XML as malformed content + return s3err.ErrMalformedXML case errors.Is(err, ErrInvalidRetentionDateFormat): + // For malformed retention date format, return MalformedDate + // This matches the test expectations return s3err.ErrMalformedDate - case errors.Is(err, ErrRetentionDateMustBeFuture), - errors.Is(err, ErrObjectLockModeRequiresDate), - errors.Is(err, ErrRetentionDateRequiresMode): + case errors.Is(err, ErrRetentionDateMustBeFuture): + // For retention dates in the past, return InvalidRequest + // This matches the test expectations return s3err.ErrInvalidRequest - case errors.Is(err, ErrGovernanceBypassVersioningRequired): + case errors.Is(err, ErrObjectLockModeRequiresDate): + // For mode without retention date, return InvalidRequest + // This matches the test expectations return s3err.ErrInvalidRequest - case errors.Is(err, ErrInvalidObjectLockDuration): + case errors.Is(err, ErrRetentionDateRequiresMode): + // For retention date without mode, return InvalidRequest + // This matches the test expectations return s3err.ErrInvalidRequest - case errors.Is(err, ErrObjectLockDurationExceeded): + case errors.Is(err, ErrGovernanceBypassVersioningRequired): + // For governance bypass on non-versioned bucket, return InvalidRequest + // This matches the test expectations return s3err.ErrInvalidRequest - default: + case errors.Is(err, ErrMalformedXML): + // For malformed XML in request body, return MalformedXML + // This matches the test expectations for invalid retention mode and legal hold status + return s3err.ErrMalformedXML + case errors.Is(err, ErrInvalidRetentionPeriod): + // For invalid retention period (e.g., Days <= 0), return InvalidRetentionPeriod + // This matches the test expectations + return s3err.ErrInvalidRetentionPeriod + case errors.Is(err, ErrComplianceModeActive): + // For compliance mode retention violations, return AccessDenied + // This matches the test expectations + return s3err.ErrAccessDenied + case errors.Is(err, ErrGovernanceModeActive): + // For governance mode retention violations, return AccessDenied + // This matches the test expectations + return s3err.ErrAccessDenied + case errors.Is(err, ErrObjectUnderLegalHold): + // For legal hold violations, return AccessDenied + // This matches the test expectations + return s3err.ErrAccessDenied + case errors.Is(err, ErrGovernanceBypassNotPermitted): + // For governance bypass permission violations, return AccessDenied + // This matches the test expectations + return s3err.ErrAccessDenied + // Validation error constants + case errors.Is(err, ErrObjectLockConfigurationMissingEnabled): + return s3err.ErrMalformedXML + case errors.Is(err, ErrInvalidObjectLockEnabledValue): + return s3err.ErrMalformedXML + case errors.Is(err, ErrRuleMissingDefaultRetention): + return s3err.ErrMalformedXML + case errors.Is(err, ErrDefaultRetentionMissingMode): + return s3err.ErrMalformedXML + case errors.Is(err, ErrInvalidDefaultRetentionMode): + return s3err.ErrMalformedXML + case errors.Is(err, ErrDefaultRetentionMissingPeriod): + return s3err.ErrMalformedXML + case errors.Is(err, ErrDefaultRetentionBothDaysAndYears): + return s3err.ErrMalformedXML + case errors.Is(err, ErrDefaultRetentionDaysOutOfRange): + return s3err.ErrInvalidRetentionPeriod + case errors.Is(err, ErrDefaultRetentionYearsOutOfRange): + return s3err.ErrInvalidRetentionPeriod + } + + // Check for error constants from the updated validation functions + switch { + case errors.Is(err, ErrRetentionMissingMode): + return s3err.ErrInvalidRequest + case errors.Is(err, ErrRetentionMissingRetainUntilDate): return s3err.ErrInvalidRequest + case errors.Is(err, ErrInvalidRetentionModeValue): + return s3err.ErrMalformedXML } + + return s3err.ErrInvalidRequest } diff --git a/weed/s3api/s3api_object_handlers_retention.go b/weed/s3api/s3api_object_handlers_retention.go index a419b469e..899c2453c 100644 --- a/weed/s3api/s3api_object_handlers_retention.go +++ b/weed/s3api/s3api_object_handlers_retention.go @@ -25,8 +25,8 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http // Get version ID from query parameters versionId := r.URL.Query().Get("versionId") - // Check for bypass governance retention header - bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true" + // Evaluate governance bypass request (header + permission validation) + governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object) // Parse retention configuration from request body retention, err := parseObjectRetention(r) @@ -39,12 +39,12 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http // Validate retention configuration if err := validateRetention(retention); err != nil { glog.Errorf("PutObjectRetentionHandler: invalid retention config: %v", err) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err)) return } // Set retention on the object - if err := s3a.setObjectRetention(bucket, object, versionId, retention, bypassGovernance); err != nil { + if err := s3a.setObjectRetention(bucket, object, versionId, retention, governanceBypassAllowed); err != nil { glog.Errorf("PutObjectRetentionHandler: failed to set retention: %v", err) // Handle specific error cases @@ -54,6 +54,7 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http } if errors.Is(err, ErrComplianceModeActive) || errors.Is(err, ErrGovernanceModeActive) { + // Return 403 Forbidden for retention mode changes without proper permissions s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) return } @@ -62,6 +63,11 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http return } + // Add VersionId to response headers if available (expected by s3-tests) + if versionId != "" { + w.Header().Set("x-amz-version-id", versionId) + } + // Record metrics stats_collect.RecordBucketActiveTime(bucket) @@ -96,7 +102,7 @@ func (s3a *S3ApiServer) GetObjectRetentionHandler(w http.ResponseWriter, r *http } if errors.Is(err, ErrNoRetentionConfiguration) { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchObjectLockConfiguration) + s3err.WriteErrorResponse(w, r, s3err.ErrObjectLockConfigurationNotFoundError) return } diff --git a/weed/s3api/s3api_object_lock_headers_test.go b/weed/s3api/s3api_object_lock_headers_test.go index 111aa0fa9..fc8a01232 100644 --- a/weed/s3api/s3api_object_lock_headers_test.go +++ b/weed/s3api/s3api_object_lock_headers_test.go @@ -568,7 +568,7 @@ func TestMapValidationErrorToS3Error(t *testing.T) { { name: "ErrInvalidLegalHoldStatus", inputError: ErrInvalidLegalHoldStatus, - expectedCode: s3err.ErrInvalidRequest, + expectedCode: s3err.ErrMalformedXML, }, { name: "ErrInvalidRetentionDateFormat", diff --git a/weed/s3api/s3api_object_retention.go b/weed/s3api/s3api_object_retention.go index 88a5d1261..49092ef3e 100644 --- a/weed/s3api/s3api_object_retention.go +++ b/weed/s3api/s3api_object_retention.go @@ -31,6 +31,14 @@ var ( var ( ErrObjectUnderLegalHold = errors.New("object is under legal hold and cannot be deleted or modified") ErrGovernanceBypassNotPermitted = errors.New("user does not have permission to bypass governance retention") + ErrInvalidRetentionPeriod = errors.New("invalid retention period specified") + ErrBothDaysAndYearsSpecified = errors.New("both days and years cannot be specified in the same retention configuration") + ErrMalformedXML = errors.New("malformed XML in request body") + + // Validation error constants with specific messages for tests + ErrRetentionMissingMode = errors.New("retention configuration must specify Mode") + ErrRetentionMissingRetainUntilDate = errors.New("retention configuration must specify RetainUntilDate") + ErrInvalidRetentionModeValue = errors.New("invalid retention mode") ) const ( @@ -42,59 +50,66 @@ const ( // ObjectRetention represents S3 Object Retention configuration type ObjectRetention struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Retention"` - Mode string `xml:"Mode,omitempty"` - RetainUntilDate *time.Time `xml:"RetainUntilDate,omitempty"` + Mode string `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Mode,omitempty"` + RetainUntilDate *time.Time `xml:"http://s3.amazonaws.com/doc/2006-03-01/ RetainUntilDate,omitempty"` } // ObjectLegalHold represents S3 Object Legal Hold configuration type ObjectLegalHold struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ LegalHold"` - Status string `xml:"Status,omitempty"` + Status string `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Status,omitempty"` } // ObjectLockConfiguration represents S3 Object Lock Configuration type ObjectLockConfiguration struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ObjectLockConfiguration"` - ObjectLockEnabled string `xml:"ObjectLockEnabled,omitempty"` - Rule *ObjectLockRule `xml:"Rule,omitempty"` + ObjectLockEnabled string `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ObjectLockEnabled,omitempty"` + Rule *ObjectLockRule `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Rule,omitempty"` } // ObjectLockRule represents an Object Lock Rule type ObjectLockRule struct { - XMLName xml.Name `xml:"Rule"` - DefaultRetention *DefaultRetention `xml:"DefaultRetention,omitempty"` + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Rule"` + DefaultRetention *DefaultRetention `xml:"http://s3.amazonaws.com/doc/2006-03-01/ DefaultRetention,omitempty"` } // DefaultRetention represents default retention settings +// Implements custom XML unmarshal to track if Days/Years were present in XML + type DefaultRetention struct { - XMLName xml.Name `xml:"DefaultRetention"` - Mode string `xml:"Mode,omitempty"` - Days int `xml:"Days,omitempty"` - Years int `xml:"Years,omitempty"` + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ DefaultRetention"` + Mode string `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Mode,omitempty"` + Days int `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Days,omitempty"` + Years int `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Years,omitempty"` + DaysSet bool `xml:"-"` + YearsSet bool `xml:"-"` } -// Custom time unmarshalling for AWS S3 ISO8601 format -func (or *ObjectRetention) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { - type Alias ObjectRetention +func (dr *DefaultRetention) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + type Alias DefaultRetention aux := &struct { *Alias - RetainUntilDate *string `xml:"RetainUntilDate,omitempty"` - }{ - Alias: (*Alias)(or), - } - + Days *int `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Days,omitempty"` + Years *int `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Years,omitempty"` + }{Alias: (*Alias)(dr)} if err := d.DecodeElement(aux, &start); err != nil { + glog.V(2).Infof("DefaultRetention.UnmarshalXML: decode error: %v", err) return err } - - if aux.RetainUntilDate != nil { - t, err := time.Parse(time.RFC3339, *aux.RetainUntilDate) - if err != nil { - return err - } - or.RetainUntilDate = &t + if aux.Days != nil { + dr.Days = *aux.Days + dr.DaysSet = true + glog.V(4).Infof("DefaultRetention.UnmarshalXML: Days present, value=%d", dr.Days) + } else { + glog.V(4).Infof("DefaultRetention.UnmarshalXML: Days not present") + } + if aux.Years != nil { + dr.Years = *aux.Years + dr.YearsSet = true + glog.V(4).Infof("DefaultRetention.UnmarshalXML: Years present, value=%d", dr.Years) + } else { + glog.V(4).Infof("DefaultRetention.UnmarshalXML: Years not present") } - return nil } @@ -153,21 +168,24 @@ func parseObjectLockConfiguration(request *http.Request) (*ObjectLockConfigurati // validateRetention validates retention configuration func validateRetention(retention *ObjectRetention) error { - // AWS requires both Mode and RetainUntilDate for PutObjectRetention + // Check if mode is specified if retention.Mode == "" { - return fmt.Errorf("retention configuration must specify Mode") + return ErrRetentionMissingMode } + // Check if retain until date is specified if retention.RetainUntilDate == nil { - return fmt.Errorf("retention configuration must specify RetainUntilDate") + return ErrRetentionMissingRetainUntilDate } + // Check if mode is valid if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance { - return fmt.Errorf("invalid retention mode: %s", retention.Mode) + return ErrInvalidRetentionModeValue } + // Check if retain until date is in the future if retention.RetainUntilDate.Before(time.Now()) { - return fmt.Errorf("retain until date must be in the future") + return ErrRetentionDateMustBeFuture } return nil @@ -175,8 +193,9 @@ func validateRetention(retention *ObjectRetention) error { // validateLegalHold validates legal hold configuration func validateLegalHold(legalHold *ObjectLegalHold) error { + // Check if status is valid if legalHold.Status != s3_constants.LegalHoldOn && legalHold.Status != s3_constants.LegalHoldOff { - return fmt.Errorf("invalid legal hold status: %s", legalHold.Status) + return ErrInvalidLegalHoldStatus } return nil @@ -186,18 +205,19 @@ func validateLegalHold(legalHold *ObjectLegalHold) error { func validateObjectLockConfiguration(config *ObjectLockConfiguration) error { // ObjectLockEnabled is required for bucket-level configuration if config.ObjectLockEnabled == "" { - return fmt.Errorf("object lock configuration must specify ObjectLockEnabled") + return ErrObjectLockConfigurationMissingEnabled } // Validate ObjectLockEnabled value if config.ObjectLockEnabled != s3_constants.ObjectLockEnabled { - return fmt.Errorf("invalid object lock enabled value: %s", config.ObjectLockEnabled) + // ObjectLockEnabled can only be 'Enabled', any other value (including 'Disabled') is malformed XML + return ErrInvalidObjectLockEnabledValue } // Validate Rule if present if config.Rule != nil { if config.Rule.DefaultRetention == nil { - return fmt.Errorf("rule configuration must specify DefaultRetention") + return ErrRuleMissingDefaultRetention } return validateDefaultRetention(config.Rule.DefaultRetention) } @@ -207,34 +227,47 @@ func validateObjectLockConfiguration(config *ObjectLockConfiguration) error { // validateDefaultRetention validates default retention configuration func validateDefaultRetention(retention *DefaultRetention) error { + glog.V(2).Infof("validateDefaultRetention: Mode=%s, Days=%d (set=%v), Years=%d (set=%v)", retention.Mode, retention.Days, retention.DaysSet, retention.Years, retention.YearsSet) // Mode is required if retention.Mode == "" { - return fmt.Errorf("default retention must specify Mode") + return ErrDefaultRetentionMissingMode } - // Mode must be valid if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance { - return fmt.Errorf("invalid default retention mode: %s", retention.Mode) + return ErrInvalidDefaultRetentionMode } - - // Exactly one of Days or Years must be specified - if retention.Days == 0 && retention.Years == 0 { - return fmt.Errorf("default retention must specify either Days or Years") + // Check for invalid Years value (negative values are always invalid) + if retention.YearsSet && retention.Years < 0 { + return ErrInvalidRetentionPeriod } - - if retention.Days > 0 && retention.Years > 0 { - return fmt.Errorf("default retention cannot specify both Days and Years") + // Check for invalid Days value (negative values are invalid) + if retention.DaysSet && retention.Days < 0 { + return ErrInvalidRetentionPeriod } - - // Validate ranges - if retention.Days < 0 || retention.Days > MaxRetentionDays { - return fmt.Errorf("default retention days must be between 0 and %d", MaxRetentionDays) + // Check for invalid Days value (zero is invalid when explicitly provided) + if retention.DaysSet && retention.Days == 0 { + return ErrInvalidRetentionPeriod } - - if retention.Years < 0 || retention.Years > MaxRetentionYears { - return fmt.Errorf("default retention years must be between 0 and %d", MaxRetentionYears) + // Check for neither Days nor Years being specified + if !retention.DaysSet && !retention.YearsSet { + return ErrDefaultRetentionMissingPeriod + } + // Check for both Days and Years being specified + if retention.DaysSet && retention.YearsSet && retention.Days > 0 && retention.Years > 0 { + return ErrDefaultRetentionBothDaysAndYears + } + // Validate Days if specified + if retention.DaysSet && retention.Days > 0 { + if retention.Days > MaxRetentionDays { + return ErrDefaultRetentionDaysOutOfRange + } + } + // Validate Years if specified + if retention.YearsSet && retention.Years > 0 { + if retention.Years > MaxRetentionYears { + return ErrDefaultRetentionYearsOutOfRange + } } - return nil } @@ -344,16 +377,40 @@ func (s3a *S3ApiServer) setObjectRetention(bucket, object, versionId string, ret // Check if object is already under retention if entry.Extended != nil { if existingMode, exists := entry.Extended[s3_constants.ExtObjectLockModeKey]; exists { - if string(existingMode) == s3_constants.RetentionModeCompliance && !bypassGovernance { - return fmt.Errorf("cannot modify retention on object under COMPLIANCE mode") + // Check if attempting to change retention mode + if retention.Mode != "" && string(existingMode) != retention.Mode { + // Attempting to change retention mode + if string(existingMode) == s3_constants.RetentionModeCompliance { + // Cannot change compliance mode retention without bypass + return ErrComplianceModeActive + } + + if string(existingMode) == s3_constants.RetentionModeGovernance && !bypassGovernance { + // Cannot change governance mode retention without bypass + return ErrGovernanceModeActive + } } if existingDateBytes, dateExists := entry.Extended[s3_constants.ExtRetentionUntilDateKey]; dateExists { if timestamp, err := strconv.ParseInt(string(existingDateBytes), 10, 64); err == nil { existingDate := time.Unix(timestamp, 0) - if existingDate.After(time.Now()) && string(existingMode) == s3_constants.RetentionModeGovernance && !bypassGovernance { - return fmt.Errorf("cannot modify retention on object under GOVERNANCE mode without bypass") + + // Check if the new retention date is earlier than the existing one + if retention.RetainUntilDate != nil && retention.RetainUntilDate.Before(existingDate) { + // Attempting to decrease retention period + if string(existingMode) == s3_constants.RetentionModeCompliance { + // Cannot decrease compliance mode retention without bypass + return ErrComplianceModeActive + } + + if string(existingMode) == s3_constants.RetentionModeGovernance && !bypassGovernance { + // Cannot decrease governance mode retention without bypass + return ErrGovernanceModeActive + } } + + // If new retention date is later or same, allow the operation + // This covers both increasing retention period and overriding with same/later date } } } @@ -490,38 +547,62 @@ func (s3a *S3ApiServer) isObjectRetentionActive(bucket, object, versionId string return false, nil } -// getObjectRetentionWithStatus retrieves retention configuration and returns both the data and active status -// This is an optimization to avoid duplicate fetches when both retention data and status are needed -func (s3a *S3ApiServer) getObjectRetentionWithStatus(bucket, object, versionId string) (*ObjectRetention, bool, error) { - retention, err := s3a.getObjectRetention(bucket, object, versionId) - if err != nil { - // If no retention found, object is not under retention - if errors.Is(err, ErrNoRetentionConfiguration) { - return nil, false, nil +// getRetentionFromEntry extracts retention configuration from an existing entry +func (s3a *S3ApiServer) getRetentionFromEntry(entry *filer_pb.Entry) (*ObjectRetention, bool, error) { + if entry.Extended == nil { + return nil, false, nil + } + + retention := &ObjectRetention{} + + if modeBytes, exists := entry.Extended[s3_constants.ExtObjectLockModeKey]; exists { + retention.Mode = string(modeBytes) + } + + if dateBytes, exists := entry.Extended[s3_constants.ExtRetentionUntilDateKey]; exists { + if timestamp, err := strconv.ParseInt(string(dateBytes), 10, 64); err == nil { + t := time.Unix(timestamp, 0) + retention.RetainUntilDate = &t + } else { + return nil, false, fmt.Errorf("failed to parse retention timestamp: corrupted timestamp data") } - return nil, false, err + } + + if retention.Mode == "" || retention.RetainUntilDate == nil { + return nil, false, nil } // Check if retention is currently active - isActive := retention.RetainUntilDate != nil && retention.RetainUntilDate.After(time.Now()) + isActive := retention.RetainUntilDate.After(time.Now()) return retention, isActive, nil } -// isObjectLegalHoldActive checks if an object is currently under legal hold -func (s3a *S3ApiServer) isObjectLegalHoldActive(bucket, object, versionId string) (bool, error) { - legalHold, err := s3a.getObjectLegalHold(bucket, object, versionId) - if err != nil { - // If no legal hold found, object is not under legal hold - if errors.Is(err, ErrNoLegalHoldConfiguration) { - return false, nil - } - return false, err +// getLegalHoldFromEntry extracts legal hold configuration from an existing entry +func (s3a *S3ApiServer) getLegalHoldFromEntry(entry *filer_pb.Entry) (*ObjectLegalHold, bool, error) { + if entry.Extended == nil { + return nil, false, nil + } + + legalHold := &ObjectLegalHold{} + + if statusBytes, exists := entry.Extended[s3_constants.ExtLegalHoldKey]; exists { + legalHold.Status = string(statusBytes) + } else { + return nil, false, nil } - return legalHold.Status == s3_constants.LegalHoldOn, nil + isActive := legalHold.Status == s3_constants.LegalHoldOn + return legalHold, isActive, nil } -// checkGovernanceBypassPermission checks if the user has permission to bypass governance retention +// checkGovernanceBypassPermission validates if the user has IAM permission to bypass governance retention. +// This is the low-level permission check that integrates with the IAM system. +// +// Returns true if: +// - User has s3:BypassGovernanceRetention permission for the resource, OR +// - User has Admin permissions for the resource +// +// This function does NOT check if the bypass header is present - that's handled separately. func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, bucket, object string) bool { // Use the existing IAM auth system to check the specific permission // Create the governance bypass action with proper bucket/object concatenation @@ -552,21 +633,86 @@ func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, b return false } -// checkObjectLockPermissions checks if an object can be deleted or modified -func (s3a *S3ApiServer) checkObjectLockPermissions(request *http.Request, bucket, object, versionId string, bypassGovernance bool) error { - // Get retention configuration and status in a single call to avoid duplicate fetches - retention, retentionActive, err := s3a.getObjectRetentionWithStatus(bucket, object, versionId) +// evaluateGovernanceBypassRequest determines if a governance bypass should be allowed. +// This is the high-level validation that combines header checking with permission validation. +// +// AWS S3 requires BOTH conditions: +// 1. Client sends x-amz-bypass-governance-retention: true header (intent) +// 2. User has s3:BypassGovernanceRetention IAM permission (authorization) +// +// Returns true only if both conditions are met. +// Used by all handlers that need to check governance bypass (DELETE, PUT, etc.). +func (s3a *S3ApiServer) evaluateGovernanceBypassRequest(r *http.Request, bucket, object string) bool { + // Step 1: Check if governance bypass was requested via header + bypassRequested := r.Header.Get("x-amz-bypass-governance-retention") == "true" + if !bypassRequested { + // No bypass requested - normal retention enforcement applies + return false + } + + // Step 2: Validate user has permission to bypass governance retention + hasPermission := s3a.checkGovernanceBypassPermission(r, bucket, object) + if !hasPermission { + glog.V(2).Infof("Governance bypass denied for %s/%s: user lacks s3:BypassGovernanceRetention permission", bucket, object) + return false + } + + glog.V(2).Infof("Governance bypass granted for %s/%s: header present and user has permission", bucket, object) + return true +} + +// enforceObjectLockProtections checks if an object operation should be blocked by object lock. +// This function enforces retention and legal hold policies based on pre-validated permissions. +// +// Parameters: +// - request: HTTP request (for logging/context only - permissions already validated) +// - bucket, object, versionId: Object identifier +// - governanceBypassAllowed: Pre-validated governance bypass permission (from evaluateGovernanceBypassRequest) +// +// Important: The governanceBypassAllowed parameter is TRUSTED - it should only be set to true +// if evaluateGovernanceBypassRequest() has already validated both header presence and IAM permissions. +// +// Returns error if operation should be blocked, nil if operation is allowed. +func (s3a *S3ApiServer) enforceObjectLockProtections(request *http.Request, bucket, object, versionId string, governanceBypassAllowed bool) error { + // Get the object entry to check both retention and legal hold + // For delete operations without versionId, we need to check the latest version + var entry *filer_pb.Entry + var err error + + if versionId != "" { + // Check specific version + entry, err = s3a.getObjectEntry(bucket, object, versionId) + } else { + // Check latest version for delete marker creation + entry, err = s3a.getObjectEntry(bucket, object, "") + } + if err != nil { - glog.Warningf("Error checking retention for %s/%s: %v", bucket, object, err) + // If object doesn't exist, it's not under retention or legal hold - this is expected during delete operations + if errors.Is(err, ErrObjectNotFound) || errors.Is(err, ErrVersionNotFound) || errors.Is(err, ErrLatestVersionNotFound) { + // Object doesn't exist, so it can't be under retention or legal hold - this is normal + glog.V(4).Infof("Object %s/%s (versionId: %s) not found during object lock check (expected during delete operations)", bucket, object, versionId) + return nil + } + glog.Warningf("Error retrieving object %s/%s (versionId: %s) for lock check: %v", bucket, object, versionId, err) + return err } - // Check if object is under legal hold - legalHoldActive, err := s3a.isObjectLegalHoldActive(bucket, object, versionId) + // Extract retention information from the entry + retention, retentionActive, err := s3a.getRetentionFromEntry(entry) if err != nil { - glog.Warningf("Error checking legal hold for %s/%s: %v", bucket, object, err) + glog.Warningf("Error parsing retention for %s/%s (versionId: %s): %v", bucket, object, versionId, err) + // Continue with legal hold check even if retention parsing fails } - // If object is under legal hold, it cannot be deleted or modified + // Extract legal hold information from the entry + _, legalHoldActive, err := s3a.getLegalHoldFromEntry(entry) + if err != nil { + glog.Warningf("Error parsing legal hold for %s/%s (versionId: %s): %v", bucket, object, versionId, err) + // Continue with retention check even if legal hold parsing fails + } + + // If object is under legal hold, it cannot be deleted or modified (including delete marker creation) if legalHoldActive { return ErrObjectUnderLegalHold } @@ -578,15 +724,11 @@ func (s3a *S3ApiServer) checkObjectLockPermissions(request *http.Request, bucket } if retention.Mode == s3_constants.RetentionModeGovernance { - if !bypassGovernance { + if !governanceBypassAllowed { return ErrGovernanceModeActive } - - // If bypass is requested, check if user has permission - if !s3a.checkGovernanceBypassPermission(request, bucket, object) { - glog.V(2).Infof("User does not have s3:BypassGovernanceRetention permission for %s/%s", bucket, object) - return ErrGovernanceBypassNotPermitted - } + // Note: governanceBypassAllowed parameter is already validated by evaluateGovernanceBypassRequest() + // which checks both header presence and IAM permissions, so we trust it here } } @@ -620,6 +762,8 @@ func (s3a *S3ApiServer) handleObjectLockAvailabilityCheck(w http.ResponseWriter, if errors.Is(err, ErrBucketNotFound) { s3err.WriteErrorResponse(w, request, s3err.ErrNoSuchBucket) } else { + // Return InvalidRequest for object lock operations on buckets without object lock enabled + // This matches AWS S3 behavior and s3-tests expectations (400 Bad Request) s3err.WriteErrorResponse(w, request, s3err.ErrInvalidRequest) } return false diff --git a/weed/s3api/s3api_object_retention_test.go b/weed/s3api/s3api_object_retention_test.go index 0caa50b42..638d37d14 100644 --- a/weed/s3api/s3api_object_retention_test.go +++ b/weed/s3api/s3api_object_retention_test.go @@ -498,8 +498,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) { ObjectLockEnabled: "Enabled", Rule: &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: "GOVERNANCE", - Days: 30, + Mode: "GOVERNANCE", + Days: 30, + DaysSet: true, }, }, }, @@ -511,8 +512,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) { ObjectLockEnabled: "Enabled", Rule: &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: "COMPLIANCE", - Years: 1, + Mode: "COMPLIANCE", + Years: 1, + YearsSet: true, }, }, }, @@ -545,9 +547,11 @@ func TestValidateObjectLockConfiguration(t *testing.T) { ObjectLockEnabled: "Enabled", Rule: &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: "GOVERNANCE", - Days: 30, - Years: 1, + Mode: "GOVERNANCE", + Days: 30, + Years: 1, + DaysSet: true, + YearsSet: true, }, }, }, @@ -573,8 +577,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) { ObjectLockEnabled: "Enabled", Rule: &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: "INVALID_MODE", - Days: 30, + Mode: "INVALID_MODE", + Days: 30, + DaysSet: true, }, }, }, @@ -587,8 +592,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) { ObjectLockEnabled: "Enabled", Rule: &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: "GOVERNANCE", - Days: 50000, + Mode: "GOVERNANCE", + Days: 50000, + DaysSet: true, }, }, }, @@ -601,8 +607,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) { ObjectLockEnabled: "Enabled", Rule: &ObjectLockRule{ DefaultRetention: &DefaultRetention{ - Mode: "GOVERNANCE", - Years: 200, + Mode: "GOVERNANCE", + Years: 200, + YearsSet: true, }, }, }, @@ -651,23 +658,26 @@ func TestValidateDefaultRetention(t *testing.T) { { name: "Valid retention with days", retention: &DefaultRetention{ - Mode: "GOVERNANCE", - Days: 30, + Mode: "GOVERNANCE", + Days: 30, + DaysSet: true, }, expectError: false, }, { name: "Valid retention with years", retention: &DefaultRetention{ - Mode: "COMPLIANCE", - Years: 1, + Mode: "COMPLIANCE", + Years: 1, + YearsSet: true, }, expectError: false, }, { name: "Missing mode", retention: &DefaultRetention{ - Days: 30, + Days: 30, + DaysSet: true, }, expectError: true, errorMsg: "default retention must specify Mode", @@ -675,8 +685,9 @@ func TestValidateDefaultRetention(t *testing.T) { { name: "Invalid mode", retention: &DefaultRetention{ - Mode: "INVALID", - Days: 30, + Mode: "INVALID", + Days: 30, + DaysSet: true, }, expectError: true, errorMsg: "invalid default retention mode", @@ -684,9 +695,11 @@ func TestValidateDefaultRetention(t *testing.T) { { name: "Both days and years specified", retention: &DefaultRetention{ - Mode: "GOVERNANCE", - Days: 30, - Years: 1, + Mode: "GOVERNANCE", + Days: 30, + Years: 1, + DaysSet: true, + YearsSet: true, }, expectError: true, errorMsg: "default retention cannot specify both Days and Years", diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go index 17057f604..613b27b95 100644 --- a/weed/s3api/s3err/s3api_errors.go +++ b/weed/s3api/s3err/s3api_errors.go @@ -57,6 +57,7 @@ const ( ErrNoSuchKey ErrNoSuchUpload ErrInvalidBucketName + ErrInvalidBucketState ErrInvalidDigest ErrInvalidMaxKeys ErrInvalidMaxUploads @@ -112,6 +113,8 @@ const ( ErrNoSuchTagSet ErrNoSuchObjectLockConfiguration ErrNoSuchObjectLegalHold + ErrInvalidRetentionPeriod + ErrObjectLockConfigurationNotFoundError ) // Error message constants for checksum validation @@ -154,6 +157,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The specified bucket is not valid.", HTTPStatusCode: http.StatusBadRequest, }, + ErrInvalidBucketState: { + Code: "InvalidBucketState", + Description: "The bucket is not in a valid state for the requested operation", + HTTPStatusCode: http.StatusConflict, + }, ErrInvalidDigest: { Code: "InvalidDigest", Description: "The Content-Md5 you specified is not valid.", @@ -209,6 +217,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The specified object does not have a legal hold configuration", HTTPStatusCode: http.StatusNotFound, }, + ErrInvalidRetentionPeriod: { + Code: "InvalidRetentionPeriod", + Description: "The retention period specified is invalid", + HTTPStatusCode: http.StatusBadRequest, + }, ErrNoSuchCORSConfiguration: { Code: "NoSuchCORSConfiguration", Description: "The CORS configuration does not exist", @@ -447,6 +460,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The bucket ownership controls were not found", HTTPStatusCode: http.StatusNotFound, }, + ErrObjectLockConfigurationNotFoundError: { + Code: "ObjectLockConfigurationNotFoundError", + Description: "Object Lock configuration does not exist for this bucket", + HTTPStatusCode: http.StatusNotFound, + }, } // GetAPIError provides API Error for input API error code.