From 9906b3ade97e55472f178c55628733c706b291a0 Mon Sep 17 00:00:00 2001 From: jiuker <2818723467@qq.com> Date: Wed, 22 May 2024 14:50:03 +0800 Subject: [PATCH] fix: reject ilm rule when bucket LockEnabled (#19785) --- cmd/admin-bucket-handlers.go | 8 ++++++-- cmd/bucket-lifecycle-handlers.go | 5 +++-- cmd/site-replication.go | 8 +++++++- internal/bucket/lifecycle/lifecycle.go | 9 ++++++++- internal/bucket/lifecycle/lifecycle_test.go | 7 ++++--- 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/cmd/admin-bucket-handlers.go b/cmd/admin-bucket-handlers.go index 0b5d96ade..0bd15c80f 100644 --- a/cmd/admin-bucket-handlers.go +++ b/cmd/admin-bucket-handlers.go @@ -837,9 +837,13 @@ func (a adminAPIHandlers) ImportBucketMetadataHandler(w http.ResponseWriter, r * rpt.SetStatus(bucket, fileName, err) continue } - + rcfg, err := globalBucketObjectLockSys.Get(bucket) + if err != nil { + rpt.SetStatus(bucket, fileName, err) + continue + } // Validate the received bucket policy document - if err = bucketLifecycle.Validate(); err != nil { + if err = bucketLifecycle.Validate(rcfg); err != nil { rpt.SetStatus(bucket, fileName, err) continue } diff --git a/cmd/bucket-lifecycle-handlers.go b/cmd/bucket-lifecycle-handlers.go index d983f9b6e..bb7741277 100644 --- a/cmd/bucket-lifecycle-handlers.go +++ b/cmd/bucket-lifecycle-handlers.go @@ -64,7 +64,8 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r } // Check if bucket exists. - if _, err := objAPI.GetBucketInfo(ctx, bucket, BucketOptions{}); err != nil { + rcfg, err := globalBucketObjectLockSys.Get(bucket) + if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } @@ -76,7 +77,7 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r } // Validate the received bucket policy document - if err = bucketLifecycle.Validate(); err != nil { + if err = bucketLifecycle.Validate(rcfg); err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } diff --git a/cmd/site-replication.go b/cmd/site-replication.go index 81ff50519..8cfb7ad98 100644 --- a/cmd/site-replication.go +++ b/cmd/site-replication.go @@ -6211,7 +6211,13 @@ func mergeWithCurrentLCConfig(ctx context.Context, bucket string, expLCCfg *stri Rules: rules, ExpiryUpdatedAt: &updatedAt, } - if err := finalLcCfg.Validate(); err != nil { + + rcfg, err := globalBucketObjectLockSys.Get(bucket) + if err != nil { + return nil, err + } + + if err := finalLcCfg.Validate(rcfg); err != nil { return []byte{}, err } finalConfigData, err := xml.Marshal(finalLcCfg) diff --git a/internal/bucket/lifecycle/lifecycle.go b/internal/bucket/lifecycle/lifecycle.go index d6ba7ae11..ea1e7a05d 100644 --- a/internal/bucket/lifecycle/lifecycle.go +++ b/internal/bucket/lifecycle/lifecycle.go @@ -27,6 +27,7 @@ import ( "time" "github.com/google/uuid" + "github.com/minio/minio/internal/bucket/object/lock" xhttp "github.com/minio/minio/internal/http" ) @@ -236,7 +237,7 @@ func ParseLifecycleConfig(reader io.Reader) (*Lifecycle, error) { } // Validate - validates the lifecycle configuration -func (lc Lifecycle) Validate() error { +func (lc Lifecycle) Validate(lr lock.Retention) error { // Lifecycle config can't have more than 1000 rules if len(lc.Rules) > 1000 { return errLifecycleTooManyRules @@ -251,6 +252,12 @@ func (lc Lifecycle) Validate() error { if err := r.Validate(); err != nil { return err } + if (r.Expiration.DeleteMarker.val || // DeleteVersionAction + !r.DelMarkerExpiration.Empty() || // DelMarkerDeleteAllVersionsAction + !r.NoncurrentVersionExpiration.IsDaysNull() || // DeleteVersionAction + !r.Expiration.IsDaysNull()) && lr.LockEnabled { + return fmt.Errorf("DeleteAllVersions and DeleteMarkerDeleteAllVersions cannot be set when bucket lock is enabled") + } } // Make sure Rule ID is unique for i := range lc.Rules { diff --git a/internal/bucket/lifecycle/lifecycle_test.go b/internal/bucket/lifecycle/lifecycle_test.go index 258d77ed2..766c4c8b8 100644 --- a/internal/bucket/lifecycle/lifecycle_test.go +++ b/internal/bucket/lifecycle/lifecycle_test.go @@ -30,6 +30,7 @@ import ( "github.com/dustin/go-humanize" "github.com/google/uuid" + "github.com/minio/minio/internal/bucket/object/lock" xhttp "github.com/minio/minio/internal/http" ) @@ -144,7 +145,7 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) { // no need to continue this test. return } - err = lc.Validate() + err = lc.Validate(lock.Retention{}) if err != tc.expectedValidationErr { t.Fatalf("%d: Expected %v during validation but got %v", i+1, tc.expectedValidationErr, err) } @@ -779,7 +780,7 @@ func TestHasActiveRules(t *testing.T) { t.Fatalf("Got unexpected error: %v", err) } // To ensure input lifecycle configurations are valid - if err := lc.Validate(); err != nil { + if err := lc.Validate(lock.Retention{}); err != nil { t.Fatalf("Invalid test case: %d %v", i+1, err) } if got := lc.HasActiveRules(tc.prefix); got != tc.want { @@ -1365,7 +1366,7 @@ func TestFilterRules(t *testing.T) { for i, tc := range tests { t.Run(fmt.Sprintf("test-%d", i+1), func(t *testing.T) { - if err := tc.lc.Validate(); err != nil { + if err := tc.lc.Validate(lock.Retention{}); err != nil { t.Fatalf("Lifecycle validation failed - %v", err) } rules := tc.lc.FilterRules(tc.opts)