diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index 05c93a913..f06749f1d 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 + 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 + } + // Delete specific version err := s3a.deleteSpecificObjectVersion(bucket, object, versionId) if err != nil { @@ -73,7 +71,7 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque // Set version ID in response header w.Header().Set("x-amz-version-id", versionId) } else { - // Create delete marker (logical delete) + // Create delete marker (logical delete) - this is NOT blocked by object lock retention deleteMarkerVersionId, err := s3a.createDeleteMarker(bucket, object) if err != nil { glog.Errorf("Failed to create delete marker: %v", err) diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 049e0e4ae..8142113d8 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -573,9 +573,9 @@ func mapValidationErrorToS3Error(err error) s3err.ErrorCode { // This matches the test expectations return s3err.ErrInvalidRequest case errors.Is(err, ErrInvalidLegalHoldStatus): - // For invalid legal hold status, return InvalidRequest - // This matches the test expectations - return s3err.ErrInvalidRequest + // For invalid legal hold status, return MalformedXML + // AWS S3 returns MalformedXML for invalid legal hold status values + return s3err.ErrMalformedXML case errors.Is(err, ErrInvalidRetentionDateFormat): // For malformed retention date format, return MalformedDate // This matches the test expectations 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",