From 3f954e34dc8b49c5b7aa1acfc5a50d718615240f Mon Sep 17 00:00:00 2001 From: Ping Qiu Date: Thu, 29 Oct 2026 16:10:36 -0700 Subject: [PATCH] s3api: label bucket-level requests correctly in the access log (#12224) * s3api: label bucket-level requests correctly in the access log getResourceType treated a request as bucket-level only when the object key was "/". NormalizeObjectKey strips the leading slash, so bucket routes arrive with an empty key and every bucket operation was logged as an object operation: ListObjectsV2 as REST.GET.OBJECT, DeleteObjects as REST.POST.OBJECT, and the lifecycle, ACL and policy calls likewise. Treat an empty object key as bucket-level too. The new test covers five bucket-level and two object-level request shapes; it fails without the fix. Co-Authored-By: Claude Fable 5.1 * s3api: label only POST ?delete as a batch delete in the access log With bucket-level requests now reaching the bucket branch, any method on /bucket?delete was labelled BATCH.DELETE.OBJECT. Only the POST route is DeleteObjects; label other methods as the plain bucket operation. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Fable 5.1 --- weed/s3api/s3err/audit_fluent.go | 8 +++++-- weed/s3api/s3err/audit_fluent_test.go | 30 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/weed/s3api/s3err/audit_fluent.go b/weed/s3api/s3err/audit_fluent.go index a8d28035be..077018f892 100644 --- a/weed/s3api/s3err/audit_fluent.go +++ b/weed/s3api/s3err/audit_fluent.go @@ -116,10 +116,14 @@ func getREST(httpMetod string, resourceType string) string { } func getResourceType(object string, query_key string, method string) (string, bool) { - if object == "/" { + // Bucket-level routes have no object; NormalizeObjectKey makes that "", not "/". + if object == "" || object == "/" { switch query_key { case "delete": - return "BATCH.DELETE.OBJECT", true + if method == http.MethodPost { + return "BATCH.DELETE.OBJECT", true + } + return getREST(method, "BUCKET"), false case "tagging": return getREST(method, "OBJECTTAGGING"), true case "lifecycle": diff --git a/weed/s3api/s3err/audit_fluent_test.go b/weed/s3api/s3err/audit_fluent_test.go index 6e9792b37d..a40b32846e 100644 --- a/weed/s3api/s3err/audit_fluent_test.go +++ b/weed/s3api/s3err/audit_fluent_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "testing" + "github.com/gorilla/mux" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/util/request_id" "github.com/stretchr/testify/assert" @@ -160,3 +161,32 @@ func TestAuditTrackingFlag(t *testing.T) { MarkAuditLogged(tracked) assert.True(t, AuditAlreadyLogged(tracked), "flag flips after MarkAuditLogged") } + +// Bucket-level requests reach the audit with an empty object key: NormalizeObjectKey strips the +// leading "/", so the old object == "/" test never matched and every bucket operation was labelled +// as an object operation (ListObjectsV2 showed up as REST.GET.OBJECT with an empty key). +func TestGetAccessLogLabelsBucketLevelOperations(t *testing.T) { + cases := []struct { + method, target, object, want string + }{ + {http.MethodGet, "/bucket?list-type=2", "", "REST.GET.BUCKET"}, + {http.MethodGet, "/bucket", "", "REST.GET.BUCKET"}, + {http.MethodPost, "/bucket?delete", "", "BATCH.DELETE.OBJECT"}, + // only the POST batch-delete route is a batch delete; other methods on ?delete are bucket requests + {http.MethodHead, "/bucket?delete", "", "REST.HEAD.BUCKET"}, + {http.MethodGet, "/bucket?delete", "", "REST.GET.BUCKET"}, + {http.MethodGet, "/bucket?lifecycle", "", "REST.GET.LIFECYCLECONFIGURATION"}, + {http.MethodPut, "/bucket?policy", "", "REST.PUT.BUCKETPOLICY"}, + {http.MethodGet, "/bucket/object", "object", "REST.GET.OBJECT"}, + {http.MethodGet, "/bucket/object?tagging", "object", "REST.GET.OBJECTTAGGING"}, + } + for _, c := range cases { + req := httptest.NewRequest(c.method, c.target, nil) + vars := map[string]string{"bucket": "bucket"} + if c.object != "" { + vars["object"] = c.object + } + req = mux.SetURLVars(req, vars) + assert.Equal(t, c.want, GetAccessLog(req, http.StatusOK, ErrNone).Operation, c.method+" "+c.target) + } +}