From 486b8a4284698b472526c75cbb8dd4306cb98ccd Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Mon, 26 Feb 2024 17:01:13 +0300 Subject: [PATCH] [#321] Use correct owner id in billing metrics Signed-off-by: Marina Biryukova --- api/middleware/auth.go | 7 +++++++ api/middleware/metrics.go | 12 +----------- api/middleware/reqinfo.go | 1 + api/router_mock_test.go | 22 +++++++++++++++++++++- api/router_test.go | 34 +++++++++++++++++++++++++++------- 5 files changed, 57 insertions(+), 19 deletions(-) diff --git a/api/middleware/auth.go b/api/middleware/auth.go index 8e079c1a..f676788d 100644 --- a/api/middleware/auth.go +++ b/api/middleware/auth.go @@ -6,6 +6,7 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" + "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" "go.uber.org/zap" ) @@ -13,6 +14,8 @@ func Auth(center auth.Center, log *zap.Logger) Func { return func(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() + reqInfo := GetReqInfo(ctx) + reqInfo.User = "anon" box, err := center.Authenticate(r) if err != nil { if err == auth.ErrNoAuthorizationHeader { @@ -31,6 +34,10 @@ func Auth(center auth.Center, log *zap.Logger) Func { ctx = SetClientTime(ctx, box.ClientTime) } ctx = SetAuthHeaders(ctx, box.AuthHeaders) + + if box.AccessBox.Gate.BearerToken != nil { + reqInfo.User = bearer.ResolveIssuer(*box.AccessBox.Gate.BearerToken).String() + } } h.ServeHTTP(w, r.WithContext(ctx)) diff --git a/api/middleware/metrics.go b/api/middleware/metrics.go index 2d0a4b9d..0ce7e570 100644 --- a/api/middleware/metrics.go +++ b/api/middleware/metrics.go @@ -12,7 +12,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/metrics" - "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer" "go.uber.org/zap" ) @@ -80,9 +79,8 @@ func stats(f http.HandlerFunc, resolveCID cidResolveFunc, appMetrics *metrics.Ap // simply for the fact that it is not human-readable. durationSecs := time.Since(statsWriter.startTime).Seconds() - user := resolveUser(r.Context()) cnrID := resolveCID(r.Context(), reqInfo) - appMetrics.Update(user, reqInfo.BucketName, cnrID, requestTypeFromAPI(reqInfo.API), in.countBytes, out.countBytes) + appMetrics.Update(reqInfo.User, reqInfo.BucketName, cnrID, requestTypeFromAPI(reqInfo.API), in.countBytes, out.countBytes) code := statsWriter.statusCode // A successful request has a 2xx response code @@ -148,14 +146,6 @@ func resolveCID(log *zap.Logger, resolveBucket BucketResolveFunc) cidResolveFunc } } -func resolveUser(ctx context.Context) string { - user := "anon" - if bd, err := GetBoxData(ctx); err == nil && bd.Gate.BearerToken != nil { - user = bearer.ResolveIssuer(*bd.Gate.BearerToken).String() - } - return user -} - // WriteHeader -- writes http status code. func (w *responseWrapper) WriteHeader(code int) { w.Do(func() { diff --git a/api/middleware/reqinfo.go b/api/middleware/reqinfo.go index 0a96ca82..c889c24b 100644 --- a/api/middleware/reqinfo.go +++ b/api/middleware/reqinfo.go @@ -36,6 +36,7 @@ type ( BucketName string // Bucket name ObjectName string // Object name TraceID string // Trace ID + User string // User owner id URL *url.URL // Request url tags []KeyVal // Any additional info not accommodated by above fields } diff --git a/api/router_mock_test.go b/api/router_mock_test.go index 0ba5386c..097d4fd3 100644 --- a/api/router_mock_test.go +++ b/api/router_mock_test.go @@ -9,14 +9,34 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/middleware" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/accessbox" + bearertest "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer/test" "github.com/stretchr/testify/require" ) +type anonCenterMock struct { +} + +func (c *anonCenterMock) Authenticate(*http.Request) (*auth.Box, error) { + return &auth.Box{ + AccessBox: &accessbox.Box{ + Gate: &accessbox.GateData{}, + }, + }, nil +} + type centerMock struct { } func (c *centerMock) Authenticate(*http.Request) (*auth.Box, error) { - return &auth.Box{}, nil + token := bearertest.Token() + return &auth.Box{ + AccessBox: &accessbox.Box{ + Gate: &accessbox.GateData{ + BearerToken: &token, + }, + }, + }, nil } type handlerMock struct { diff --git a/api/router_test.go b/api/router_test.go index 153eec51..97918adc 100644 --- a/api/router_test.go +++ b/api/router_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/auth" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/metrics" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" @@ -18,7 +19,7 @@ import ( ) func TestRouterUploadPart(t *testing.T) { - chiRouter := prepareRouter(t) + chiRouter := prepareRouter(t, &anonCenterMock{}) w := httptest.NewRecorder() r := httptest.NewRequest(http.MethodPut, "/dkirillov/fix-object", nil) @@ -33,7 +34,7 @@ func TestRouterUploadPart(t *testing.T) { } func TestRouterListMultipartUploads(t *testing.T) { - chiRouter := prepareRouter(t) + chiRouter := prepareRouter(t, &anonCenterMock{}) w := httptest.NewRecorder() r := httptest.NewRequest(http.MethodGet, "/test-bucket", nil) @@ -47,7 +48,7 @@ func TestRouterListMultipartUploads(t *testing.T) { } func TestRouterObjectWithSlashes(t *testing.T) { - chiRouter := prepareRouter(t) + chiRouter := prepareRouter(t, &anonCenterMock{}) bktName, objName := "dkirillov", "/fix/object" target := fmt.Sprintf("/%s/%s", bktName, objName) @@ -62,7 +63,7 @@ func TestRouterObjectWithSlashes(t *testing.T) { } func TestRouterObjectEscaping(t *testing.T) { - chiRouter := prepareRouter(t) + chiRouter := prepareRouter(t, &anonCenterMock{}) bktName := "dkirillov" @@ -106,19 +107,38 @@ func TestRouterObjectEscaping(t *testing.T) { } } -func prepareRouter(t *testing.T) *chi.Mux { +func TestOwnerIDRetrieving(t *testing.T) { + anonRouter := prepareRouter(t, &anonCenterMock{}) + + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/test-bucket", nil) + + anonRouter.ServeHTTP(w, r) + resp := readResponse(t, w) + require.Equal(t, "anon", resp.ReqInfo.User) + + chiRouter := prepareRouter(t, ¢erMock{}) + + w = httptest.NewRecorder() + r = httptest.NewRequest(http.MethodGet, "/test-bucket", nil) + + chiRouter.ServeHTTP(w, r) + resp = readResponse(t, w) + require.NotEqual(t, "anon", resp.ReqInfo.User) +} + +func prepareRouter(t *testing.T, center auth.Center) *chi.Mux { throttleOps := middleware.ThrottleOpts{ Limit: 10, BacklogTimeout: 30 * time.Second, } handleMock := &handlerMock{t: t} - cntrMock := ¢erMock{} log := zaptest.NewLogger(t) metric := &metrics.AppMetrics{} chiRouter := chi.NewRouter() - AttachChi(chiRouter, nil, throttleOps, handleMock, cntrMock, log, metric) + AttachChi(chiRouter, nil, throttleOps, handleMock, center, log, metric) return chiRouter }