From 667693163b38e4b303494656b90007125004cd75 Mon Sep 17 00:00:00 2001 From: Jordan Nimlos Date: Wed, 4 Oct 2023 14:39:00 -0500 Subject: [PATCH 1/5] allow passing Logger instance in config --- smtpsrv/config.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/smtpsrv/config.go b/smtpsrv/config.go index d9a7f74998..de425c37d9 100644 --- a/smtpsrv/config.go +++ b/smtpsrv/config.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "github.com/target/goalert/alert" + "github.com/target/goalert/util/log" ) // Config is used to configure the SMTP server. @@ -15,6 +16,7 @@ type Config struct { MaxRecipients int BackgroundContext func() context.Context + Logger *log.Logger AuthorizeFunc func(ctx context.Context, id string) (context.Context, error) CreateAlertFunc func(ctx context.Context, a *alert.Alert) error From ffa345e575440a38aac64c3a5625809c09cc2471 Mon Sep 17 00:00:00 2001 From: Jordan Nimlos Date: Wed, 4 Oct 2023 14:39:45 -0500 Subject: [PATCH 2/5] pass main app logger to smtp config --- app/initsmtpsrv.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/initsmtpsrv.go b/app/initsmtpsrv.go index dc92a57f86..b8f02fb741 100644 --- a/app/initsmtpsrv.go +++ b/app/initsmtpsrv.go @@ -24,6 +24,7 @@ func (app *App) initSMTPServer(ctx context.Context) error { TLSConfig: app.cfg.TLSConfigSMTP, MaxRecipients: app.cfg.SMTPMaxRecipients, BackgroundContext: app.LogBackgroundContext, + Logger: app.cfg.Logger, AuthorizeFunc: func(ctx context.Context, id string) (context.Context, error) { tok, _, err := authtoken.Parse(id, nil) if err != nil { From 0dbcf2efa4d5f5482d94452147738bf220e47385 Mon Sep 17 00:00:00 2001 From: Jordan Nimlos Date: Wed, 4 Oct 2023 15:00:19 -0500 Subject: [PATCH 3/5] implement smtp.Logger in smtpsrv to make logs go through app.cfg.Logger https://github.com/target/goalert/issues/3335 --- smtpsrv/server.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/smtpsrv/server.go b/smtpsrv/server.go index b5267e633b..c001ecd94c 100644 --- a/smtpsrv/server.go +++ b/smtpsrv/server.go @@ -2,12 +2,28 @@ package smtpsrv import ( "context" + "errors" + "fmt" "net" "time" "github.com/emersion/go-smtp" + "github.com/target/goalert/util/log" ) +// SMTPLogger implements the smtp.Logger interface using the main app Logger +type SMTPLogger struct { + logger *log.Logger +} + +func (l *SMTPLogger) Printf(format string, v ...interface{}) { + l.logger.Error(context.Background(), errors.New(fmt.Sprintf(format, v...))) +} + +func (l *SMTPLogger) Println(v ...interface{}) { + l.logger.Error(context.Background(), errors.New(fmt.Sprint(v...))) +} + // Server implements an SMTP server that creates alerts. type Server struct { cfg Config @@ -19,6 +35,7 @@ func NewServer(cfg Config) *Server { s := &Server{cfg: cfg} srv := smtp.NewServer(s) + srv.ErrorLog = &SMTPLogger{logger: cfg.Logger} srv.Domain = cfg.Domain srv.ReadTimeout = 10 * time.Second srv.WriteTimeout = 10 * time.Second From 727294e8cc4b120523609ed4531c5bc68ba4b5bc Mon Sep 17 00:00:00 2001 From: Jordan Nimlos Date: Wed, 4 Oct 2023 16:24:02 -0500 Subject: [PATCH 4/5] filter out TCP ECONNRESET error messages --- smtpsrv/server.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/smtpsrv/server.go b/smtpsrv/server.go index c001ecd94c..51a179907c 100644 --- a/smtpsrv/server.go +++ b/smtpsrv/server.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net" + "strings" "time" "github.com/emersion/go-smtp" @@ -16,12 +17,20 @@ type SMTPLogger struct { logger *log.Logger } +// Printf adheres to smtp.Server's Logger interface while filtering out ECONNRESET errors caused by TCP health checks func (l *SMTPLogger) Printf(format string, v ...interface{}) { - l.logger.Error(context.Background(), errors.New(fmt.Sprintf(format, v...))) + s := fmt.Sprintf(format, v...) + if !strings.Contains(s, "read: connection reset by peer") { + l.logger.Error(context.Background(), errors.New(s)) + } } +// Print adheres to smtp.Server's Logger interface while filtering out ECONNRESET errors caused by TCP health checks func (l *SMTPLogger) Println(v ...interface{}) { - l.logger.Error(context.Background(), errors.New(fmt.Sprint(v...))) + s := fmt.Sprint(v...) + if !strings.Contains(s, "read: connection reset by peer") { + l.logger.Error(context.Background(), errors.New(s)) + } } // Server implements an SMTP server that creates alerts. From 264aa4b00660a8f2fb4fa9eb82178f498f0e5994 Mon Sep 17 00:00:00 2001 From: Jordan Nimlos Date: Thu, 5 Oct 2023 09:17:58 -0500 Subject: [PATCH 5/5] early return instead of negation; add TODO --- smtpsrv/server.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/smtpsrv/server.go b/smtpsrv/server.go index 51a179907c..411bfa66e6 100644 --- a/smtpsrv/server.go +++ b/smtpsrv/server.go @@ -12,25 +12,31 @@ import ( "github.com/target/goalert/util/log" ) -// SMTPLogger implements the smtp.Logger interface using the main app Logger +// SMTPLogger implements the smtp.Logger interface using the main app Logger. type SMTPLogger struct { logger *log.Logger } -// Printf adheres to smtp.Server's Logger interface while filtering out ECONNRESET errors caused by TCP health checks +// Printf adheres to smtp.Server's Logger interface. func (l *SMTPLogger) Printf(format string, v ...interface{}) { s := fmt.Sprintf(format, v...) - if !strings.Contains(s, "read: connection reset by peer") { - l.logger.Error(context.Background(), errors.New(s)) + // TODO: Uses string compare to filter out errors caused by TCP health checks, + // remove once https://github.com/emersion/go-smtp/issues/236 has been fixed. + if strings.Contains(s, "read: connection reset by peer") { + return } + l.logger.Error(context.Background(), errors.New(s)) } -// Print adheres to smtp.Server's Logger interface while filtering out ECONNRESET errors caused by TCP health checks +// Print adheres to smtp.Server's Logger interface. func (l *SMTPLogger) Println(v ...interface{}) { s := fmt.Sprint(v...) - if !strings.Contains(s, "read: connection reset by peer") { - l.logger.Error(context.Background(), errors.New(s)) + // TODO: Uses string compare to filter out errors caused by TCP health checks, + // remove once https://github.com/emersion/go-smtp/issues/236 has been fixed. + if strings.Contains(s, "read: connection reset by peer") { + return } + l.logger.Error(context.Background(), errors.New(s)) } // Server implements an SMTP server that creates alerts.