From e2d589f3cc48700cbbc3207ccef1a919374dc491 Mon Sep 17 00:00:00 2001 From: Egon Elbre Date: Wed, 14 Oct 2020 14:11:36 +0300 Subject: [PATCH] certificate/rpcerrs: move logging sanitizer into certificate Currently that is the only place using it and it's tied to zap implementation. We don't want to have zap in common to reduce common dependencies. Change-Id: I72c064008f83ad3a8a3aa21944753208d4844c85 --- certificate/endpoint.go | 23 +++--- certificate/rpcerrs/log.go | 52 +++++++++++++ certificate/rpcerrs/log_test.go | 132 ++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 12 deletions(-) create mode 100644 certificate/rpcerrs/log.go create mode 100644 certificate/rpcerrs/log_test.go diff --git a/certificate/endpoint.go b/certificate/endpoint.go index 03880773d..9fe113b00 100644 --- a/certificate/endpoint.go +++ b/certificate/endpoint.go @@ -8,17 +8,17 @@ import ( "go.uber.org/zap" - "storj.io/common/errs2" "storj.io/common/identity" "storj.io/common/pb" "storj.io/common/rpc/rpcpeer" "storj.io/common/rpc/rpcstatus" "storj.io/storj/certificate/authorization" + "storj.io/storj/certificate/rpcerrs" ) // Endpoint implements pb.CertificatesServer. type Endpoint struct { - sanitizer *errs2.LoggingSanitizer + rpclog *rpcerrs.Log log *zap.Logger ca *identity.FullCertificateAuthority authorizationDB *authorization.DB @@ -27,16 +27,15 @@ type Endpoint struct { // NewEndpoint creates a new certificate signing server. func NewEndpoint(log *zap.Logger, ca *identity.FullCertificateAuthority, authorizationDB *authorization.DB, minDifficulty uint16) *Endpoint { - codeMap := errs2.CodeMap{ + rpclog := rpcerrs.NewLog(&Error, log, rpcerrs.StatusMap{ &authorization.ErrNotFound: rpcstatus.Unauthenticated, &authorization.ErrInvalidClaim: rpcstatus.InvalidArgument, &authorization.ErrInvalidToken: rpcstatus.InvalidArgument, &authorization.ErrAlreadyClaimed: rpcstatus.AlreadyExists, - } - sanitizer := errs2.NewLoggingSanitizer(&Error, log, codeMap) + }) return &Endpoint{ - sanitizer: sanitizer, + rpclog: rpclog, log: log, ca: ca, authorizationDB: authorizationDB, @@ -51,19 +50,19 @@ func (endpoint Endpoint) Sign(ctx context.Context, req *pb.SigningRequest) (_ *p peer, err := rpcpeer.FromContext(ctx) if err != nil { msg := "error getting peer from context" - return nil, endpoint.sanitizer.Error(msg, err) + return nil, endpoint.rpclog.Error(msg, err) } peerIdent, err := identity.PeerIdentityFromPeer(peer) if err != nil { msg := "error getting peer identity" - return nil, endpoint.sanitizer.Error(msg, err) + return nil, endpoint.rpclog.Error(msg, err) } signedPeerCA, err := endpoint.ca.Sign(peerIdent.CA) if err != nil { msg := "error signing peer CA" - return nil, endpoint.sanitizer.Error(msg, err) + return nil, endpoint.rpclog.Error(msg, err) } signedChainBytes := [][]byte{signedPeerCA.Raw, endpoint.ca.Cert.Raw} @@ -76,18 +75,18 @@ func (endpoint Endpoint) Sign(ctx context.Context, req *pb.SigningRequest) (_ *p }) if err != nil { msg := "error claiming authorization" - return nil, endpoint.sanitizer.Error(msg, err) + return nil, endpoint.rpclog.Error(msg, err) } difficulty, err := peerIdent.ID.Difficulty() if err != nil { msg := "error checking difficulty" - return nil, endpoint.sanitizer.Error(msg, err) + return nil, endpoint.rpclog.Error(msg, err) } token, err := authorization.ParseToken(req.AuthToken) if err != nil { msg := "error parsing auth token" - return nil, endpoint.sanitizer.Error(msg, err) + return nil, endpoint.rpclog.Error(msg, err) } tokenFormatter := authorization.Authorization{ Token: *token, diff --git a/certificate/rpcerrs/log.go b/certificate/rpcerrs/log.go new file mode 100644 index 000000000..9d981d720 --- /dev/null +++ b/certificate/rpcerrs/log.go @@ -0,0 +1,52 @@ +// Copyright (C) 2020 Storj Labs, Inc. +// See LICENSE for copying information. + +package rpcerrs + +import ( + "github.com/zeebo/errs" + "go.uber.org/zap" + + "storj.io/common/rpc/rpcstatus" +) + +// StatusMap is used to apply the correct rpc status code to error classes. +type StatusMap map[*errs.Class]rpcstatus.StatusCode + +// Log consolidates logging of original errors with sanitization of internal errors. +type Log struct { + wrapper *errs.Class + log *zap.Logger + codeMap StatusMap +} + +// NewLog creates a new Log. +func NewLog(wrapper *errs.Class, log *zap.Logger, codeMap StatusMap) *Log { + return &Log{ + wrapper: wrapper, + log: log, + codeMap: codeMap, + } +} + +// Error logs the message and error to the logger and returns the mapped rpcstatus code. +func (sanitizer *Log) Error(msg string, err error) error { + if sanitizer.wrapper != nil { + err = sanitizer.wrapper.Wrap(err) + } + + if sanitizer.log != nil { + sanitizer.log.Error(msg, zap.Error(err)) + } + + for errClass, code := range sanitizer.codeMap { + if errClass.Has(err) { + return rpcstatus.Error(code, err.Error()) + } + } + + if sanitizer.wrapper == nil { + return rpcstatus.Error(rpcstatus.Internal, msg) + } + return rpcstatus.Error(rpcstatus.Internal, sanitizer.wrapper.New(msg).Error()) +} diff --git a/certificate/rpcerrs/log_test.go b/certificate/rpcerrs/log_test.go new file mode 100644 index 000000000..aa9fce110 --- /dev/null +++ b/certificate/rpcerrs/log_test.go @@ -0,0 +1,132 @@ +// Copyright (C) 2019 Storj Labs, Inc. +// See LICENSE for copying information. + +package rpcerrs_test + +import ( + "fmt" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "github.com/zeebo/errs" + "go.uber.org/zap" + + "storj.io/common/rpc/rpcstatus" + "storj.io/common/testcontext" + "storj.io/storj/certificate/rpcerrs" +) + +func TestLoggingSanitizer_Error(t *testing.T) { + ctx := testcontext.New(t) + defer ctx.Cleanup() + + logPath := ctx.File("log") + logFile, err := os.Create(logPath) + require.NoError(t, err) + defer ctx.Check(logFile.Close) + + wrapper := errs.Class("wrapper class") + unauthenticatedClass := errs.Class("unauthorized class") + notFoundClass := errs.Class("not found class") + internalClass := errs.Class("internal class") + internalErr := internalClass.New("internal error") + msg := "message" + statusMap := rpcerrs.StatusMap{ + &unauthenticatedClass: rpcstatus.Unauthenticated, + ¬FoundClass: rpcstatus.NotFound, + } + + testLogConfig := zap.NewDevelopmentConfig() + testLogConfig.OutputPaths = []string{logPath} + testLog, err := testLogConfig.Build() + require.NoError(t, err) + + scenarios := []struct { + name string + wrapper *errs.Class + log *zap.Logger + }{ + { + "with wrapper and log", + &wrapper, + testLog, + }, + { + "with wrapper, no log", + &wrapper, + nil, + }, + { + "with log, no wrapper", + nil, + testLog, + }, + { + "no wrapper or log", + nil, + nil, + }, + } + + for _, s := range scenarios { + s := s + t.Run(s.name, func(t *testing.T) { + sanitizer := rpcerrs.NewLog(s.wrapper, s.log, statusMap) + + t.Log("exposed errors") + for errClass, code := range statusMap { + errInstance := errClass.New("%s", strings.Replace(string(*errClass), "class", "error", 1)) + + sanitizedErr := sanitizer.Error(msg, errInstance) + require.Error(t, sanitizedErr) + require.Equal(t, code, rpcstatus.Code(sanitizedErr)) + require.Contains(t, sanitizedErr.Error(), *errClass) + if s.wrapper == nil { + require.Contains(t, sanitizedErr.Error(), errInstance.Error()) + } else { + require.Contains(t, sanitizedErr.Error(), wrapper.Wrap(errInstance).Error()) + } + + if s.log != nil { + logData, err := ioutil.ReadAll(logFile) + require.NoError(t, err) + + logStr := string(logData) + require.Contains(t, logStr, msg) + if s.wrapper == nil { + require.Contains(t, logStr, fmt.Sprintf(`"error": "%s"`, errInstance)) + } else { + require.Contains(t, logStr, fmt.Sprintf(`"error": "%s: %s"`, wrapper, errInstance)) + } + } + } + + t.Log("internal error") + sanitizedErr := sanitizer.Error(msg, internalErr) + require.Error(t, sanitizedErr) + require.Equal(t, rpcstatus.Internal, rpcstatus.Code(sanitizedErr)) + require.NotContains(t, sanitizedErr.Error(), internalClass) + if s.wrapper == nil { + require.Contains(t, sanitizedErr.Error(), msg) + } else { + require.Equal(t, wrapper.New(msg).Error(), sanitizedErr.Error()) + } + + if s.log != nil { + logData, err := ioutil.ReadAll(logFile) + require.NoError(t, err) + + logStr := string(logData) + require.Contains(t, logStr, msg) + if s.wrapper == nil { + require.Contains(t, logStr, fmt.Sprintf(`"error": "%s"`, internalErr)) + } else { + require.Contains(t, logStr, fmt.Sprintf(`"error": "%s: %s"`, wrapper, internalErr)) + } + } + }) + } +}