From 518009dcd4b15226e4ba2201009007dc1ed86afb Mon Sep 17 00:00:00 2001 From: Tsic Date: Wed, 11 Nov 2026 13:12:49 +0800 Subject: [PATCH] filer: apply filerGroup prefix to bucket default collection (#12499) filer: prefix bucket default collection, assign copy collection explicitly S3 server-side copy paths (CopyObject/UploadPartCopy) assign volumes with an empty collection and relied on the filer's bucket fallback. The fallback used the raw bucket name, while PutObject uses getCollectionName(bucket) = _, so copies landed in a second, unprefixed collection. Prefix the filer's bucket default with the filer group, and have the S3 copy path pass that collection explicitly, like PutObject. Direct filer writes keep path-rule precedence, so this only aligns S3 copy with S3 put. Adds regression tests for the fallback, explicit collection precedence, and the S3 copy collection resolver. --- weed/s3api/bucket_paths.go | 22 +++ weed/s3api/s3api_object_handlers_copy.go | 7 +- ...pi_object_handlers_copy_collection_test.go | 25 ++++ .../filer_server_bucket_collection_test.go | 140 ++++++++++++++++++ weed/server/filer_server_handlers_write.go | 23 ++- 5 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 weed/server/filer_server_bucket_collection_test.go diff --git a/weed/s3api/bucket_paths.go b/weed/s3api/bucket_paths.go index 2d70a03826..e89e946327 100644 --- a/weed/s3api/bucket_paths.go +++ b/weed/s3api/bucket_paths.go @@ -53,6 +53,28 @@ func (s3a *S3ApiServer) bucketDir(bucket string) string { return path.Join(s3a.bucketRoot(bucket), bucket) } +// collectionForCopyPath returns the collection a server-side copy must assign +// its destination chunks to. It mirrors PutObject: with a filer group the +// bucket's prefixed collection wins over a path rule, so a copy and a put of +// the same bucket cannot split into two collections. +func (s3a *S3ApiServer) collectionForCopyPath(dstPath string) string { + if s3a.option.FilerGroup == "" || s3a.option.BucketsPath == "" { + return "" + } + prefix := strings.TrimSuffix(s3a.option.BucketsPath, "/") + "/" + if !strings.HasPrefix(dstPath, prefix) { + return "" + } + bucket := strings.TrimPrefix(dstPath, prefix) + if i := strings.Index(bucket, "/"); i >= 0 { + bucket = bucket[:i] + } + if bucket == "" { + return "" + } + return s3a.getCollectionName(bucket) +} + func (s3a *S3ApiServer) validateTableBucketObjectPath(bucket, object string) error { if !s3a.isTableBucket(bucket) { return nil diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index fa991cca91..7b94772c4a 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -1653,14 +1653,17 @@ func (s3a *S3ApiServer) copySingleChunkForRange(originalChunk, rangeChunk *filer return dstChunk, nil } -// assignNewVolume assigns a new volume for the chunk +// assignNewVolume assigns a new volume for the chunk. The collection is passed +// explicitly, like PutObject, so a copy does not depend on the filer's rule +// precedence to resolve the destination bucket's collection. func (s3a *S3ApiServer) assignNewVolume(dstPath string, expectedDataSize uint64) (*filer_pb.AssignVolumeResponse, error) { + collection := s3a.collectionForCopyPath(dstPath) var assignResult *filer_pb.AssignVolumeResponse err := s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { resp, err := client.AssignVolume(context.Background(), &filer_pb.AssignVolumeRequest{ Count: 1, Replication: "", - Collection: "", + Collection: collection, DiskType: "", DataCenter: s3a.option.DataCenter, Path: dstPath, diff --git a/weed/s3api/s3api_object_handlers_copy_collection_test.go b/weed/s3api/s3api_object_handlers_copy_collection_test.go index 165cea5b3c..c153628ca1 100644 --- a/weed/s3api/s3api_object_handlers_copy_collection_test.go +++ b/weed/s3api/s3api_object_handlers_copy_collection_test.go @@ -73,3 +73,28 @@ func TestMultipartStorageRuleFollowsDestinationObject(t *testing.T) { t.Fatalf("destination path %q resolved ttl %q, want 30d", dstPath, got) } } + +// Server-side copies pass the grouped bucket collection explicitly, matching +// PutObject, instead of relying on the filer's rule precedence. +func TestCollectionForCopyPath(t *testing.T) { + s3a := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets", FilerGroup: "group1"}} + cases := []struct { + path string + want string + }{ + {"/buckets/bucket1/key", "group1_bucket1"}, + {"/buckets/bucket1/.uploads/uploadid/1", "group1_bucket1"}, + {"/buckets/", ""}, + {"/other/path", ""}, + } + for _, tc := range cases { + if got := s3a.collectionForCopyPath(tc.path); got != tc.want { + t.Fatalf("collectionForCopyPath(%q) = %q, want %q", tc.path, got, tc.want) + } + } + + noGroup := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets"}} + if got := noGroup.collectionForCopyPath("/buckets/bucket1/key"); got != "" { + t.Fatalf("collectionForCopyPath without filer group = %q, want empty", got) + } +} diff --git a/weed/server/filer_server_bucket_collection_test.go b/weed/server/filer_server_bucket_collection_test.go new file mode 100644 index 0000000000..a31677e747 --- /dev/null +++ b/weed/server/filer_server_bucket_collection_test.go @@ -0,0 +1,140 @@ +package weed_server + +import ( + "context" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/filer" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" +) + +func TestBucketDefaultCollection(t *testing.T) { + cases := []struct { + name string + filerGroup string + path string + want string + }{ + { + name: "filer group prefixes bucket", + filerGroup: "group1", + path: "/buckets/bucket1/path/to/object", + want: "group1_bucket1", + }, + { + name: "no filer group keeps bucket name", + path: "/buckets/bucket1/path/to/object", + want: "bucket1", + }, + { + name: "bucket root still resolves", + filerGroup: "group1", + path: "/buckets/bucket1", + want: "group1_bucket1", + }, + { + name: "unrelated path has no bucket collection", + filerGroup: "group1", + path: "/other/path", + want: "", + }, + { + name: "buckets folder itself has no bucket", + filerGroup: "group1", + path: "/buckets/", + want: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fs := &FilerServer{ + option: &FilerOption{FilerGroup: tc.filerGroup}, + filer: &filer.Filer{DirBucketsPath: "/buckets"}, + } + if got := fs.bucketDefaultCollection(tc.path); got != tc.want { + t.Fatalf("bucketDefaultCollection(%q) = %q, want %q", tc.path, got, tc.want) + } + }) + } +} + +func TestDetectStorageOptionUsesFilerGroupBucketCollection(t *testing.T) { + fs := &FilerServer{ + option: &FilerOption{FilerGroup: "group1", DefaultReplication: "001"}, + filer: &filer.Filer{ + DirBucketsPath: "/buckets", + FilerConf: filer.NewFilerConf(), + }, + } + + so, err := fs.detectStorageOption(context.Background(), "/buckets/bucket1/key", "", "", 0, "", "", "", "") + if err != nil { + t.Fatalf("detectStorageOption: %v", err) + } + if so.Collection != "group1_bucket1" { + t.Fatalf("collection = %q, want %q", so.Collection, "group1_bucket1") + } +} + +func newBucketRuleFiler(t *testing.T) *filer.Filer { + t.Helper() + fc := filer.NewFilerConf() + if err := fc.SetLocationConf(&filer_pb.FilerConf_PathConf{ + LocationPrefix: "/buckets/bucket1", + Collection: "archive", + }); err != nil { + t.Fatalf("SetLocationConf: %v", err) + } + return &filer.Filer{DirBucketsPath: "/buckets", FilerConf: fc} +} + +// Direct filer writes keep the path rule. S3 copy passes the grouped bucket +// collection explicitly, so it overrides the rule the same way PutObject does. +func TestDetectStorageOptionPreservesRuleForDirectWrites(t *testing.T) { + fs := &FilerServer{ + option: &FilerOption{FilerGroup: "group1", DefaultReplication: "001"}, + filer: newBucketRuleFiler(t), + } + + so, err := fs.detectStorageOption(context.Background(), "/buckets/bucket1/key", "", "", 0, "", "", "", "") + if err != nil { + t.Fatalf("detectStorageOption: %v", err) + } + if so.Collection != "archive" { + t.Fatalf("collection = %q, want %q", so.Collection, "archive") + } +} + +// An explicit request collection wins over both the grouped bucket default and +// the path rule; S3 copy and PutObject rely on this precedence. +func TestDetectStorageOptionExplicitCollectionWins(t *testing.T) { + fs := &FilerServer{ + option: &FilerOption{FilerGroup: "group1", DefaultReplication: "001"}, + filer: newBucketRuleFiler(t), + } + + so, err := fs.detectStorageOption(context.Background(), "/buckets/bucket1/key", "explicit", "", 0, "", "", "", "") + if err != nil { + t.Fatalf("detectStorageOption: %v", err) + } + if so.Collection != "explicit" { + t.Fatalf("collection = %q, want %q", so.Collection, "explicit") + } +} + +// Without a filer group the path rule still names the collection, as before. +func TestDetectStorageOptionRuleWinsWithoutFilerGroup(t *testing.T) { + fs := &FilerServer{ + option: &FilerOption{DefaultReplication: "001"}, + filer: newBucketRuleFiler(t), + } + + so, err := fs.detectStorageOption(context.Background(), "/buckets/bucket1/key", "", "", 0, "", "", "", "") + if err != nil { + t.Fatalf("detectStorageOption: %v", err) + } + if so.Collection != "archive" { + t.Fatalf("collection = %q, want %q", so.Collection, "archive") + } +} diff --git a/weed/server/filer_server_handlers_write.go b/weed/server/filer_server_handlers_write.go index 28f1b61f3b..8c73bb0e4d 100644 --- a/weed/server/filer_server_handlers_write.go +++ b/weed/server/filer_server_handlers_write.go @@ -280,6 +280,21 @@ func (fs *FilerServer) DeleteHandler(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +// bucketDefaultCollection returns the collection a write under the buckets +// folder falls back to when neither the request nor a filer.conf rule names +// one. S3 bucket collections are namespaced by filer group; S3 copy passes the +// prefixed collection explicitly, so direct filer writes keep rule priority. +func (fs *FilerServer) bucketDefaultCollection(requestURI string) string { + if !strings.HasPrefix(requestURI, fs.filer.DirBucketsPath+"/") { + return "" + } + bucket := fs.filer.DetectBucket(util.FullPath(requestURI)) + if bucket == "" || fs.option.FilerGroup == "" { + return bucket + } + return fmt.Sprintf("%s_%s", fs.option.FilerGroup, bucket) +} + func (fs *FilerServer) detectStorageOption(ctx context.Context, requestURI, qCollection, qReplication string, ttlSeconds int32, diskType, dataCenter, rack, dataNode string) (*operation.StorageOption, error) { rule := fs.filer.FilerConf.MatchStorageRule(requestURI) @@ -300,11 +315,9 @@ func (fs *FilerServer) detectStorageOption(ctx context.Context, requestURI, qCol maxFileNameLength = fs.filer.MaxFilenameLength } - // required by buckets folder - bucketDefaultCollection := "" - if strings.HasPrefix(requestURI, fs.filer.DirBucketsPath+"/") { - bucketDefaultCollection = fs.filer.DetectBucket(util.FullPath(requestURI)) - } + // required by buckets folder; direct writes keep path-rule precedence. + // S3 copy assigns the grouped bucket collection explicitly. + bucketDefaultCollection := fs.bucketDefaultCollection(requestURI) if ttlSeconds == 0 { ttl, err := needle.ReadTTL(rule.GetTtl())