diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go
index 901aeb912..3e7ccefc2 100644
--- a/weed/s3api/s3api_object_handlers_delete.go
+++ b/weed/s3api/s3api_object_handlers_delete.go
@@ -49,12 +49,14 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone)
}
- // Check object lock permissions before deletion
- bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
- if err := s3a.checkObjectLockPermissions(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
+ // 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(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 {
@@ -192,6 +194,18 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
// 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 {
+ 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
+ }
+
s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
// delete file entries
@@ -200,15 +214,17 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
continue
}
- // Check object lock permissions before deletion
- if err := s3a.checkObjectLockPermissions(bucket, object.ObjectName, "", bypassGovernance); err != nil {
- glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, object.ObjectName, err)
- deleteErrors = append(deleteErrors, DeleteError{
- Code: "AccessDenied",
- Message: err.Error(),
- Key: object.ObjectName,
- })
- continue
+ // Check object lock permissions before deletion (only for versioned buckets)
+ if versioningEnabled {
+ if err := s3a.checkObjectLockPermissions(bucket, object.ObjectName, "", bypassGovernance); err != nil {
+ glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, object.ObjectName, err)
+ deleteErrors = append(deleteErrors, DeleteError{
+ Code: "AccessDenied",
+ Message: err.Error(),
+ Key: object.ObjectName,
+ })
+ continue
+ }
}
lastSeparator := strings.LastIndex(object.ObjectName, "/")
parentDirectoryPath, entryName, isDeleteData, isRecursive := "", object.ObjectName, true, false
diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go
index 80f7979af..371ab870f 100644
--- a/weed/s3api/s3api_object_handlers_put.go
+++ b/weed/s3api/s3api_object_handlers_put.go
@@ -85,16 +85,15 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request)
glog.V(1).Infof("PutObjectHandler: bucket %s, object %s, versioningEnabled=%v", bucket, object, versioningEnabled)
+ // Check object lock permissions before PUT operation (only for versioned buckets)
+ bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+ if err := s3a.checkObjectLockPermissionsForPut(bucket, object, bypassGovernance, versioningEnabled); err != nil {
+ s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
+ return
+ }
+
if versioningEnabled {
// Handle versioned PUT
- // Check object lock permissions before creating new version (objects under retention should not be overwritten even in versioned buckets)
- bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
- if err := s3a.checkObjectLockPermissions(bucket, object, "", bypassGovernance); err != nil {
- glog.V(2).Infof("PutObjectHandler: object lock check failed for versioned PUT %s/%s: %v", bucket, object, err)
- s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
- return
- }
-
glog.V(1).Infof("PutObjectHandler: using versioned PUT for %s/%s", bucket, object)
versionId, etag, errCode := s3a.putVersionedObject(r, bucket, object, dataReader, objectContentType)
if errCode != s3err.ErrNone {
@@ -111,14 +110,6 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request)
setEtag(w, etag)
} else {
// Handle regular PUT (non-versioned)
- // Check object lock permissions before overwriting (only for non-versioned buckets)
- bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
- if err := s3a.checkObjectLockPermissions(bucket, object, "", bypassGovernance); err != nil {
- glog.V(2).Infof("PutObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
- s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
- return
- }
-
glog.V(1).Infof("PutObjectHandler: using regular PUT for %s/%s", bucket, object)
uploadUrl := s3a.toFilerUrl(bucket, object)
if objectContentType == "" {
diff --git a/weed/s3api/s3api_object_handlers_retention.go b/weed/s3api/s3api_object_handlers_retention.go
index eaf73bd12..08224cc2f 100644
--- a/weed/s3api/s3api_object_handlers_retention.go
+++ b/weed/s3api/s3api_object_handlers_retention.go
@@ -17,6 +17,17 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
bucket, object := s3_constants.GetBucketAndObject(r)
glog.V(3).Infof("PutObjectRetentionHandler %s %s", bucket, object)
+ // Check if Object Lock is available for this bucket (requires versioning)
+ if err := s3a.isObjectLockAvailable(bucket); err != nil {
+ glog.Errorf("PutObjectRetentionHandler: object lock not available for bucket %s: %v", bucket, err)
+ if strings.Contains(err.Error(), "bucket not found") {
+ s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+ } else {
+ s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+ }
+ return
+ }
+
// Get version ID from query parameters
versionId := r.URL.Query().Get("versionId")
@@ -71,6 +82,17 @@ func (s3a *S3ApiServer) GetObjectRetentionHandler(w http.ResponseWriter, r *http
bucket, object := s3_constants.GetBucketAndObject(r)
glog.V(3).Infof("GetObjectRetentionHandler %s %s", bucket, object)
+ // Check if Object Lock is available for this bucket (requires versioning)
+ if err := s3a.isObjectLockAvailable(bucket); err != nil {
+ glog.Errorf("GetObjectRetentionHandler: object lock not available for bucket %s: %v", bucket, err)
+ if strings.Contains(err.Error(), "bucket not found") {
+ s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+ } else {
+ s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+ }
+ return
+ }
+
// Get version ID from query parameters
versionId := r.URL.Query().Get("versionId")
@@ -129,6 +151,17 @@ func (s3a *S3ApiServer) PutObjectLegalHoldHandler(w http.ResponseWriter, r *http
bucket, object := s3_constants.GetBucketAndObject(r)
glog.V(3).Infof("PutObjectLegalHoldHandler %s %s", bucket, object)
+ // Check if Object Lock is available for this bucket (requires versioning)
+ if err := s3a.isObjectLockAvailable(bucket); err != nil {
+ glog.Errorf("PutObjectLegalHoldHandler: object lock not available for bucket %s: %v", bucket, err)
+ if strings.Contains(err.Error(), "bucket not found") {
+ s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+ } else {
+ s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+ }
+ return
+ }
+
// Get version ID from query parameters
versionId := r.URL.Query().Get("versionId")
@@ -175,6 +208,17 @@ func (s3a *S3ApiServer) GetObjectLegalHoldHandler(w http.ResponseWriter, r *http
bucket, object := s3_constants.GetBucketAndObject(r)
glog.V(3).Infof("GetObjectLegalHoldHandler %s %s", bucket, object)
+ // Check if Object Lock is available for this bucket (requires versioning)
+ if err := s3a.isObjectLockAvailable(bucket); err != nil {
+ glog.Errorf("GetObjectLegalHoldHandler: object lock not available for bucket %s: %v", bucket, err)
+ if strings.Contains(err.Error(), "bucket not found") {
+ s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
+ } else {
+ s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
+ }
+ return
+ }
+
// Get version ID from query parameters
versionId := r.URL.Query().Get("versionId")
@@ -242,8 +286,8 @@ func (s3a *S3ApiServer) PutObjectLockConfigurationHandler(w http.ResponseWriter,
}
// Validate object lock configuration
- if config.ObjectLockEnabled != "" && config.ObjectLockEnabled != s3_constants.ObjectLockEnabled {
- glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock enabled value: %s", config.ObjectLockEnabled)
+ if err := validateObjectLockConfiguration(config); err != nil {
+ glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock config: %v", err)
s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
return
}
diff --git a/weed/s3api/s3api_object_retention.go b/weed/s3api/s3api_object_retention.go
index b23806c35..32307e0e7 100644
--- a/weed/s3api/s3api_object_retention.go
+++ b/weed/s3api/s3api_object_retention.go
@@ -149,6 +149,56 @@ func validateLegalHold(legalHold *ObjectLegalHold) error {
return nil
}
+// validateObjectLockConfiguration validates object lock configuration
+func validateObjectLockConfiguration(config *ObjectLockConfiguration) error {
+ // Validate ObjectLockEnabled if present
+ if config.ObjectLockEnabled != "" && config.ObjectLockEnabled != s3_constants.ObjectLockEnabled {
+ return fmt.Errorf("invalid object lock enabled value: %s", config.ObjectLockEnabled)
+ }
+
+ // Validate Rule if present
+ if config.Rule != nil {
+ if config.Rule.DefaultRetention != nil {
+ return validateDefaultRetention(config.Rule.DefaultRetention)
+ }
+ }
+
+ return nil
+}
+
+// validateDefaultRetention validates default retention configuration
+func validateDefaultRetention(retention *DefaultRetention) error {
+ // Mode is required
+ if retention.Mode == "" {
+ return fmt.Errorf("default retention must specify Mode")
+ }
+
+ // 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)
+ }
+
+ // 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")
+ }
+
+ if retention.Days > 0 && retention.Years > 0 {
+ return fmt.Errorf("default retention cannot specify both Days and Years")
+ }
+
+ // Validate ranges
+ if retention.Days < 0 || retention.Days > 36500 {
+ return fmt.Errorf("default retention days must be between 0 and 36500")
+ }
+
+ if retention.Years < 0 || retention.Years > 100 {
+ return fmt.Errorf("default retention years must be between 0 and 100")
+ }
+
+ return nil
+}
+
// getObjectRetention retrieves retention configuration from object metadata
func (s3a *S3ApiServer) getObjectRetention(bucket, object, versionId string) (*ObjectRetention, error) {
var entry *filer_pb.Entry
@@ -448,3 +498,37 @@ func (s3a *S3ApiServer) checkObjectLockPermissions(bucket, object, versionId str
return nil
}
+
+// isObjectLockAvailable checks if Object Lock features are available for the bucket
+// Object Lock requires versioning to be enabled (AWS S3 requirement)
+func (s3a *S3ApiServer) isObjectLockAvailable(bucket string) error {
+ versioningEnabled, err := s3a.isVersioningEnabled(bucket)
+ if err != nil {
+ if err == filer_pb.ErrNotFound {
+ return fmt.Errorf("bucket not found")
+ }
+ return fmt.Errorf("error checking versioning status: %v", err)
+ }
+
+ if !versioningEnabled {
+ return fmt.Errorf("object lock requires versioning to be enabled")
+ }
+
+ return nil
+}
+
+// checkObjectLockPermissionsForPut checks object lock permissions for PUT operations
+// This is a shared helper to avoid code duplication in PUT handlers
+func (s3a *S3ApiServer) checkObjectLockPermissionsForPut(bucket, object string, bypassGovernance bool, versioningEnabled bool) error {
+ // Object Lock only applies to versioned buckets (AWS S3 requirement)
+ if !versioningEnabled {
+ return nil
+ }
+
+ // For PUT operations, we check permissions on the current object (empty versionId)
+ if err := s3a.checkObjectLockPermissions(bucket, object, "", bypassGovernance); err != nil {
+ glog.V(2).Infof("checkObjectLockPermissionsForPut: object lock check failed for %s/%s: %v", bucket, object, err)
+ return err
+ }
+ return nil
+}
diff --git a/weed/s3api/s3api_object_retention_test.go b/weed/s3api/s3api_object_retention_test.go
index ebe55fbfd..a495150b4 100644
--- a/weed/s3api/s3api_object_retention_test.go
+++ b/weed/s3api/s3api_object_retention_test.go
@@ -326,7 +326,7 @@ func TestParseXMLGeneric(t *testing.T) {
xmlBody: `
GOVERNANCE
- 2024-12-31T23:59:59Z
+ 2025-01-01T00:00:00Z
`,
expectError: false,
},
@@ -334,11 +334,11 @@ func TestParseXMLGeneric(t *testing.T) {
name: "Empty body",
xmlBody: "",
expectError: true,
- errorMsg: "empty request body",
+ errorMsg: "error parsing XML",
},
{
name: "Invalid XML",
- xmlBody: "",
+ xmlBody: "not xml",
expectError: true,
errorMsg: "error parsing XML",
},
@@ -346,13 +346,8 @@ func TestParseXMLGeneric(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- var req *http.Request
- if tt.xmlBody == "" {
- req = &http.Request{Body: nil}
- } else {
- req = &http.Request{
- Body: io.NopCloser(strings.NewReader(tt.xmlBody)),
- }
+ req := &http.Request{
+ Body: io.NopCloser(strings.NewReader(tt.xmlBody)),
}
var retention ObjectRetention
@@ -362,11 +357,242 @@ func TestParseXMLGeneric(t *testing.T) {
if err == nil {
t.Errorf("Expected error but got none")
} else if !strings.Contains(err.Error(), tt.errorMsg) {
- t.Errorf("Expected error message to contain '%s', got '%s'", tt.errorMsg, err.Error())
+ t.Errorf("Expected error message to contain '%s', got: %v", tt.errorMsg, err)
}
} else {
if err != nil {
- t.Errorf("Expected no error but got: %v", err)
+ t.Errorf("Unexpected error: %v", err)
+ }
+ }
+ })
+ }
+}
+
+func TestValidateObjectLockConfiguration(t *testing.T) {
+ tests := []struct {
+ name string
+ config *ObjectLockConfiguration
+ expectError bool
+ errorMsg string
+ }{
+ {
+ name: "Valid config with ObjectLockEnabled only",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ },
+ expectError: false,
+ },
+ {
+ name: "Valid config with rule and days",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ Days: 30,
+ },
+ },
+ },
+ expectError: false,
+ },
+ {
+ name: "Valid config with rule and years",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Mode: "COMPLIANCE",
+ Years: 1,
+ },
+ },
+ },
+ expectError: false,
+ },
+ {
+ name: "Invalid ObjectLockEnabled value",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "InvalidValue",
+ },
+ expectError: true,
+ errorMsg: "invalid object lock enabled value",
+ },
+ {
+ name: "Invalid rule - missing mode",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Days: 30,
+ },
+ },
+ },
+ expectError: true,
+ errorMsg: "default retention must specify Mode",
+ },
+ {
+ name: "Invalid rule - both days and years",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ Days: 30,
+ Years: 1,
+ },
+ },
+ },
+ expectError: true,
+ errorMsg: "default retention cannot specify both Days and Years",
+ },
+ {
+ name: "Invalid rule - neither days nor years",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ },
+ },
+ },
+ expectError: true,
+ errorMsg: "default retention must specify either Days or Years",
+ },
+ {
+ name: "Invalid rule - invalid mode",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Mode: "INVALID_MODE",
+ Days: 30,
+ },
+ },
+ },
+ expectError: true,
+ errorMsg: "invalid default retention mode",
+ },
+ {
+ name: "Invalid rule - days out of range",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ Days: 50000,
+ },
+ },
+ },
+ expectError: true,
+ errorMsg: "default retention days must be between 0 and 36500",
+ },
+ {
+ name: "Invalid rule - years out of range",
+ config: &ObjectLockConfiguration{
+ ObjectLockEnabled: "Enabled",
+ Rule: &ObjectLockRule{
+ DefaultRetention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ Years: 200,
+ },
+ },
+ },
+ expectError: true,
+ errorMsg: "default retention years must be between 0 and 100",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ err := validateObjectLockConfiguration(tt.config)
+
+ if tt.expectError {
+ if err == nil {
+ t.Errorf("Expected error but got none")
+ } else if !strings.Contains(err.Error(), tt.errorMsg) {
+ t.Errorf("Expected error message to contain '%s', got: %v", tt.errorMsg, err)
+ }
+ } else {
+ if err != nil {
+ t.Errorf("Unexpected error: %v", err)
+ }
+ }
+ })
+ }
+}
+
+func TestValidateDefaultRetention(t *testing.T) {
+ tests := []struct {
+ name string
+ retention *DefaultRetention
+ expectError bool
+ errorMsg string
+ }{
+ {
+ name: "Valid retention with days",
+ retention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ Days: 30,
+ },
+ expectError: false,
+ },
+ {
+ name: "Valid retention with years",
+ retention: &DefaultRetention{
+ Mode: "COMPLIANCE",
+ Years: 1,
+ },
+ expectError: false,
+ },
+ {
+ name: "Missing mode",
+ retention: &DefaultRetention{
+ Days: 30,
+ },
+ expectError: true,
+ errorMsg: "default retention must specify Mode",
+ },
+ {
+ name: "Invalid mode",
+ retention: &DefaultRetention{
+ Mode: "INVALID",
+ Days: 30,
+ },
+ expectError: true,
+ errorMsg: "invalid default retention mode",
+ },
+ {
+ name: "Both days and years specified",
+ retention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ Days: 30,
+ Years: 1,
+ },
+ expectError: true,
+ errorMsg: "default retention cannot specify both Days and Years",
+ },
+ {
+ name: "Neither days nor years specified",
+ retention: &DefaultRetention{
+ Mode: "GOVERNANCE",
+ },
+ expectError: true,
+ errorMsg: "default retention must specify either Days or Years",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ err := validateDefaultRetention(tt.retention)
+
+ if tt.expectError {
+ if err == nil {
+ t.Errorf("Expected error but got none")
+ } else if !strings.Contains(err.Error(), tt.errorMsg) {
+ t.Errorf("Expected error message to contain '%s', got: %v", tt.errorMsg, err)
+ }
+ } else {
+ if err != nil {
+ t.Errorf("Unexpected error: %v", err)
}
}
})