address comments

weed/s3api/s3api_object_handlers_put.go

@@ -87,6 +87,14 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request)
 
 		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 {

weed/s3api/s3api_object_retention.go

@@ -150,20 +150,21 @@ func parseObjectLockConfiguration(r *http.Request) (*ObjectLockConfiguration, er
 
 // validateRetention validates retention configuration
 func validateRetention(retention *ObjectRetention) error {
-	if retention.Mode == "" && retention.RetainUntilDate == nil {
-		return fmt.Errorf("retention configuration must specify either Mode or RetainUntilDate")
+	// AWS requires both Mode and RetainUntilDate for PutObjectRetention
+	if retention.Mode == "" {
+		return fmt.Errorf("retention configuration must specify Mode")
 	}
 
-	if retention.Mode != "" {
-		if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
-			return fmt.Errorf("invalid retention mode: %s", retention.Mode)
-		}
+	if retention.RetainUntilDate == nil {
+		return fmt.Errorf("retention configuration must specify RetainUntilDate")
 	}
 
-	if retention.RetainUntilDate != nil {
-		if retention.RetainUntilDate.Before(time.Now()) {
-			return fmt.Errorf("retain until date must be in the future")
-		}
+	if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
+		return fmt.Errorf("invalid retention mode: %s", retention.Mode)
+	}
+
+	if retention.RetainUntilDate.Before(time.Now()) {
+		return fmt.Errorf("retain until date must be in the future")
 	}
 
 	return nil

weed/s3api/s3api_object_retention_test.go

@@ -0,0 +1,169 @@
+package s3api
+
+import (
+	"testing"
+	"time"
+
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+)
+
+func TestValidateRetention(t *testing.T) {
+	futureTime := time.Now().Add(24 * time.Hour)
+	pastTime := time.Now().Add(-24 * time.Hour)
+
+	tests := []struct {
+		name        string
+		retention   *ObjectRetention
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name: "Valid GOVERNANCE retention",
+			retention: &ObjectRetention{
+				Mode:            s3_constants.RetentionModeGovernance,
+				RetainUntilDate: &futureTime,
+			},
+			expectError: false,
+		},
+		{
+			name: "Valid COMPLIANCE retention",
+			retention: &ObjectRetention{
+				Mode:            s3_constants.RetentionModeCompliance,
+				RetainUntilDate: &futureTime,
+			},
+			expectError: false,
+		},
+		{
+			name: "Missing Mode",
+			retention: &ObjectRetention{
+				RetainUntilDate: &futureTime,
+			},
+			expectError: true,
+			errorMsg:    "retention configuration must specify Mode",
+		},
+		{
+			name: "Missing RetainUntilDate",
+			retention: &ObjectRetention{
+				Mode: s3_constants.RetentionModeGovernance,
+			},
+			expectError: true,
+			errorMsg:    "retention configuration must specify RetainUntilDate",
+		},
+		{
+			name: "Invalid Mode",
+			retention: &ObjectRetention{
+				Mode:            "INVALID_MODE",
+				RetainUntilDate: &futureTime,
+			},
+			expectError: true,
+			errorMsg:    "invalid retention mode: INVALID_MODE",
+		},
+		{
+			name: "Past RetainUntilDate",
+			retention: &ObjectRetention{
+				Mode:            s3_constants.RetentionModeGovernance,
+				RetainUntilDate: &pastTime,
+			},
+			expectError: true,
+			errorMsg:    "retain until date must be in the future",
+		},
+		{
+			name:        "Empty retention",
+			retention:   &ObjectRetention{},
+			expectError: true,
+			errorMsg:    "retention configuration must specify Mode",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateRetention(tt.retention)
+
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("Expected error but got none")
+				} else if err.Error() != tt.errorMsg {
+					t.Errorf("Expected error message '%s', got '%s'", tt.errorMsg, err.Error())
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Expected no error but got: %v", err)
+				}
+			}
+		})
+	}
+}
+
+func TestValidateLegalHold(t *testing.T) {
+	tests := []struct {
+		name        string
+		legalHold   *ObjectLegalHold
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name: "Valid ON status",
+			legalHold: &ObjectLegalHold{
+				Status: s3_constants.LegalHoldOn,
+			},
+			expectError: false,
+		},
+		{
+			name: "Valid OFF status",
+			legalHold: &ObjectLegalHold{
+				Status: s3_constants.LegalHoldOff,
+			},
+			expectError: false,
+		},
+		{
+			name: "Invalid status",
+			legalHold: &ObjectLegalHold{
+				Status: "INVALID_STATUS",
+			},
+			expectError: true,
+			errorMsg:    "invalid legal hold status: INVALID_STATUS",
+		},
+		{
+			name: "Empty status",
+			legalHold: &ObjectLegalHold{
+				Status: "",
+			},
+			expectError: true,
+			errorMsg:    "invalid legal hold status: ",
+		},
+		{
+			name: "Lowercase on",
+			legalHold: &ObjectLegalHold{
+				Status: "on",
+			},
+			expectError: true,
+			errorMsg:    "invalid legal hold status: on",
+		},
+		{
+			name: "Lowercase off",
+			legalHold: &ObjectLegalHold{
+				Status: "off",
+			},
+			expectError: true,
+			errorMsg:    "invalid legal hold status: off",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateLegalHold(tt.legalHold)
+
+			if tt.expectError {
+				if err == nil {
+					t.Errorf("Expected error but got none")
+				} else if err.Error() != tt.errorMsg {
+					t.Errorf("Expected error message '%s', got '%s'", tt.errorMsg, err.Error())
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Expected no error but got: %v", err)
+				}
+			}
+		})
+	}
+}

weed/s3api/s3api_server.go

@@ -206,8 +206,6 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
 		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectRetentionHandler, ACTION_WRITE)), "PUT")).Queries("retention", "")
 		// PutObjectLegalHold
 		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLegalHoldHandler, ACTION_WRITE)), "PUT")).Queries("legal-hold", "")
-		// PutObjectLockConfiguration
-		bucket.Methods(http.MethodPut).Path("/{object:.+}").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLockConfigurationHandler, ACTION_WRITE)), "PUT")).Queries("object-lock", "")
 
 		// GetObjectACL
 		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectAclHandler, ACTION_READ_ACP)), "GET")).Queries("acl", "")
@@ -215,8 +213,6 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
 		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectRetentionHandler, ACTION_READ)), "GET")).Queries("retention", "")
 		// GetObjectLegalHold
 		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLegalHoldHandler, ACTION_READ)), "GET")).Queries("legal-hold", "")
-		// GetObjectLockConfiguration
-		bucket.Methods(http.MethodGet).Path("/{object:.+}").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLockConfigurationHandler, ACTION_READ)), "GET")).Queries("object-lock", "")
 
 		// objects with query
 
@@ -278,6 +274,10 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
 		bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketVersioningHandler, ACTION_READ)), "GET")).Queries("versioning", "")
 		bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketVersioningHandler, ACTION_WRITE)), "PUT")).Queries("versioning", "")
 
+		// GetObjectLockConfiguration / PutObjectLockConfiguration (bucket-level operations)
+		bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLockConfigurationHandler, ACTION_READ)), "GET")).Queries("object-lock", "")
+		bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLockConfigurationHandler, ACTION_WRITE)), "PUT")).Queries("object-lock", "")
+
 		// GetBucketTagging
 		bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketTaggingHandler, ACTION_TAGGING)), "GET")).Queries("tagging", "")
 		bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketTaggingHandler, ACTION_TAGGING)), "PUT")).Queries("tagging", "")

weed/s3api/s3err/s3api_errors.go

@@ -199,7 +199,7 @@ var errorCodeResponse = map[ErrorCode]APIError{
 		HTTPStatusCode: http.StatusNotFound,
 	},
 	ErrNoSuchObjectLockConfiguration: {
-		Code:           "ObjectLockConfigurationNotFoundError",
+		Code:           "NoSuchObjectLockConfiguration",
 		Description:    "The specified object does not have an ObjectLock configuration",
 		HTTPStatusCode: http.StatusNotFound,
 	},