From 921535411bc28191d5e62a040127038eb40c6778 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 16 Oct 2026 15:45:36 +0900 Subject: [PATCH] s3: default allowDeleteBucketNotEmpty to false (#11769) * s3: default allowDeleteBucketNotEmpty to false DeleteBucket on a non-empty bucket silently destroyed every object, while AWS returns 409 BucketNotEmpty. Clients like 'aws s3 rb' rely on that refusal as the 'delete only if empty' safety net. Deployments that need the recursive delete can opt back in with -allowDeleteBucketNotEmpty=true. * test: pin allowDeleteBucketNotEmpty=true on versioning config1 The config1 run relied on the flag's old true default to contrast with config2's false. Pin it so the two modes stay distinct. * test: drop redundant allowDeleteBucketNotEmpty=false in s3 integration test False is the default now, so TestS3DeleteBucketNotEmpty exercises the out-of-box behavior. --- test/s3/normal/s3_integration_test.go | 4 ++-- test/s3/versioning/Makefile | 4 ++-- weed/command/filer.go | 2 +- weed/command/mini.go | 2 +- weed/command/s3.go | 2 +- weed/command/server.go | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/s3/normal/s3_integration_test.go b/test/s3/normal/s3_integration_test.go index b64ba4db7..e9a9ed690 100644 --- a/test/s3/normal/s3_integration_test.go +++ b/test/s3/normal/s3_integration_test.go @@ -114,7 +114,7 @@ func TestS3Integration(t *testing.T) { // findAvailablePort finds an available port by binding to port 0 // startMiniCluster starts a weed mini instance directly without exec. -// Extra flags (e.g. "-s3.allowDeleteBucketNotEmpty=false") can be appended via extraArgs. +// Extra flags can be appended via extraArgs. func startMiniCluster(t *testing.T, extraArgs ...string) (*TestCluster, error) { // Allocate non-colliding ports (including gRPC offsets) for weed mini. ports := testutil.MustFreeMiniPorts(t, []string{"Master", "Volume", "Filer", "S3"}) @@ -872,7 +872,7 @@ func TestS3DeleteBucketNotEmpty(t *testing.T) { t.Skip("Skipping integration test in short mode") } - cluster, err := startMiniCluster(t, "-s3.allowDeleteBucketNotEmpty=false") + cluster, err := startMiniCluster(t) require.NoError(t, err) defer cluster.Stop() diff --git a/test/s3/versioning/Makefile b/test/s3/versioning/Makefile index 5fde72044..621543bbc 100644 --- a/test/s3/versioning/Makefile +++ b/test/s3/versioning/Makefile @@ -228,8 +228,8 @@ test-with-server: start-server # Test with different configurations test-versioning-with-configs: check-deps @echo "Testing with different S3 configurations..." - @echo "Testing with empty folder allowed..." - @$(WEED_BINARY) mini -s3.port=$(S3_PORT) > weed-test-config1.log 2>&1 & echo $$! > weed-config1.pid + @echo "Testing with delete bucket not empty allowed..." + @$(WEED_BINARY) mini -s3.port=$(S3_PORT) -s3.allowDeleteBucketNotEmpty=true > weed-test-config1.log 2>&1 & echo $$! > weed-config1.pid @sleep 5 @go test -v -timeout=5m -run "TestVersioningBasicWorkflow" . || true @if [ -f weed-config1.pid ]; then kill -TERM $$(cat weed-config1.pid) 2>/dev/null || true; rm -f weed-config1.pid; fi diff --git a/weed/command/filer.go b/weed/command/filer.go index 03d2f11be..98e243a09 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -154,7 +154,7 @@ func init() { filerS3Options.metricsHttpPort = cmdFiler.Flag.Int("s3.metricsPort", 0, "Prometheus metrics listen port") filerS3Options.metricsHttpIp = cmdFiler.Flag.String("s3.metricsIp", "", "metrics listen ip. If empty, default to same as -s3.ip.bind option.") cmdFiler.Flag.Bool("s3.allowEmptyFolder", true, "deprecated, ignored. Empty folder cleanup is now automatic.") - filerS3Options.allowDeleteBucketNotEmpty = cmdFiler.Flag.Bool("s3.allowDeleteBucketNotEmpty", true, "allow recursive deleting all entries along with bucket") + filerS3Options.allowDeleteBucketNotEmpty = cmdFiler.Flag.Bool("s3.allowDeleteBucketNotEmpty", false, "allow recursive deleting all entries along with bucket") filerS3Options.autoCreateBucket = cmdFiler.Flag.Bool("s3.autoCreateBucket", true, "create the bucket on upload if it does not exist, for admin identities only") filerS3Options.localSocket = cmdFiler.Flag.String("s3.localSocket", "", "default to /tmp/seaweedfs-s3-.sock") filerS3Options.tlsCACertificate = cmdFiler.Flag.String("s3.cacert.file", "", "path to the TLS CA certificate file") diff --git a/weed/command/mini.go b/weed/command/mini.go index e4a97f4a1..a44c55bb9 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -407,7 +407,7 @@ var ( miniMetricsHttpIp = cmdMini.Flag.String("metricsIp", "", "metrics listen ip. If empty, default to same as -ip.bind option.") miniS3Config = cmdMini.Flag.String("s3.config", "", "path to the S3 config file") miniIamConfig = cmdMini.Flag.String("s3.iam.config", "", "path to the advanced IAM config file for S3") - miniS3AllowDeleteBucketNotEmpty = cmdMini.Flag.Bool("s3.allowDeleteBucketNotEmpty", true, "allow recursive deleting all entries along with bucket") + miniS3AllowDeleteBucketNotEmpty = cmdMini.Flag.Bool("s3.allowDeleteBucketNotEmpty", false, "allow recursive deleting all entries along with bucket") miniS3AutoCreateBucket = cmdMini.Flag.Bool("s3.autoCreateBucket", true, "create the bucket on upload if it does not exist, for admin identities only") miniBucket = cmdMini.Flag.String("bucket", "", "comma-separated S3 bucket names to create on startup if they do not already exist; leave empty to skip. Falls back to S3_BUCKET env var.") miniTableBucket = cmdMini.Flag.String("tableBucket", "", "comma-separated S3 Tables buckets to create on startup if they do not already exist, each name[:FORMAT] with FORMAT one of ICEBERG (default) or LANCE, e.g. warehouse,vectors:LANCE; leave empty to skip. Falls back to S3_TABLE_BUCKET env var.") diff --git a/weed/command/s3.go b/weed/command/s3.go index 8df38908b..7400266db 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -114,7 +114,7 @@ func init() { s3StandaloneOptions.metricsHttpPort = cmdS3.Flag.Int("metricsPort", 0, "Prometheus metrics listen port") s3StandaloneOptions.metricsHttpIp = cmdS3.Flag.String("metricsIp", "", "metrics listen ip. If empty, default to same as -ip.bind option.") cmdS3.Flag.Bool("allowEmptyFolder", true, "deprecated, ignored. Empty folder cleanup is now automatic.") - s3StandaloneOptions.allowDeleteBucketNotEmpty = cmdS3.Flag.Bool("allowDeleteBucketNotEmpty", true, "allow recursive deleting all entries along with bucket") + s3StandaloneOptions.allowDeleteBucketNotEmpty = cmdS3.Flag.Bool("allowDeleteBucketNotEmpty", false, "allow recursive deleting all entries along with bucket") s3StandaloneOptions.autoCreateBucket = cmdS3.Flag.Bool("autoCreateBucket", true, "create the bucket on upload if it does not exist, for admin identities only") s3StandaloneOptions.localFilerSocket = cmdS3.Flag.String("localFilerSocket", "", "local filer socket path") s3StandaloneOptions.localSocket = cmdS3.Flag.String("localSocket", "", "default to /tmp/seaweedfs-s3-.sock") diff --git a/weed/command/server.go b/weed/command/server.go index da1385a08..22a3ae4f4 100644 --- a/weed/command/server.go +++ b/weed/command/server.go @@ -182,7 +182,7 @@ func init() { s3Options.iamConfig = cmdServer.Flag.String("s3.iam.config", "", "path to the advanced IAM config file for S3. Overrides -iam.config if both are provided.") s3Options.auditLogConfig = cmdServer.Flag.String("s3.auditLogConfig", "", "path to the audit log config file") cmdServer.Flag.Bool("s3.allowEmptyFolder", true, "deprecated, ignored. Empty folder cleanup is now automatic.") - s3Options.allowDeleteBucketNotEmpty = cmdServer.Flag.Bool("s3.allowDeleteBucketNotEmpty", true, "allow recursive deleting all entries along with bucket") + s3Options.allowDeleteBucketNotEmpty = cmdServer.Flag.Bool("s3.allowDeleteBucketNotEmpty", false, "allow recursive deleting all entries along with bucket") s3Options.autoCreateBucket = cmdServer.Flag.Bool("s3.autoCreateBucket", true, "create the bucket on upload if it does not exist, for admin identities only") s3Options.localSocket = cmdServer.Flag.String("s3.localSocket", "", "default to /tmp/seaweedfs-s3-.sock") s3Options.bindIp = cmdServer.Flag.String("s3.ip.bind", "", "ip address to bind to. If empty, default to same as -ip.bind option.")