From 3b5de82f5add0dd39b8b6e2e958abd9badb13074 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 2 Sep 2026 14:06:56 +0800 Subject: [PATCH] fix: keep plain Enabled versioning when Object Lock is enabled on a bucket Site adoption and ForceCreate preserved a suspended or prefix-excluded versioning configuration while bootstrapping Object Lock, persisting a state that PutBucketVersioning itself rejects: objects under an excluded prefix in a WORM bucket were not versioned and escaped retention. enablePeerBucketVersioning now takes the lock intent and replaces such configurations with plain Enabled versioning, and metadata loading ignores prefix exclusions on a locked bucket as it ignored suspension before. The adoption tests assert the normalized state and keep the timestamp-preservation checks on valid documents. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01PvgysXDmhPBBimCReYtA8q Signed-off-by: Feng Ruohang --- cmd/bucket-metadata.go | 4 +++- cmd/erasure-server-pool.go | 4 ++-- cmd/site-replication-bucket-adoption_test.go | 16 +++++++++------- cmd/site-replication.go | 10 +++++++--- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/cmd/bucket-metadata.go b/cmd/bucket-metadata.go index 9c78e0eb6..2909ced4e 100644 --- a/cmd/bucket-metadata.go +++ b/cmd/bucket-metadata.go @@ -378,8 +378,10 @@ func (b *BucketMetadata) parseAllConfigs(ctx context.Context, objectAPI ObjectLa } 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() { + if versioningErr != nil || !config.Enabled() || config.PrefixesExcluded() { b.VersioningConfigXML = enabledBucketVersioningConfig } } diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index 1e85cf039..4a41e3b27 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -911,7 +911,7 @@ func (z *erasureServerPools) MakeBucket(ctx context.Context, bucket string, opts meta.SetCreatedAt(opts.CreatedAt) } if opts.LockEnabled { - if err := enablePeerBucketVersioning(&meta); err != nil { + if err := enablePeerBucketVersioning(&meta, true); err != nil { return err } if len(meta.ObjectLockConfigXML) == 0 { @@ -920,7 +920,7 @@ func (z *erasureServerPools) MakeBucket(ctx context.Context, bucket string, opts } } if opts.VersioningEnabled { - if err := enablePeerBucketVersioning(&meta); err != nil { + if err := enablePeerBucketVersioning(&meta, opts.LockEnabled); err != nil { return err } } diff --git a/cmd/site-replication-bucket-adoption_test.go b/cmd/site-replication-bucket-adoption_test.go index 2a5d98b3d..2c056c5ab 100644 --- a/cmd/site-replication-bucket-adoption_test.go +++ b/cmd/site-replication-bucket-adoption_test.go @@ -39,7 +39,9 @@ func testPeerBucketAdoptionPreservesLockAndVersioningConfigs(_ ObjectLayer, inst _ http.Handler, _ auth.Credentials, t *testing.T, ) { objectLockXML := []byte(`EnabledGOVERNANCE30`) - versioningXML := []byte(`Enabledtruetemporary/`) + // A locked bucket carries plain Enabled versioning; adoption must keep the + // existing document and its timestamp rather than rewrite them. + versioningXML := []byte(`Enabled`) if _, err := globalBucketMetadataSys.Update(t.Context(), bucketName, objectLockConfig, objectLockXML); err != nil { t.Fatal(err) } @@ -77,15 +79,15 @@ func TestPeerBucketAdoptionBootstrapsMissingConfigs(t *testing.T) { }) } -func TestPeerBucketAdoptionPreservesCustomVersioningWhenEnablingLock(t *testing.T) { +func TestPeerBucketAdoptionNormalizesVersioningWhenEnablingLock(t *testing.T) { defer DetectTestLeak(t)() ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{ t: t, - objAPITest: testPeerBucketAdoptionPreservesCustomVersioningWhenEnablingLock, + objAPITest: testPeerBucketAdoptionNormalizesVersioningWhenEnablingLock, }) } -func testPeerBucketAdoptionPreservesCustomVersioningWhenEnablingLock(_ ObjectLayer, instanceType, bucketName string, +func testPeerBucketAdoptionNormalizesVersioningWhenEnablingLock(_ ObjectLayer, instanceType, bucketName string, _ http.Handler, _ auth.Credentials, t *testing.T, ) { versioningXML := []byte(`Enabledtruetemporary/`) @@ -106,8 +108,8 @@ func testPeerBucketAdoptionPreservesCustomVersioningWhenEnablingLock(_ ObjectLay if err != nil { t.Fatal(err) } - if !bytes.Equal(after.VersioningConfigXML, before.VersioningConfigXML) || !after.VersioningConfigUpdatedAt.Equal(before.VersioningConfigUpdatedAt) { - t.Fatalf("%s: custom versioning changed while enabling Object Lock", instanceType) + if !bytes.Equal(after.VersioningConfigXML, enabledBucketVersioningConfig) || !after.VersioningConfigUpdatedAt.After(before.VersioningConfigUpdatedAt) { + t.Fatalf("%s: prefix-excluded versioning survived enabling Object Lock: %q", instanceType, after.VersioningConfigXML) } if !bytes.Equal(after.ObjectLockConfigXML, enabledBucketObjectLockConfig) { t.Fatalf("%s: Object Lock was not bootstrapped", instanceType) @@ -152,7 +154,7 @@ func TestEnablePeerBucketVersioningRepairsInvalidConfig(t *testing.T) { meta := newBucketMetadata("bucket") meta.Created = time.Date(2026, time.August, 29, 8, 0, 0, 0, time.UTC) meta.VersioningConfigXML = []byte(``) - if err := enablePeerBucketVersioning(&meta); err != nil { + if err := enablePeerBucketVersioning(&meta, false); err != nil { t.Fatal(err) } if !bytes.Equal(meta.VersioningConfigXML, enabledBucketVersioningConfig) || meta.VersioningConfigUpdatedAt.IsZero() { diff --git a/cmd/site-replication.go b/cmd/site-replication.go index 386d321b7..976437235 100644 --- a/cmd/site-replication.go +++ b/cmd/site-replication.go @@ -889,7 +889,11 @@ func (c *SiteReplicationSys) DeleteBucketHook(ctx context.Context, bucket string return errors.Unwrap(cerr) } -func enablePeerBucketVersioning(meta *BucketMetadata) error { +// enablePeerBucketVersioning turns versioning on for a bucket that is being +// created or adopted. With lockEnabled, Object Lock requires every object to +// be versioned: the S3 API rejects suspended or prefix-excluded versioning on +// a locked bucket, so such a configuration is replaced rather than preserved. +func enablePeerBucketVersioning(meta *BucketMetadata, lockEnabled bool) error { if len(meta.VersioningConfigXML) == 0 { meta.VersioningConfigXML = enabledBucketVersioningConfig if meta.VersioningConfigUpdatedAt.IsZero() { @@ -898,7 +902,7 @@ func enablePeerBucketVersioning(meta *BucketMetadata) error { return nil } config, err := versioning.ParseConfig(bytes.NewReader(meta.VersioningConfigXML)) - if err != nil { + if err != nil || (lockEnabled && (config.Suspended() || config.PrefixesExcluded())) { meta.VersioningConfigXML = enabledBucketVersioningConfig meta.VersioningConfigUpdatedAt = UTCNow() return nil @@ -943,7 +947,7 @@ func (c *SiteReplicationSys) PeerBucketMakeWithVersioningHandler(ctx context.Con } meta.SetCreatedAt(opts.CreatedAt) - if err = enablePeerBucketVersioning(&meta); err != nil { + if err = enablePeerBucketVersioning(&meta, opts.LockEnabled || len(meta.ObjectLockConfigXML) != 0); err != nil { return err } if opts.LockEnabled && len(meta.ObjectLockConfigXML) == 0 {