lsm/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2025-08-20 04:33:36 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

refactor (#6999)

Browse Source 
* 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

* refactor

* rename

---------

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>
This commit is contained in:
Chris Lu 2025-07-19 00:49:56 -07:00 committed by GitHub
parent 26403e8a0d
commit 0e4d803896
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 179 additions and 152 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

124
weed/s3api/object_lock_utils.go
View File

@ -4,7 +4,9 @@ import (
"encoding/xml"
"fmt"
"strconv"
"time"
"github.com/seaweedfs/seaweedfs/weed/glog"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
@ -237,3 +239,125 @@ func ValidateObjectLockParameters(enabled bool, mode string, duration int32) err
return nil
}
// ====================================================================
// OBJECT LOCK VALIDATION FUNCTIONS
// ====================================================================
// These validation functions provide comprehensive validation for
// all Object Lock related configurations and requests.
// ValidateRetention validates retention configuration for object-level retention
func ValidateRetention(retention *ObjectRetention) error {
// Check if mode is specified
if retention.Mode == "" {
return ErrRetentionMissingMode
}
// Check if retain until date is specified
if retention.RetainUntilDate == nil {
return ErrRetentionMissingRetainUntilDate
}
// Check if mode is valid
if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
return ErrInvalidRetentionModeValue
}
// Check if retain until date is in the future
if retention.RetainUntilDate.Before(time.Now()) {
return ErrRetentionDateMustBeFuture
}
return nil
}
// 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 ErrInvalidLegalHoldStatus
}
return nil
}
// ValidateObjectLockConfiguration validates object lock configuration at bucket level
func ValidateObjectLockConfiguration(config *ObjectLockConfiguration) error {
// ObjectLockEnabled is required for bucket-level configuration
if config.ObjectLockEnabled == "" {
return ErrObjectLockConfigurationMissingEnabled
}
// Validate ObjectLockEnabled value
if config.ObjectLockEnabled != s3_constants.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 ErrRuleMissingDefaultRetention
}
return validateDefaultRetention(config.Rule.DefaultRetention)
}
return nil
}
// validateDefaultRetention validates default retention configuration for bucket-level settings
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 ErrDefaultRetentionMissingMode
}
// Mode must be valid
if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
return ErrInvalidDefaultRetentionMode
}
// Check for invalid Years value (negative values are always invalid)
if retention.YearsSet && retention.Years < 0 {
return ErrInvalidRetentionPeriod
}
// Check for invalid Days value (negative values are invalid)
if retention.DaysSet && retention.Days < 0 {
return ErrInvalidRetentionPeriod
}
// Check for invalid Days value (zero is invalid when explicitly provided)
if retention.DaysSet && retention.Days == 0 {
return ErrInvalidRetentionPeriod
}
// 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 {
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
}

2
weed/s3api/s3api_bucket_handlers_object_lock_config.go
View File

@ -41,7 +41,7 @@ func (s3a *S3ApiServer) PutObjectLockConfigurationHandler(w http.ResponseWriter,
}
// Validate object lock configuration
if err := validateObjectLockConfiguration(config); err != nil {
if err := ValidateObjectLockConfiguration(config); err != nil {
glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock config: %v", err)
s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err))
return

2
weed/s3api/s3api_object_handlers_legal_hold.go
View File

@ -34,7 +34,7 @@ func (s3a *S3ApiServer) PutObjectLegalHoldHandler(w http.ResponseWriter, r *http
}
// Validate legal hold configuration
if err := validateLegalHold(legalHold); err != nil {
if err := ValidateLegalHold(legalHold); err != nil {
glog.Errorf("PutObjectLegalHoldHandler: invalid legal hold config: %v", err)
s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err))
return

2
weed/s3api/s3api_object_handlers_retention.go
View File

@ -37,7 +37,7 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
}
// Validate retention configuration
if err := validateRetention(retention); err != nil {
if err := ValidateRetention(retention); err != nil {
glog.Errorf("PutObjectRetentionHandler: invalid retention config: %v", err)
s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err))
return

195
weed/s3api/s3api_object_retention.go
View File

@ -15,6 +15,10 @@ import (
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
)
// ====================================================================
// ERROR DEFINITIONS
// ====================================================================
// Sentinel errors for proper error handling instead of string matching
var (
ErrNoRetentionConfiguration = errors.New("no retention configuration found")
@ -47,6 +51,10 @@ const (
MaxRetentionYears = 100 // Maximum number of years for object retention
)
// ====================================================================
// DATA STRUCTURES
// ====================================================================
// ObjectRetention represents S3 Object Retention configuration
type ObjectRetention struct {
XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Retention"`
@ -75,7 +83,6 @@ type ObjectLockRule struct {
// 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:"http://s3.amazonaws.com/doc/2006-03-01/ DefaultRetention"`
Mode string `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Mode,omitempty"`
@ -85,6 +92,12 @@ type DefaultRetention struct {
YearsSet bool `xml:"-"`
}
// ====================================================================
// XML PARSING
// ====================================================================
// UnmarshalXML implements custom XML unmarshaling for DefaultRetention
// to track whether Days/Years fields were explicitly present in the XML
func (dr *DefaultRetention) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
type Alias DefaultRetention
aux := &struct {
@ -166,110 +179,9 @@ func parseObjectLockConfiguration(request *http.Request) (*ObjectLockConfigurati
return &config, nil
}
// validateRetention validates retention configuration
func validateRetention(retention *ObjectRetention) error {
// Check if mode is specified
if retention.Mode == "" {
return ErrRetentionMissingMode
}
// Check if retain until date is specified
if retention.RetainUntilDate == nil {
return ErrRetentionMissingRetainUntilDate
}
// Check if mode is valid
if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
return ErrInvalidRetentionModeValue
}
// Check if retain until date is in the future
if retention.RetainUntilDate.Before(time.Now()) {
return ErrRetentionDateMustBeFuture
}
return nil
}
// 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 ErrInvalidLegalHoldStatus
}
return nil
}
// validateObjectLockConfiguration validates object lock configuration
func validateObjectLockConfiguration(config *ObjectLockConfiguration) error {
// ObjectLockEnabled is required for bucket-level configuration
if config.ObjectLockEnabled == "" {
return ErrObjectLockConfigurationMissingEnabled
}
// Validate ObjectLockEnabled value
if config.ObjectLockEnabled != s3_constants.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 ErrRuleMissingDefaultRetention
}
return validateDefaultRetention(config.Rule.DefaultRetention)
}
return nil
}
// 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 ErrDefaultRetentionMissingMode
}
// Mode must be valid
if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
return ErrInvalidDefaultRetentionMode
}
// Check for invalid Years value (negative values are always invalid)
if retention.YearsSet && retention.Years < 0 {
return ErrInvalidRetentionPeriod
}
// Check for invalid Days value (negative values are invalid)
if retention.DaysSet && retention.Days < 0 {
return ErrInvalidRetentionPeriod
}
// Check for invalid Days value (zero is invalid when explicitly provided)
if retention.DaysSet && retention.Days == 0 {
return ErrInvalidRetentionPeriod
}
// 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
}
// ====================================================================
// OBJECT ENTRY OPERATIONS
// ====================================================================
// getObjectEntry retrieves the appropriate object entry based on versioning and versionId
func (s3a *S3ApiServer) getObjectEntry(bucket, object, versionId string) (*filer_pb.Entry, error) {
@ -300,7 +212,11 @@ func (s3a *S3ApiServer) getObjectEntry(bucket, object, versionId string) (*filer
return entry, nil
}
// getObjectRetention retrieves retention configuration from object metadata
// ====================================================================
// RETENTION OPERATIONS
// ====================================================================
// getObjectRetention retrieves object retention configuration
func (s3a *S3ApiServer) getObjectRetention(bucket, object, versionId string) (*ObjectRetention, error) {
entry, err := s3a.getObjectEntry(bucket, object, versionId)
if err != nil {
@ -333,7 +249,7 @@ func (s3a *S3ApiServer) getObjectRetention(bucket, object, versionId string) (*O
return retention, nil
}
// setObjectRetention sets retention configuration on object metadata
// setObjectRetention sets object retention configuration
func (s3a *S3ApiServer) setObjectRetention(bucket, object, versionId string, retention *ObjectRetention, bypassGovernance bool) error {
var entry *filer_pb.Entry
var err error
@ -446,7 +362,11 @@ func (s3a *S3ApiServer) setObjectRetention(bucket, object, versionId string, ret
})
}
// getObjectLegalHold retrieves legal hold configuration from object metadata
// ====================================================================
// LEGAL HOLD OPERATIONS
// ====================================================================
// getObjectLegalHold retrieves object legal hold configuration
func (s3a *S3ApiServer) getObjectLegalHold(bucket, object, versionId string) (*ObjectLegalHold, error) {
entry, err := s3a.getObjectEntry(bucket, object, versionId)
if err != nil {
@ -468,7 +388,7 @@ func (s3a *S3ApiServer) getObjectLegalHold(bucket, object, versionId string) (*O
return legalHold, nil
}
// setObjectLegalHold sets legal hold configuration on object metadata
// setObjectLegalHold sets object legal hold configuration
func (s3a *S3ApiServer) setObjectLegalHold(bucket, object, versionId string, legalHold *ObjectLegalHold) error {
var entry *filer_pb.Entry
var err error
@ -529,7 +449,11 @@ func (s3a *S3ApiServer) setObjectLegalHold(bucket, object, versionId string, leg
})
}
// isObjectRetentionActive checks if an object is currently under retention
// ====================================================================
// PROTECTION ENFORCEMENT
// ====================================================================
// isObjectRetentionActive checks if object has active retention
func (s3a *S3ApiServer) isObjectRetentionActive(bucket, object, versionId string) (bool, error) {
retention, err := s3a.getObjectRetention(bucket, object, versionId)
if err != nil {
@ -547,7 +471,7 @@ func (s3a *S3ApiServer) isObjectRetentionActive(bucket, object, versionId string
return false, nil
}
// getRetentionFromEntry extracts retention configuration from an existing entry
// getRetentionFromEntry extracts retention configuration from filer entry
func (s3a *S3ApiServer) getRetentionFromEntry(entry *filer_pb.Entry) (*ObjectRetention, bool, error) {
if entry.Extended == nil {
return nil, false, nil
@ -577,7 +501,7 @@ func (s3a *S3ApiServer) getRetentionFromEntry(entry *filer_pb.Entry) (*ObjectRet
return retention, isActive, nil
}
// getLegalHoldFromEntry extracts legal hold configuration from an existing entry
// getLegalHoldFromEntry extracts legal hold configuration from filer entry
func (s3a *S3ApiServer) getLegalHoldFromEntry(entry *filer_pb.Entry) (*ObjectLegalHold, bool, error) {
if entry.Extended == nil {
return nil, false, nil
@ -595,14 +519,11 @@ func (s3a *S3ApiServer) getLegalHoldFromEntry(entry *filer_pb.Entry) (*ObjectLeg
return legalHold, isActive, nil
}
// 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.
// ====================================================================
// GOVERNANCE BYPASS
// ====================================================================
// checkGovernanceBypassPermission checks if user has permission to bypass governance retention
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
@ -633,15 +554,7 @@ func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, b
return false
}
// 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.).
// evaluateGovernanceBypassRequest evaluates if governance bypass is requested and permitted
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"
@ -661,18 +574,7 @@ func (s3a *S3ApiServer) evaluateGovernanceBypassRequest(r *http.Request, bucket,
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.
// enforceObjectLockProtections enforces object lock protections for operations
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
@ -735,8 +637,11 @@ func (s3a *S3ApiServer) enforceObjectLockProtections(request *http.Request, buck
return nil
}
// isObjectLockAvailable checks if Object Lock features are available for the bucket
// Object Lock requires versioning to be enabled (AWS S3 requirement)
// ====================================================================
// AVAILABILITY CHECKS
// ====================================================================
// isObjectLockAvailable checks if object lock is available for the bucket
func (s3a *S3ApiServer) isObjectLockAvailable(bucket string) error {
versioningEnabled, err := s3a.isVersioningEnabled(bucket)
if err != nil {
@ -753,9 +658,7 @@ func (s3a *S3ApiServer) isObjectLockAvailable(bucket string) error {
return nil
}
// handleObjectLockAvailabilityCheck is a helper function to check object lock availability
// and write the appropriate error response if not available. This reduces code duplication
// across all retention handlers.
// handleObjectLockAvailabilityCheck handles object lock availability checks for API endpoints
func (s3a *S3ApiServer) handleObjectLockAvailabilityCheck(w http.ResponseWriter, request *http.Request, bucket, handlerName string) bool {
if err := s3a.isObjectLockAvailable(bucket); err != nil {
glog.Errorf("%s: object lock not available for bucket %s: %v", handlerName, bucket, err)

6
weed/s3api/s3api_object_retention_test.go
View File

@ -80,7 +80,7 @@ func TestValidateRetention(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateRetention(tt.retention)
err := ValidateRetention(tt.retention)
if tt.expectError {
if err == nil {
@ -154,7 +154,7 @@ func TestValidateLegalHold(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateLegalHold(tt.legalHold)
err := ValidateLegalHold(tt.legalHold)
if tt.expectError {
if err == nil {
@ -631,7 +631,7 @@ func TestValidateObjectLockConfiguration(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateObjectLockConfiguration(tt.config)
err := ValidateObjectLockConfiguration(tt.config)
if tt.expectError {
if err == nil {