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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Ping QiuandClaude Opus 5.5 authored and GitHub committed 2026-10-29 16:10:36 -07:00
1 parent fd493f2547
commit 3f954e34dc
2 files changed
+36 -2

No files matched your search

+6 -2
View File
@@ -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":
+30
View File
@@ -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)
}
}