From f9cbed7ee1a59c1345466b80e254a656352cc143 Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 17 Jul 2025 23:12:19 -0700 Subject: [PATCH] fix GetObjectLockConfigurationHandler --- weed/s3api/s3api_object_handlers_retention.go | 25 ++++-- weed/s3api/s3api_object_lock_fix_test.go | 90 +++++++++++++++++++ 2 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 weed/s3api/s3api_object_lock_fix_test.go diff --git a/weed/s3api/s3api_object_handlers_retention.go b/weed/s3api/s3api_object_handlers_retention.go index e92e821c8..f15e2fa54 100644 --- a/weed/s3api/s3api_object_handlers_retention.go +++ b/weed/s3api/s3api_object_handlers_retention.go @@ -322,14 +322,27 @@ func (s3a *S3ApiServer) GetObjectLockConfigurationHandler(w http.ResponseWriter, return } - // Check if object lock configuration exists - if bucketConfig.Entry.Extended == nil { - s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchObjectLockConfiguration) - return + var configXML []byte + + if bucketConfig.Entry.Extended != nil { + // First check if we have stored XML configuration (includes retention policies) + if storedConfigXML, exists := bucketConfig.Entry.Extended[s3_constants.ExtObjectLockConfigKey]; exists { + configXML = storedConfigXML + } else { + // Check if Object Lock is enabled via boolean flag + if enabledBytes, exists := bucketConfig.Entry.Extended[s3_constants.ExtObjectLockEnabledKey]; exists { + enabled := string(enabledBytes) + if enabled == s3_constants.ObjectLockEnabled || enabled == "true" { + // Generate minimal XML configuration for enabled Object Lock without retention policies + minimalConfig := `Enabled` + configXML = []byte(minimalConfig) + } + } + } } - configXML, exists := bucketConfig.Entry.Extended[s3_constants.ExtObjectLockConfigKey] - if !exists { + // If no Object Lock configuration found, return error + if len(configXML) == 0 { s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchObjectLockConfiguration) return } diff --git a/weed/s3api/s3api_object_lock_fix_test.go b/weed/s3api/s3api_object_lock_fix_test.go new file mode 100644 index 000000000..e8a3cf6ba --- /dev/null +++ b/weed/s3api/s3api_object_lock_fix_test.go @@ -0,0 +1,90 @@ +package s3api + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/stretchr/testify/assert" +) + +// TestVeeamObjectLockBugFix tests the fix for the bug where GetObjectLockConfigurationHandler +// would return NoSuchObjectLockConfiguration for buckets with no extended attributes, +// even when Object Lock was enabled. This caused Veeam to think Object Lock wasn't supported. +func TestVeeamObjectLockBugFix(t *testing.T) { + + t.Run("Bug case: bucket with no extended attributes", func(t *testing.T) { + // This simulates the bug case where a bucket has no extended attributes at all + // The old code would immediately return NoSuchObjectLockConfiguration + // The new code correctly checks if Object Lock is enabled before returning an error + + bucketConfig := &BucketConfig{ + Name: "test-bucket", + Entry: &filer_pb.Entry{ + Name: "test-bucket", + Extended: nil, // This is the key - no extended attributes + }, + } + + // Simulate the isObjectLockEnabledForBucket logic + enabled := false + if bucketConfig.Entry.Extended != nil { + if enabledBytes, exists := bucketConfig.Entry.Extended[s3_constants.ExtObjectLockEnabledKey]; exists { + enabled = string(enabledBytes) == s3_constants.ObjectLockEnabled || string(enabledBytes) == "true" + } + } + + // Should correctly return false (not enabled) - this would trigger 404 correctly + assert.False(t, enabled, "Object Lock should not be enabled when no extended attributes exist") + }) + + t.Run("Fix verification: bucket with Object Lock enabled via boolean flag", func(t *testing.T) { + // This verifies the fix works when Object Lock is enabled via boolean flag + + bucketConfig := &BucketConfig{ + Name: "test-bucket", + Entry: &filer_pb.Entry{ + Name: "test-bucket", + Extended: map[string][]byte{ + s3_constants.ExtObjectLockEnabledKey: []byte("true"), + }, + }, + } + + // Simulate the isObjectLockEnabledForBucket logic + enabled := false + if bucketConfig.Entry.Extended != nil { + if enabledBytes, exists := bucketConfig.Entry.Extended[s3_constants.ExtObjectLockEnabledKey]; exists { + enabled = string(enabledBytes) == s3_constants.ObjectLockEnabled || string(enabledBytes) == "true" + } + } + + // Should correctly return true (enabled) - this would generate minimal XML response + assert.True(t, enabled, "Object Lock should be enabled when boolean flag is set") + }) + + t.Run("Fix verification: bucket with Object Lock enabled via Enabled constant", func(t *testing.T) { + // Test using the s3_constants.ObjectLockEnabled constant + + bucketConfig := &BucketConfig{ + Name: "test-bucket", + Entry: &filer_pb.Entry{ + Name: "test-bucket", + Extended: map[string][]byte{ + s3_constants.ExtObjectLockEnabledKey: []byte(s3_constants.ObjectLockEnabled), + }, + }, + } + + // Simulate the isObjectLockEnabledForBucket logic + enabled := false + if bucketConfig.Entry.Extended != nil { + if enabledBytes, exists := bucketConfig.Entry.Extended[s3_constants.ExtObjectLockEnabledKey]; exists { + enabled = string(enabledBytes) == s3_constants.ObjectLockEnabled || string(enabledBytes) == "true" + } + } + + // Should correctly return true (enabled) + assert.True(t, enabled, "Object Lock should be enabled when constant is used") + }) +}