From efbed39e255465cdc4f3323a1bea4f0c1455b03b Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 2 Apr 2026 11:51:54 -0700 Subject: [PATCH 1/3] S3: map canned ACL to file permissions and add configurable default file mode (#8886) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * S3: map canned ACL to file permissions and add configurable default file mode S3 uploads were hardcoded to 0660 regardless of ACL headers. Now the X-Amz-Acl header maps to Unix file permissions per-object: - public-read, authenticated-read, bucket-owner-read → 0644 - public-read-write → 0666 - private, bucket-owner-full-control → 0660 Also adds -defaultFileMode / -s3.defaultFileMode flag to set a server-wide default when no ACL header is present. Closes #8874 * Address review feedback for S3 file mode feature - Extract hardcoded 0660 to defaultFileMode constant - Change parseDefaultFileMode to return error instead of calling Fatalf - Add -s3.defaultFileMode flag to filer.go and mini.go (was missing) - Add doc comment to S3Options about updating all four flag sites - Add TestResolveFileMode with 10 test cases covering ACL mapping, server default, and priority ordering --- weed/command/filer.go | 1 + weed/command/mini.go | 1 + weed/command/s3.go | 23 ++++++++++++ weed/command/server.go | 1 + weed/s3api/s3api_object_handlers_put.go | 25 ++++++++++++- weed/s3api/s3api_object_handlers_put_test.go | 38 ++++++++++++++++++++ weed/s3api/s3api_server.go | 1 + 7 files changed, 89 insertions(+), 1 deletion(-) diff --git a/weed/command/filer.go b/weed/command/filer.go index d6a990ebd..edfd8afef 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -146,6 +146,7 @@ func init() { filerS3Options.iamReadOnly = cmdFiler.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") filerS3Options.portIceberg = cmdFiler.Flag.Int("s3.port.iceberg", 8181, "Iceberg REST Catalog server listen port (0 to disable)") filerS3Options.externalUrl = cmdFiler.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + filerS3Options.defaultFileMode = cmdFiler.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") // start webdav on filer filerStartWebDav = cmdFiler.Flag.Bool("webdav", false, "whether to start webdav gateway") diff --git a/weed/command/mini.go b/weed/command/mini.go index b5a3bd367..38257bdff 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -250,6 +250,7 @@ func initMiniS3Flags() { miniS3Options.auditLogConfig = cmdMini.Flag.String("s3.auditLogConfig", "", "path to the audit log config file") miniS3Options.allowDeleteBucketNotEmpty = miniS3AllowDeleteBucketNotEmpty miniS3Options.externalUrl = cmdMini.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + miniS3Options.defaultFileMode = cmdMini.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") // In mini mode, S3 uses the shared debug server started at line 681, not its own separate debug server miniS3Options.debug = new(bool) // explicitly false miniS3Options.debugPort = cmdMini.Flag.Int("s3.debug.port", 6060, "http port for debugging (unused in mini mode)") diff --git a/weed/command/s3.go b/weed/command/s3.go index 081e84f13..dc2191bcf 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -10,6 +10,7 @@ import ( "net/http" "os" "runtime" + "strconv" "strings" "time" @@ -36,6 +37,9 @@ var ( s3StandaloneOptions S3Options ) +// S3Options holds CLI flags for the S3 gateway. +// Flags are registered in multiple commands: s3.go (standalone), server.go, filer.go, and mini.go. +// When adding a new field, update all four flag registration sites. type S3Options struct { filer *string bindIp *string @@ -68,6 +72,7 @@ type S3Options struct { debugPort *int cipher *bool externalUrl *string + defaultFileMode *string } func init() { @@ -103,6 +108,7 @@ func init() { s3StandaloneOptions.debugPort = cmdS3.Flag.Int("debug.port", 6060, "http port for debugging") s3StandaloneOptions.cipher = cmdS3.Flag.Bool("encryptVolumeData", false, "encrypt data on volume servers") s3StandaloneOptions.externalUrl = cmdS3.Flag.String("externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3StandaloneOptions.defaultFileMode = cmdS3.Flag.String("defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") } var cmdS3 = &Command{ @@ -232,6 +238,17 @@ func (s3opt *S3Options) resolveExternalUrl() string { return os.Getenv("S3_EXTERNAL_URL") } +func (s3opt *S3Options) parseDefaultFileMode() (uint32, error) { + if s3opt.defaultFileMode == nil || *s3opt.defaultFileMode == "" { + return 0, nil + } + mode, err := strconv.ParseUint(*s3opt.defaultFileMode, 8, 32) + if err != nil { + return 0, fmt.Errorf("invalid defaultFileMode %q: %v", *s3opt.defaultFileMode, err) + } + return uint32(mode), nil +} + func (s3opt *S3Options) startS3Server() bool { filerAddresses := pb.ServerAddresses(*s3opt.filer).ToAddresses() @@ -298,6 +315,11 @@ func (s3opt *S3Options) startS3Server() bool { *s3opt.bindIp = "0.0.0.0" } + defaultFileMode, fileModeErr := s3opt.parseDefaultFileMode() + if fileModeErr != nil { + glog.Fatalf("S3 API Server startup error: %v", fileModeErr) + } + s3ApiServer, s3ApiServer_err = s3api.NewS3ApiServer(router, &s3api.S3ApiServerOption{ Filers: filerAddresses, Masters: masterAddresses, @@ -320,6 +342,7 @@ func (s3opt *S3Options) startS3Server() bool { BindIp: *s3opt.bindIp, GrpcPort: *s3opt.portGrpc, ExternalUrl: s3opt.resolveExternalUrl(), + DefaultFileMode: defaultFileMode, }) if s3ApiServer_err != nil { glog.Fatalf("S3 API Server startup error: %v", s3ApiServer_err) diff --git a/weed/command/server.go b/weed/command/server.go index a2759845d..deb1c5e71 100644 --- a/weed/command/server.go +++ b/weed/command/server.go @@ -180,6 +180,7 @@ func init() { s3Options.iamReadOnly = cmdServer.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") s3Options.cipher = cmdServer.Flag.Bool("s3.encryptVolumeData", false, "encrypt data on volume servers for S3 uploads") s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3Options.defaultFileMode = cmdServer.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") sftpOptions.port = cmdServer.Flag.Int("sftp.port", 2022, "SFTP server listen port") sftpOptions.sshPrivateKey = cmdServer.Flag.String("sftp.sshPrivateKey", "", "path to the SSH private key file for host authentication") diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 5419a4852..805d63133 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -603,13 +603,14 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader } // Create entry + fileMode := s3a.resolveFileMode(r) entry := &filer_pb.Entry{ Name: path.Base(filePath), IsDirectory: false, Attributes: &filer_pb.FuseAttributes{ Crtime: now.Unix(), Mtime: now.Unix(), - FileMode: 0660, + FileMode: fileMode, Uid: 0, Gid: 0, Mime: mimeType, @@ -815,6 +816,28 @@ func (s3a *S3ApiServer) putToFiler(r *http.Request, filePath string, dataReader return etag, s3err.ErrNone, responseMetadata } +const defaultFileMode = uint32(0660) + +// resolveFileMode determines the file permission mode for an S3 upload. +// Priority: per-object X-Amz-Acl header > server default > defaultFileMode. +func (s3a *S3ApiServer) resolveFileMode(r *http.Request) uint32 { + if cannedAcl := r.Header.Get(s3_constants.AmzCannedAcl); cannedAcl != "" { + switch cannedAcl { + case s3_constants.CannedAclPublicRead, s3_constants.CannedAclAuthenticatedRead, + s3_constants.CannedAclBucketOwnerRead: + return 0644 + case s3_constants.CannedAclPublicReadWrite: + return 0666 + case s3_constants.CannedAclPrivate, s3_constants.CannedAclBucketOwnerFullControl: + return defaultFileMode + } + } + if s3a.option.DefaultFileMode != 0 { + return s3a.option.DefaultFileMode + } + return defaultFileMode +} + func setEtag(w http.ResponseWriter, etag string) { if etag != "" { if strings.HasPrefix(etag, "\"") { diff --git a/weed/s3api/s3api_object_handlers_put_test.go b/weed/s3api/s3api_object_handlers_put_test.go index eaad6ab6d..a5646bff7 100644 --- a/weed/s3api/s3api_object_handlers_put_test.go +++ b/weed/s3api/s3api_object_handlers_put_test.go @@ -301,3 +301,41 @@ func TestWithObjectWriteLockSerializesConcurrentPreconditions(t *testing.T) { t.Fatalf("expected %d precondition failures, got %d", workers-1, preconditionFailedCount) } } + +func TestResolveFileMode(t *testing.T) { + tests := []struct { + name string + acl string + defaultFileMode uint32 + expected uint32 + }{ + {"no acl, no default", "", 0, 0660}, + {"no acl, with default", "", 0644, 0644}, + {"private", s3_constants.CannedAclPrivate, 0, 0660}, + {"private overrides default", s3_constants.CannedAclPrivate, 0644, 0660}, + {"public-read", s3_constants.CannedAclPublicRead, 0, 0644}, + {"public-read overrides default", s3_constants.CannedAclPublicRead, 0666, 0644}, + {"public-read-write", s3_constants.CannedAclPublicReadWrite, 0, 0666}, + {"authenticated-read", s3_constants.CannedAclAuthenticatedRead, 0, 0644}, + {"bucket-owner-read", s3_constants.CannedAclBucketOwnerRead, 0, 0644}, + {"bucket-owner-full-control", s3_constants.CannedAclBucketOwnerFullControl, 0, 0660}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s3a := &S3ApiServer{ + option: &S3ApiServerOption{ + DefaultFileMode: tt.defaultFileMode, + }, + } + req := httptest.NewRequest(http.MethodPut, "/bucket/object", nil) + if tt.acl != "" { + req.Header.Set(s3_constants.AmzCannedAcl, tt.acl) + } + got := s3a.resolveFileMode(req) + if got != tt.expected { + t.Errorf("resolveFileMode() = %04o, want %04o", got, tt.expected) + } + }) + } +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 08e376aec..35f3684d3 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -60,6 +60,7 @@ type S3ApiServerOption struct { BindIp string GrpcPort int ExternalUrl string // external URL clients use, for signature verification behind a reverse proxy + DefaultFileMode uint32 // default file permission mode for S3 uploads (e.g. 0660, 0644) } type S3ApiServer struct { From 888c32cbde334f86d6a7ce89885a68f3084b7755 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 2 Apr 2026 11:54:19 -0700 Subject: [PATCH 2/3] fix(admin): respect urlPrefix in S3 bucket and S3Tables navigation links (#8885) * fix(admin): respect urlPrefix in S3 bucket and S3Tables navigation links (#8884) Several admin UI templates used hardcoded URLs (templ.SafeURL) instead of dash.PUrl(ctx, ...) for navigation links, causing 404 errors when the admin is deployed with --urlPrefix. Fixed in: s3_buckets.templ, s3tables_buckets.templ, s3tables_tables.templ * fix(admin): URL-escape bucketName in S3Tables navigation links Add url.PathEscape(bucketName) for consistency and correctness in s3tables_tables.templ (back-to-namespaces link) and s3tables_buckets.templ (namespace link), matching the escaping already used in the table details link. --- weed/admin/view/app/s3_buckets.templ | 4 +- weed/admin/view/app/s3_buckets_templ.go | 8 ++-- weed/admin/view/app/s3tables_buckets.templ | 3 +- weed/admin/view/app/s3tables_buckets_templ.go | 41 ++++++++++--------- weed/admin/view/app/s3tables_tables.templ | 4 +- weed/admin/view/app/s3tables_tables_templ.go | 8 ++-- 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/weed/admin/view/app/s3_buckets.templ b/weed/admin/view/app/s3_buckets.templ index bd7ff71f1..15e59fb93 100644 --- a/weed/admin/view/app/s3_buckets.templ +++ b/weed/admin/view/app/s3_buckets.templ @@ -163,7 +163,7 @@ templ S3Buckets(data dash.S3BucketsData) { for _, bucket := range data.Buckets { - {bucket.Name} @@ -236,7 +236,7 @@ templ S3Buckets(data dash.S3BucketsData) {
- diff --git a/weed/admin/view/app/s3_buckets_templ.go b/weed/admin/view/app/s3_buckets_templ.go index 787305812..87996fa41 100644 --- a/weed/admin/view/app/s3_buckets_templ.go +++ b/weed/admin/view/app/s3_buckets_templ.go @@ -171,9 +171,9 @@ func S3Buckets(data dash.S3BucketsData) templ.Component { return templ_7745c5c3_Err } var templ_7745c5c3_Var5 templ.SafeURL - templ_7745c5c3_Var5, templ_7745c5c3_Err = templ.JoinURLErrs(templ.SafeURL(fmt.Sprintf("/files?path=/buckets/%s", bucket.Name))) + templ_7745c5c3_Var5, templ_7745c5c3_Err = templ.JoinURLErrs(dash.PUrl(ctx, fmt.Sprintf("/files?path=/buckets/%s", bucket.Name))) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3_buckets.templ`, Line: 166, Col: 123} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3_buckets.templ`, Line: 166, Col: 124} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var5)) if templ_7745c5c3_Err != nil { @@ -439,9 +439,9 @@ func S3Buckets(data dash.S3BucketsData) templ.Component { return templ_7745c5c3_Err } var templ_7745c5c3_Var19 templ.SafeURL - templ_7745c5c3_Var19, templ_7745c5c3_Err = templ.JoinURLErrs(templ.SafeURL(fmt.Sprintf("/files?path=/buckets/%s", bucket.Name))) + templ_7745c5c3_Var19, templ_7745c5c3_Err = templ.JoinURLErrs(dash.PUrl(ctx, fmt.Sprintf("/files?path=/buckets/%s", bucket.Name))) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3_buckets.templ`, Line: 239, Col: 127} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3_buckets.templ`, Line: 239, Col: 128} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var19)) if templ_7745c5c3_Err != nil { diff --git a/weed/admin/view/app/s3tables_buckets.templ b/weed/admin/view/app/s3tables_buckets.templ index a148a4b34..078d8ec65 100644 --- a/weed/admin/view/app/s3tables_buckets.templ +++ b/weed/admin/view/app/s3tables_buckets.templ @@ -2,6 +2,7 @@ package app import ( "fmt" + "net/url" "github.com/seaweedfs/seaweedfs/weed/admin/dash" "github.com/seaweedfs/seaweedfs/weed/s3api/s3tables" @@ -152,7 +153,7 @@ templ S3TablesBuckets(data dash.S3TablesBucketsData) {
{{ bucketName, parseErr := s3tables.ParseBucketNameFromARN(bucket.ARN) }} if parseErr == nil { - + } else { diff --git a/weed/admin/view/app/s3tables_buckets_templ.go b/weed/admin/view/app/s3tables_buckets_templ.go index 27e1f34f1..6d6a008ab 100644 --- a/weed/admin/view/app/s3tables_buckets_templ.go +++ b/weed/admin/view/app/s3tables_buckets_templ.go @@ -10,6 +10,7 @@ import templruntime "github.com/a-h/templ/runtime" import ( "fmt" + "net/url" "github.com/seaweedfs/seaweedfs/weed/admin/dash" "github.com/seaweedfs/seaweedfs/weed/s3api/s3tables" @@ -48,7 +49,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var2 templ.SafeURL templ_7745c5c3_Var2, templ_7745c5c3_Err = templ.JoinURLErrs(templ.SafeURL(fmt.Sprintf("http://localhost:%d/v1/config", data.IcebergPort))) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 23, Col: 124} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 24, Col: 124} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var2)) if templ_7745c5c3_Err != nil { @@ -66,7 +67,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var3 string templ_7745c5c3_Var3, templ_7745c5c3_Err = templ.JoinStringErrs(fmt.Sprintf("%d", data.IcebergPort)) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 30, Col: 91} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 31, Col: 91} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var3)) if templ_7745c5c3_Err != nil { @@ -84,7 +85,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var4 string templ_7745c5c3_Var4, templ_7745c5c3_Err = templ.JoinStringErrs(fmt.Sprintf("%d", data.IcebergPort)) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 37, Col: 109} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 38, Col: 109} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var4)) if templ_7745c5c3_Err != nil { @@ -97,7 +98,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var5 string templ_7745c5c3_Var5, templ_7745c5c3_Err = templ.JoinStringErrs(fmt.Sprintf("%d", data.IcebergPort)) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 39, Col: 107} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 40, Col: 107} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var5)) if templ_7745c5c3_Err != nil { @@ -120,7 +121,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var6 string templ_7745c5c3_Var6, templ_7745c5c3_Err = templ.JoinStringErrs(fmt.Sprintf("%d", data.TotalBuckets)) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 69, Col: 47} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 70, Col: 47} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var6)) if templ_7745c5c3_Err != nil { @@ -133,7 +134,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var7 string templ_7745c5c3_Var7, templ_7745c5c3_Err = templ.JoinStringErrs(data.LastUpdated.Format("2006-01-02 15:04")) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 88, Col: 54} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 89, Col: 54} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var7)) if templ_7745c5c3_Err != nil { @@ -147,7 +148,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var8 string templ_7745c5c3_Var8, templ_7745c5c3_Err = templ.JoinStringErrs(fmt.Sprintf("%d", data.IcebergPort)) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 108, Col: 47} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 109, Col: 47} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var8)) if templ_7745c5c3_Err != nil { @@ -171,7 +172,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var9 string templ_7745c5c3_Var9, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.Name) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 146, Col: 28} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 147, Col: 28} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var9)) if templ_7745c5c3_Err != nil { @@ -184,7 +185,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var10 string templ_7745c5c3_Var10, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.OwnerAccountID) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 147, Col: 38} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 148, Col: 38} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var10)) if templ_7745c5c3_Err != nil { @@ -197,7 +198,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var11 string templ_7745c5c3_Var11, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.ARN) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 148, Col: 52} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 149, Col: 52} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var11)) if templ_7745c5c3_Err != nil { @@ -210,7 +211,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var12 string templ_7745c5c3_Var12, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.Name) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 149, Col: 52} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 150, Col: 52} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var12)) if templ_7745c5c3_Err != nil { @@ -223,7 +224,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var13 string templ_7745c5c3_Var13, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.CreatedAt.Format("2006-01-02 15:04")) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 150, Col: 60} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 151, Col: 60} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var13)) if templ_7745c5c3_Err != nil { @@ -240,9 +241,9 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { return templ_7745c5c3_Err } var templ_7745c5c3_Var14 templ.SafeURL - templ_7745c5c3_Var14, templ_7745c5c3_Err = templ.JoinURLErrs(templ.SafeURL(fmt.Sprintf("/object-store/s3tables/buckets/%s/namespaces", bucketName))) + templ_7745c5c3_Var14, templ_7745c5c3_Err = templ.JoinURLErrs(dash.PUrl(ctx, fmt.Sprintf("/object-store/s3tables/buckets/%s/namespaces", url.PathEscape(bucketName)))) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 155, Col: 149} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 156, Col: 166} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var14)) if templ_7745c5c3_Err != nil { @@ -265,7 +266,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var15 string templ_7745c5c3_Var15, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.ARN) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 163, Col: 122} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 164, Col: 122} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var15)) if templ_7745c5c3_Err != nil { @@ -278,7 +279,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var16 string templ_7745c5c3_Var16, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.ARN) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 166, Col: 126} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 167, Col: 126} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var16)) if templ_7745c5c3_Err != nil { @@ -291,7 +292,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var17 string templ_7745c5c3_Var17, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.ARN) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 169, Col: 128} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 170, Col: 128} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var17)) if templ_7745c5c3_Err != nil { @@ -304,7 +305,7 @@ func S3TablesBuckets(data dash.S3TablesBucketsData) templ.Component { var templ_7745c5c3_Var18 string templ_7745c5c3_Var18, templ_7745c5c3_Err = templ.JoinStringErrs(bucket.Name) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 169, Col: 161} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 170, Col: 161} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var18)) if templ_7745c5c3_Err != nil { @@ -342,7 +343,7 @@ CREATE SECRET ( SELECT * FROM iceberg_scan('s3://my-table-bucket/my-namespace/my-table');`) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 219, Col: 74} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 220, Col: 74} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var19)) if templ_7745c5c3_Err != nil { @@ -365,7 +366,7 @@ catalog = load_catalog( namespaces = catalog.list_namespaces()`) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 235, Col: 39} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_buckets.templ`, Line: 236, Col: 39} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var20)) if templ_7745c5c3_Err != nil { diff --git a/weed/admin/view/app/s3tables_tables.templ b/weed/admin/view/app/s3tables_tables.templ index b858676ff..e27f2320a 100644 --- a/weed/admin/view/app/s3tables_tables.templ +++ b/weed/admin/view/app/s3tables_tables.templ @@ -24,7 +24,7 @@ templ S3TablesTables(data dash.S3TablesTablesData) {
{{ bucketName, parseErr := s3tables.ParseBucketNameFromARN(data.BucketARN) }} if parseErr == nil { - + Back to Namespaces } else { @@ -126,7 +126,7 @@ templ S3TablesTables(data dash.S3TablesTablesData) {
if parseErr == nil { - + } else { diff --git a/weed/admin/view/app/s3tables_tables_templ.go b/weed/admin/view/app/s3tables_tables_templ.go index 585dbac2e..cdd52e003 100644 --- a/weed/admin/view/app/s3tables_tables_templ.go +++ b/weed/admin/view/app/s3tables_tables_templ.go @@ -48,9 +48,9 @@ func S3TablesTables(data dash.S3TablesTablesData) templ.Component { return templ_7745c5c3_Err } var templ_7745c5c3_Var2 templ.SafeURL - templ_7745c5c3_Var2, templ_7745c5c3_Err = templ.JoinURLErrs(templ.SafeURL(fmt.Sprintf("/object-store/s3tables/buckets/%s/namespaces", bucketName))) + templ_7745c5c3_Var2, templ_7745c5c3_Err = templ.JoinURLErrs(dash.PUrl(ctx, fmt.Sprintf("/object-store/s3tables/buckets/%s/namespaces", url.PathEscape(bucketName)))) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_tables.templ`, Line: 27, Col: 99} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_tables.templ`, Line: 27, Col: 116} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var2)) if templ_7745c5c3_Err != nil { @@ -308,9 +308,9 @@ func S3TablesTables(data dash.S3TablesTablesData) templ.Component { return templ_7745c5c3_Err } var templ_7745c5c3_Var17 templ.SafeURL - templ_7745c5c3_Var17, templ_7745c5c3_Err = templ.JoinURLErrs(templ.SafeURL(fmt.Sprintf("/object-store/s3tables/buckets/%s/namespaces/%s/tables/%s", url.PathEscape(bucketName), url.PathEscape(data.Namespace), url.PathEscape(tableName)))) + templ_7745c5c3_Var17, templ_7745c5c3_Err = templ.JoinURLErrs(dash.PUrl(ctx, fmt.Sprintf("/object-store/s3tables/buckets/%s/namespaces/%s/tables/%s", url.PathEscape(bucketName), url.PathEscape(data.Namespace), url.PathEscape(tableName)))) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_tables.templ`, Line: 129, Col: 237} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `view/app/s3tables_tables.templ`, Line: 129, Col: 238} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ.EscapeString(templ_7745c5c3_Var17)) if templ_7745c5c3_Err != nil { From ab4e52ae2ff268759bc28982f5f8e91d8a7158d3 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 2 Apr 2026 11:55:13 -0700 Subject: [PATCH 3/3] fix(s3): use recursive delete for .versions directory cleanup (#8887) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(s3): use recursive delete for .versions directory cleanup When only delete markers remain in a .versions directory, updateLatestVersionAfterDeletion tried to delete it non-recursively, which failed with "fail to delete non-empty folder" because the delete marker entries were still present. Use recursive deletion so the directory and its remaining delete marker entries are cleaned up together. * fix(s3): guard .versions directory deletion against truncated listings When the version listing is truncated (>1000 entries), content versions may exist beyond the first page. Skip the recursive directory deletion in this case to prevent data loss. * fix(s3): preserve delete markers in .versions directory Delete markers must be preserved per S3 semantics — they are only removed by an explicit DELETE with versionId. The previous fix would recursively delete the entire .versions directory (including delete markers) when no content versions were found. Now the logic distinguishes three cases: 1. Content versions exist → update latest version metadata 2. Only delete markers remain (or listing truncated) → keep directory 3. Truly empty → safe to delete directory (non-recursive) --- weed/s3api/s3api_object_versioning.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index 4ffb3970f..07f3cc250 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -999,7 +999,7 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) glog.V(1).Infof("updateLatestVersionAfterDeletion: updating latest version for %s/%s, listing %s", bucket, object, versionsDir) // List all remaining version files in the .versions directory - entries, _, err := s3a.list(versionsDir, "", "", false, 1000) + entries, isLast, err := s3a.list(versionsDir, "", "", false, 1000) if err != nil { glog.Errorf("updateLatestVersionAfterDeletion: failed to list versions in %s: %v", versionsDir, err) return fmt.Errorf("failed to list versions: %v", err) @@ -1011,6 +1011,7 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) var latestVersionId string var latestVersionFileName string var latestVersionEntry *filer_pb.Entry + hasDeleteMarkers := false for _, entry := range entries { if entry.Extended == nil { @@ -1027,6 +1028,7 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) // Skip delete markers when finding latest content version isDeleteMarkerBytes, _ := entry.Extended[s3_constants.ExtDeleteMarkerKey] if string(isDeleteMarkerBytes) == "true" { + hasDeleteMarkers = true continue } @@ -1070,14 +1072,18 @@ func (s3a *S3ApiServer) updateLatestVersionAfterDeletion(bucket, object string) if err != nil { return fmt.Errorf("failed to update .versions directory metadata: %v", err) } + } else if hasDeleteMarkers || !isLast { + // Delete markers still exist in the .versions directory, or the listing was + // truncated so there may be more entries. Either way, keep the directory. + glog.V(2).Infof("updateLatestVersionAfterDeletion: no content versions found for %s/%s but .versions directory still has entries (deleteMarkers=%v, isLast=%v), keeping directory", + bucket, object, hasDeleteMarkers, isLast) } else { - // No versions left - delete the .versions metadata file entirely - // This prevents clients from seeing an empty .versions file - glog.V(2).Infof("updateLatestVersionAfterDeletion: no versions left for %s/%s, deleting .versions metadata file", bucket, object) + // No entries at all - delete the .versions directory entirely + glog.V(2).Infof("updateLatestVersionAfterDeletion: no versions left for %s/%s, deleting .versions directory", bucket, object) err = s3a.rm(bucketDir, versionsObjectPath, true, false) if err != nil { - glog.Warningf("updateLatestVersionAfterDeletion: failed to delete .versions metadata file for %s/%s: %v", bucket, object, err) + glog.Warningf("updateLatestVersionAfterDeletion: failed to delete .versions directory for %s/%s: %v", bucket, object, err) // Don't return error - the versions are already deleted, this is just cleanup } }