fix: derive the Object Lock versioning rule from the parsed configuration

The load-time normalization compared the stored lock document with the
canonical enabled document byte for byte, so a lock configuration that also
carries a default retention rule kept a suspended or prefix-excluded
versioning document. Decide from the parsed configuration instead, after it
is parsed, so every writer that goes through Save, including the site
replication versioning and heal paths, ends with plain Enabled versioning on
a locked bucket. Receiving a lock configuration on a bucket created without
lock now enables versioning as well; the test that asserted the opposite is
updated, and a new test covers a rule-bearing lock document with suspended
and prefix-excluded versioning through Update, Get, and reload.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PvgysXDmhPBBimCReYtA8q
Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/bucket-metadata.go

@@ -377,15 +377,6 @@ func (b *BucketMetadata) parseAllConfigs(ctx context.Context, objectAPI ObjectLa
 		b.corsConfig = nil
 	}
 
-	if bytes.Equal(b.ObjectLockConfigXML, enabledBucketObjectLockConfig) {
-		// A locked bucket needs plain Enabled versioning; suspended or
-		// prefix-excluded configurations are not honored for it.
-		config, versioningErr := versioning.ParseConfig(bytes.NewReader(b.VersioningConfigXML))
-		if versioningErr != nil || !config.Enabled() || config.PrefixesExcluded() {
-			b.VersioningConfigXML = enabledBucketVersioningConfig
-		}
-	}
-
 	if len(b.ObjectLockConfigXML) != 0 {
 		b.objectLockConfig, err = objectlock.ParseObjectLockConfig(bytes.NewReader(b.ObjectLockConfigXML))
 		if err != nil {
@@ -394,6 +385,15 @@ func (b *BucketMetadata) parseAllConfigs(ctx context.Context, objectAPI ObjectLa
 	} else {
 		b.objectLockConfig = nil
 	}
+	if b.objectLockConfig != nil {
+		// Object Lock requires every object to be versioned. Whatever the lock
+		// document contains, a suspended or prefix-excluded versioning document
+		// is replaced by plain Enabled versioning; Save persists the result.
+		config, versioningErr := versioning.ParseConfig(bytes.NewReader(b.VersioningConfigXML))
+		if versioningErr != nil || !config.Enabled() || config.PrefixesExcluded() {
+			b.VersioningConfigXML = enabledBucketVersioningConfig
+		}
+	}
 
 	if len(b.VersioningConfigXML) != 0 {
 		b.versioningConfig, err = versioning.ParseConfig(bytes.NewReader(b.VersioningConfigXML))

cmd/site-replication-bucket-adoption_test.go

@@ -190,3 +190,48 @@ func testPeerBucketAdoptionBootstrapsMissingConfigs(_ ObjectLayer, instanceType,
 			after.ObjectLockConfigUpdatedAt, after.VersioningConfigUpdatedAt, before.Created)
 	}
 }
+
+// TestLockedBucketNormalizesVersioningOnSave covers the metadata boundary
+// itself: whatever writer stores a suspended or prefix-excluded versioning
+// document on a bucket that carries an Object Lock configuration, including
+// one with a default retention rule, Save replaces it with plain Enabled
+// versioning.
+func TestLockedBucketNormalizesVersioningOnSave(t *testing.T) {
+	defer DetectTestLeak(t)()
+	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+		t:                 t,
+		objAPITest:        testLockedBucketNormalizesVersioningOnSave,
+		makeBucketOptions: MakeBucketOptions{LockEnabled: true},
+	})
+}
+
+func testLockedBucketNormalizesVersioningOnSave(_ ObjectLayer, instanceType, bucketName string,
+	_ http.Handler, _ auth.Credentials, t *testing.T,
+) {
+	lockWithRule := []byte(`<ObjectLockConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><ObjectLockEnabled>Enabled</ObjectLockEnabled><Rule><DefaultRetention><Mode>GOVERNANCE</Mode><Days>30</Days></DefaultRetention></Rule></ObjectLockConfiguration>`)
+	if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, objectLockConfig, lockWithRule); err != nil {
+		t.Fatal(err)
+	}
+	for name, versioningXML := range map[string][]byte{
+		"prefix-excluded": []byte(`<VersioningConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Status>Enabled</Status><ExcludeFolders>true</ExcludeFolders><ExcludedPrefixes><Prefix>temporary/</Prefix></ExcludedPrefixes></VersioningConfiguration>`),
+		"suspended":       []byte(`<VersioningConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Status>Suspended</Status></VersioningConfiguration>`),
+	} {
+		if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, bucketVersioningConfig, versioningXML); err != nil {
+			t.Fatal(err)
+		}
+		meta, err := globalBucketMetadataSys.Get(bucketName)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !bytes.Equal(meta.VersioningConfigXML, enabledBucketVersioningConfig) {
+			t.Fatalf("%s/%s: locked bucket kept versioning %q", instanceType, name, meta.VersioningConfigXML)
+		}
+		reloaded, err := loadBucketMetadata(t.Context(), newObjectLayerFn(), bucketName)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !bytes.Equal(reloaded.VersioningConfigXML, enabledBucketVersioningConfig) {
+			t.Fatalf("%s/%s: locked bucket persisted versioning %q", instanceType, name, reloaded.VersioningConfigXML)
+		}
+	}
+}

cmd/site-replication-object-lock_test.go

@@ -159,8 +159,10 @@ func testPeerBucketObjectLockMetadataWithoutLockEnabled(_ ObjectLayer, instanceT
 	if err != nil {
 		t.Fatal(err)
 	}
-	if meta.objectLockConfig == nil || len(meta.VersioningConfigXML) != 0 {
+	// A lock configuration implies versioning: the bucket was created without
-		t.Fatalf("%s: unlocked bucket metadata = objectLock:%v versioning:%q", instanceType, meta.objectLockConfig, meta.VersioningConfigXML)
+	// lock, so receiving the configuration turns plain Enabled versioning on.
+	if meta.objectLockConfig == nil || !bytes.Equal(meta.VersioningConfigXML, enabledBucketVersioningConfig) {
+		t.Fatalf("%s: bucket metadata = objectLock:%v versioning:%q", instanceType, meta.objectLockConfig, meta.VersioningConfigXML)
 	}
 }