From 21646eebd2634552976a488d9e02ae34e1970af3 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 2 Sep 2026 18:49:16 +0800 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_01PvgysXDmhPBBimCReYtA8q Signed-off-by: Feng Ruohang --- cmd/bucket-metadata.go | 18 ++++---- cmd/site-replication-bucket-adoption_test.go | 45 ++++++++++++++++++++ cmd/site-replication-object-lock_test.go | 6 ++- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/cmd/bucket-metadata.go b/cmd/bucket-metadata.go index 2909ced4e..fa4f34b7a 100644 --- a/cmd/bucket-metadata.go +++ b/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)) diff --git a/cmd/site-replication-bucket-adoption_test.go b/cmd/site-replication-bucket-adoption_test.go index 2c056c5ab..09f494bd5 100644 --- a/cmd/site-replication-bucket-adoption_test.go +++ b/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(`EnabledGOVERNANCE30`) + if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, objectLockConfig, lockWithRule); err != nil { + t.Fatal(err) + } + for name, versioningXML := range map[string][]byte{ + "prefix-excluded": []byte(`Enabledtruetemporary/`), + "suspended": []byte(`Suspended`), + } { + 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) + } + } +} diff --git a/cmd/site-replication-object-lock_test.go b/cmd/site-replication-object-lock_test.go index 48d1d35f5..bf28df9fa 100644 --- a/cmd/site-replication-object-lock_test.go +++ b/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 { - t.Fatalf("%s: unlocked bucket metadata = objectLock:%v versioning:%q", instanceType, meta.objectLockConfig, meta.VersioningConfigXML) + // A lock configuration implies versioning: the bucket was created without + // 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) } }