mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-08-16 04:06:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

s3 remote: honor s3.support_tagging in UpdateFileMetadata (#10532)

Browse Source 
* s3 remote: honor s3.support_tagging in UpdateFileMetadata

The write path already skips tagging when the remote is configured
without tagging support, but the metadata-update path sent
PutObjectTagging or DeleteObjectTagging unconditionally. On remotes
that reject tagging requests, any metadata update failed -- including
updates with no tags at all, which land on the DeleteObjectTagging
branch.

* s3 remote: drop the never-read supportTagging field

Every maker set it, nothing read it: the tagging decision is made from
conf.S3SupportTagging. Keeping a field that looks like the switch but
is not invites exactly the inconsistency the previous commit fixed.
This commit is contained in:
Chris Lu
2026-08-01 20:18:59 -07:00
committed by GitHub
parent 0d7173a029
commit e696c2585e
10 changed files with 86 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/remote_storage/s3/aliyun.go
+1 -2
View File
@@ -25,8 +25,7 @@ func (s AliyunRemoteStorageMaker) HasBucket() bool {
func (s AliyunRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
accessKey := util.Nvl(conf.AliyunAccessKey, os.Getenv("ALICLOUD_ACCESS_KEY_ID"))
secretKey := util.Nvl(conf.AliyunSecretKey, os.Getenv("ALICLOUD_ACCESS_KEY_SECRET"))
weed/remote_storage/s3/backblaze.go
+1 -2
View File
@@ -23,8 +23,7 @@ func (s BackBlazeRemoteStorageMaker) HasBucket() bool {
func (s BackBlazeRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: false,
conf: conf,
conf: conf,
}
config := &aws.Config{
Endpoint: aws.String(conf.BackblazeEndpoint),
weed/remote_storage/s3/baidu.go
+1 -2
View File
@@ -26,8 +26,7 @@ func (s BaiduRemoteStorageMaker) HasBucket() bool {
func (s BaiduRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
accessKey := util.Nvl(conf.BaiduAccessKey, os.Getenv("BDCLOUD_ACCESS_KEY"))
secretKey := util.Nvl(conf.BaiduSecretKey, os.Getenv("BDCLOUD_SECRET_KEY"))
weed/remote_storage/s3/contabo.go
+1 -2
View File
@@ -25,8 +25,7 @@ func (s ContaboRemoteStorageMaker) HasBucket() bool {
func (s ContaboRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
accessKey := util.Nvl(conf.ContaboAccessKey, os.Getenv("ACCESS_KEY"))
secretKey := util.Nvl(conf.ContaboSecretKey, os.Getenv("SECRET_KEY"))
weed/remote_storage/s3/filebase.go
+1 -2
View File
@@ -26,8 +26,7 @@ func (s FilebaseRemoteStorageMaker) HasBucket() bool {
func (s FilebaseRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
accessKey := util.Nvl(conf.FilebaseAccessKey, os.Getenv("AWS_ACCESS_KEY_ID"))
secretKey := util.Nvl(conf.FilebaseSecretKey, os.Getenv("AWS_SECRET_ACCESS_KEY"))
weed/remote_storage/s3/s3_storage_client.go
+11 -7
View File
@@ -47,8 +47,7 @@ func (s s3RemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.R
// a guarded DialContext.
func MakeWithHTTPClient(conf *remote_pb.RemoteConf, httpClient *http.Client) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
config := &aws.Config{
Region: aws.String(conf.S3Region),
@@ -82,12 +81,11 @@ func MakeWithHTTPClient(conf *remote_pb.RemoteConf, httpClient *http.Client) (re
}
type s3RemoteStorageClient struct {
conf *remote_pb.RemoteConf
conn s3iface.S3API
supportTagging bool
conf *remote_pb.RemoteConf
conn s3iface.S3API
}
var _ = remote_storage.RemoteStorageClient(&s3RemoteStorageClient{supportTagging: true})
var _ = remote_storage.RemoteStorageClient(&s3RemoteStorageClient{})
func (s *s3RemoteStorageClient) Traverse(remote *remote_pb.RemoteStorageLocation, visitFn remote_storage.VisitFunc) (err error) {
@@ -470,12 +468,18 @@ func (s *s3RemoteStorageClient) UpdateFileMetadata(loc *remote_pb.RemoteStorageL
}
}
// same as the write path: a remote without tagging support rejects both
// PutObjectTagging and DeleteObjectTagging
if !s.conf.S3SupportTagging {
return
}
tagging := toTagging(newEntry.Extended)
if len(tagging.TagSet) > 0 {
_, err = s.conn.PutObjectTagging(&s3.PutObjectTaggingInput{
Bucket: aws.String(loc.Bucket),
Key: aws.String(loc.Path[1:]),
Tagging: toTagging(newEntry.Extended),
Tagging: tagging,
})
} else {
_, err = s.conn.DeleteObjectTagging(&s3.DeleteObjectTaggingInput{
weed/remote_storage/s3/s3_storage_client_test.go
+67
View File
@@ -258,6 +258,73 @@ func TestS3WriteFilePassesMimeAsContentType(t *testing.T) {
require.Equal(t, "text/html", rt.uploadContentType(), "Content-Type should match entry.Attributes.Mime")
}
// recordingRoundTripper records every request and answers all of them with a 200.
type recordingRoundTripper struct {
requests []*http.Request
}
func (c *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
c.requests = append(c.requests, req.Clone(req.Context()))
if req.Body != nil {
_, _ = io.Copy(io.Discard, req.Body)
_ = req.Body.Close()
}
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader("")),
Header: http.Header{},
Request: req,
}, nil
}
func newRecordingS3Client(t *testing.T, supportTagging bool) (*s3RemoteStorageClient, *recordingRoundTripper) {
t.Helper()
rt := &recordingRoundTripper{}
conf := &remote_pb.RemoteConf{
Name: "test",
S3Region: "us-east-1",
S3Endpoint: "https://example.invalid",
S3ForcePathStyle: true,
S3AccessKey: "test-key",
S3SecretKey: "test-secret",
S3SupportTagging: supportTagging,
}
rs, err := MakeWithHTTPClient(conf, &http.Client{Transport: rt})
require.NoError(t, err)
return rs.(*s3RemoteStorageClient), rt
}
func TestS3UpdateFileMetadataSkipsTaggingWhenUnsupported(t *testing.T) {
client, rt := newRecordingS3Client(t, false)
loc := &remote_pb.RemoteStorageLocation{Name: "test", Bucket: "bucket", Path: "/dir/file.bin"}
plain := &filer_pb.Entry{}
tagged := &filer_pb.Entry{Extended: map[string][]byte{"k1": []byte("v1")}}
require.NoError(t, client.UpdateFileMetadata(loc, plain, tagged))
require.Empty(t, rt.requests, "adding attributes must not send PutObjectTagging")
require.NoError(t, client.UpdateFileMetadata(loc, tagged, plain))
require.Empty(t, rt.requests, "removing attributes must not send DeleteObjectTagging")
}
func TestS3UpdateFileMetadataSendsTaggingWhenSupported(t *testing.T) {
client, rt := newRecordingS3Client(t, true)
loc := &remote_pb.RemoteStorageLocation{Name: "test", Bucket: "bucket", Path: "/dir/file.bin"}
plain := &filer_pb.Entry{}
tagged := &filer_pb.Entry{Extended: map[string][]byte{"k1": []byte("v1")}}
require.NoError(t, client.UpdateFileMetadata(loc, plain, tagged))
require.Len(t, rt.requests, 1)
require.Equal(t, http.MethodPut, rt.requests[0].Method)
require.Contains(t, rt.requests[0].URL.RawQuery, "tagging")
rt.requests = nil
require.NoError(t, client.UpdateFileMetadata(loc, tagged, plain))
require.Len(t, rt.requests, 1)
require.Equal(t, http.MethodDelete, rt.requests[0].Method)
require.Contains(t, rt.requests[0].URL.RawQuery, "tagging")
}
func TestS3WriteFileOmitsContentTypeWhenMimeMissing(t *testing.T) {
client, rt := newCapturingS3Client(t)
loc := &remote_pb.RemoteStorageLocation{
weed/remote_storage/s3/storj.go
+1 -2
View File
@@ -25,8 +25,7 @@ func (s StorjRemoteStorageMaker) HasBucket() bool {
func (s StorjRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
accessKey := util.Nvl(conf.StorjAccessKey, os.Getenv("AWS_ACCESS_KEY_ID"))
secretKey := util.Nvl(conf.StorjSecretKey, os.Getenv("AWS_SECRET_ACCESS_KEY"))
weed/remote_storage/s3/tencent.go
+1 -2
View File
@@ -25,8 +25,7 @@ func (s TencentRemoteStorageMaker) HasBucket() bool {
func (s TencentRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
accessKey := util.Nvl(conf.TencentSecretId, os.Getenv("COS_SECRETID"))
secretKey := util.Nvl(conf.TencentSecretKey, os.Getenv("COS_SECRETKEY"))
weed/remote_storage/s3/wasabi.go
+1 -2
View File
@@ -25,8 +25,7 @@ func (s WasabiRemoteStorageMaker) HasBucket() bool {
func (s WasabiRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
client := &s3RemoteStorageClient{
supportTagging: true,
conf: conf,
conf: conf,
}
accessKey := util.Nvl(conf.WasabiAccessKey)
secretKey := util.Nvl(conf.WasabiSecretKey)