From d386476ae466c1c500d7e03d2c7d738cd11f0f3c Mon Sep 17 00:00:00 2001 From: Roman Loginov Date: Mon, 25 Nov 2024 16:28:18 +0300 Subject: [PATCH] [#498] middleware: Add spans to detail the trace Spans are added only to the following middleware: * PolicyCheck * Auth * FrostfsIDValidation This is done this way because these middleware are basic and they interact with frostfs-storage. Signed-off-by: Roman Loginov --- api/auth/center.go | 10 +++++----- api/auth/center_test.go | 4 ++-- api/auth/presign_test.go | 2 +- api/middleware/auth.go | 34 ++++++++++++++++++++++------------ api/middleware/policy.go | 14 +++++++++----- api/router_mock_test.go | 2 +- 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/api/auth/center.go b/api/auth/center.go index e654c25..728c2bc 100644 --- a/api/auth/center.go +++ b/api/auth/center.go @@ -120,7 +120,7 @@ func IsStandardContentSHA256(key string) bool { return ok } -func (c *Center) Authenticate(r *http.Request) (*middleware.Box, error) { +func (c *Center) Authenticate(ctx context.Context, r *http.Request) (*middleware.Box, error) { var ( err error authHdr *AuthHeader @@ -152,7 +152,7 @@ func (c *Center) Authenticate(r *http.Request) (*middleware.Box, error) { authHeaderField := r.Header[AuthorizationHdr] if len(authHeaderField) != 1 { if strings.HasPrefix(r.Header.Get(ContentTypeHdr), "multipart/form-data") { - return c.checkFormData(r) + return c.checkFormData(ctx, r) } return nil, fmt.Errorf("%w: %v", middleware.ErrNoAuthorizationHeader, authHeaderField) } @@ -178,7 +178,7 @@ func (c *Center) Authenticate(r *http.Request) (*middleware.Box, error) { return nil, err } - box, attrs, err := c.cli.GetBox(r.Context(), cnrID, authHdr.AccessKeyID) + box, attrs, err := c.cli.GetBox(ctx, cnrID, authHdr.AccessKeyID) if err != nil { return nil, fmt.Errorf("get box by access key '%s': %w", authHdr.AccessKeyID, err) } @@ -251,7 +251,7 @@ func (c Center) checkAccessKeyID(accessKeyID string) error { return fmt.Errorf("%w: accesskeyID prefix isn't allowed", apierr.GetAPIError(apierr.ErrAccessDenied)) } -func (c *Center) checkFormData(r *http.Request) (*middleware.Box, error) { +func (c *Center) checkFormData(ctx context.Context, r *http.Request) (*middleware.Box, error) { if err := r.ParseMultipartForm(maxFormSizeMemory); err != nil { return nil, fmt.Errorf("%w: parse multipart form with max size %d", apierr.GetAPIError(apierr.ErrInvalidArgument), maxFormSizeMemory) } @@ -283,7 +283,7 @@ func (c *Center) checkFormData(r *http.Request) (*middleware.Box, error) { return nil, err } - box, attrs, err := c.cli.GetBox(r.Context(), cnrID, accessKeyID) + box, attrs, err := c.cli.GetBox(ctx, cnrID, accessKeyID) if err != nil { return nil, fmt.Errorf("get box by accessKeyID '%s': %w", accessKeyID, err) } diff --git a/api/auth/center_test.go b/api/auth/center_test.go index c8c8261..9eb0b78 100644 --- a/api/auth/center_test.go +++ b/api/auth/center_test.go @@ -385,7 +385,7 @@ func TestAuthenticate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { creds := tokens.New(bigConfig) cntr := New(creds, tc.prefixes, ¢erSettingsMock{}) - box, err := cntr.Authenticate(tc.request) + box, err := cntr.Authenticate(context.Background(), tc.request) if tc.err { require.Error(t, err) @@ -563,7 +563,7 @@ func TestHTTPPostAuthenticate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { creds := tokens.New(bigConfig) cntr := New(creds, tc.prefixes, ¢erSettingsMock{}) - box, err := cntr.Authenticate(tc.request) + box, err := cntr.Authenticate(context.Background(), tc.request) if tc.err { require.Error(t, err) diff --git a/api/auth/presign_test.go b/api/auth/presign_test.go index f669b1b..d2248ff 100644 --- a/api/auth/presign_test.go +++ b/api/auth/presign_test.go @@ -89,7 +89,7 @@ func TestCheckSign(t *testing.T) { postReg: NewRegexpMatcher(postPolicyCredentialRegexp), settings: ¢erSettingsMock{}, } - box, err := c.Authenticate(req) + box, err := c.Authenticate(context.Background(), req) require.NoError(t, err) require.EqualValues(t, expBox, box.AccessBox) } diff --git a/api/middleware/auth.go b/api/middleware/auth.go index 202f28c..df40792 100644 --- a/api/middleware/auth.go +++ b/api/middleware/auth.go @@ -1,6 +1,7 @@ package middleware import ( + "context" "crypto/elliptic" "errors" "fmt" @@ -8,6 +9,7 @@ import ( "time" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/acl" + "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" apierr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/creds/accessbox" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" @@ -30,7 +32,7 @@ type ( Center interface { // Authenticate validate and authenticate request. // Must return ErrNoAuthorizationHeader if auth header is missed. - Authenticate(request *http.Request) (*Box, error) + Authenticate(ctx context.Context, request *http.Request) (*Box, error) } //nolint:revive @@ -47,34 +49,38 @@ var ErrNoAuthorizationHeader = errors.New("no authorization header") func Auth(center 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) + reqCtx := r.Context() + ctx, span := tracing.StartSpanFromContext(reqCtx, "middleware.Auth") + + reqInfo := GetReqInfo(reqCtx) reqInfo.User = "anon" - box, err := center.Authenticate(r) + box, err := center.Authenticate(ctx, r) if err != nil { if errors.Is(err, ErrNoAuthorizationHeader) { - reqLogOrDefault(ctx, log).Debug(logs.CouldntReceiveAccessBoxForGateKeyRandomKeyWillBeUsed, zap.Error(err)) + reqLogOrDefault(reqCtx, log).Debug(logs.CouldntReceiveAccessBoxForGateKeyRandomKeyWillBeUsed, zap.Error(err)) } else { - reqLogOrDefault(ctx, log).Error(logs.FailedToPassAuthentication, zap.Error(err)) + reqLogOrDefault(reqCtx, log).Error(logs.FailedToPassAuthentication, zap.Error(err)) err = apierr.TransformToS3Error(err) if err.(apierr.Error).ErrCode == apierr.ErrInternalError { err = apierr.GetAPIError(apierr.ErrAccessDenied) } if _, wrErr := WriteErrorResponse(w, GetReqInfo(r.Context()), err); wrErr != nil { - reqLogOrDefault(ctx, log).Error(logs.FailedToWriteResponse, zap.Error(wrErr)) + reqLogOrDefault(reqCtx, log).Error(logs.FailedToWriteResponse, zap.Error(wrErr)) } + span.End() return } } else { - ctx = SetBox(ctx, box) + reqCtx = SetBox(reqCtx, box) if box.AccessBox.Gate.BearerToken != nil { reqInfo.User = bearer.ResolveIssuer(*box.AccessBox.Gate.BearerToken).String() } - reqLogOrDefault(ctx, log).Debug(logs.SuccessfulAuth, zap.String("accessKeyID", box.AuthHeaders.AccessKeyID)) + reqLogOrDefault(reqCtx, log).Debug(logs.SuccessfulAuth, zap.String("accessKeyID", box.AuthHeaders.AccessKeyID)) } - h.ServeHTTP(w, r.WithContext(ctx)) + span.End() + h.ServeHTTP(w, r.WithContext(reqCtx)) }) } } @@ -86,22 +92,26 @@ type FrostFSIDValidator interface { func FrostfsIDValidation(frostfsID FrostFSIDValidator, log *zap.Logger) Func { return func(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() + ctx, span := tracing.StartSpanFromContext(r.Context(), "middleware.FrostfsIDValidation") + bd, err := GetBoxData(ctx) if err != nil || bd.Gate.BearerToken == nil { reqLogOrDefault(ctx, log).Debug(logs.AnonRequestSkipFrostfsIDValidation) + span.End() h.ServeHTTP(w, r) return } if err = validateBearerToken(frostfsID, bd.Gate.BearerToken); err != nil { reqLogOrDefault(ctx, log).Error(logs.FrostfsIDValidationFailed, zap.Error(err)) - if _, wrErr := WriteErrorResponse(w, GetReqInfo(r.Context()), err); wrErr != nil { + if _, wrErr := WriteErrorResponse(w, GetReqInfo(ctx), err); wrErr != nil { reqLogOrDefault(ctx, log).Error(logs.FailedToWriteResponse, zap.Error(wrErr)) } + span.End() return } + span.End() h.ServeHTTP(w, r) }) } diff --git a/api/middleware/policy.go b/api/middleware/policy.go index 9e36f0b..a1d6a8e 100644 --- a/api/middleware/policy.go +++ b/api/middleware/policy.go @@ -10,6 +10,7 @@ import ( "net/url" "strings" + "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/data" apierr "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/api/errors" "git.frostfs.info/TrueCloudLab/frostfs-s3-gw/internal/logs" @@ -85,23 +86,26 @@ type PolicyConfig struct { func PolicyCheck(cfg PolicyConfig) Func { return func(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - if err := policyCheck(r, cfg); err != nil { + ctx, span := tracing.StartSpanFromContext(r.Context(), "middleware.PolicyCheck") + + if err := policyCheck(ctx, r, cfg); err != nil { reqLogOrDefault(ctx, cfg.Log).Error(logs.PolicyValidationFailed, zap.Error(err)) err = apierr.TransformToS3Error(err) if _, wrErr := WriteErrorResponse(w, GetReqInfo(ctx), err); wrErr != nil { reqLogOrDefault(ctx, cfg.Log).Error(logs.FailedToWriteResponse, zap.Error(wrErr)) } + span.End() return } + span.End() h.ServeHTTP(w, r) }) } } -func policyCheck(r *http.Request, cfg PolicyConfig) error { - reqInfo := GetReqInfo(r.Context()) +func policyCheck(ctx context.Context, r *http.Request, cfg PolicyConfig) error { + reqInfo := GetReqInfo(ctx) req, userKey, userGroups, err := getPolicyRequest(r, cfg, reqInfo.RequestType, reqInfo.BucketName, reqInfo.ObjectName) if err != nil { @@ -110,7 +114,7 @@ func policyCheck(r *http.Request, cfg PolicyConfig) error { var bktInfo *data.BucketInfo if reqInfo.RequestType != noneType && !strings.HasSuffix(req.Operation(), CreateBucketOperation) { - bktInfo, err = cfg.BucketResolver(r.Context(), reqInfo.BucketName) + bktInfo, err = cfg.BucketResolver(ctx, reqInfo.BucketName) if err != nil { return err } diff --git a/api/router_mock_test.go b/api/router_mock_test.go index 4f237f9..61f73fc 100644 --- a/api/router_mock_test.go +++ b/api/router_mock_test.go @@ -45,7 +45,7 @@ type centerMock struct { key *keys.PrivateKey } -func (c *centerMock) Authenticate(*http.Request) (*middleware.Box, error) { +func (c *centerMock) Authenticate(context.Context, *http.Request) (*middleware.Box, error) { if c.noAuthHeader { return nil, middleware.ErrNoAuthorizationHeader }