From c29827863d1e8169774689c15176dfa1f887b3df Mon Sep 17 00:00:00 2001 From: Ludovico Russo Date: Wed, 21 Apr 2021 22:06:55 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20add=20golang=20ci=20lin?= =?UTF-8?q?t=20and=20fix=20lint=20errors=20(#56)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add golang ci lint * fix golang-ci output * update golang ci * add golangci config file * fix linting issue --- .github/workflows/golang.yaml | 18 +++++++++ .golangci.yaml | 51 +++++++++++++++++++++++++ cmd/api/{admin_api => adminapi}/api.go | 12 +++--- cmd/api/{mail_api => mailapi}/mailer.go | 17 ++++----- cmd/api/main.go | 26 ++++++------- cmd/dispatcher/dispatcher.go | 10 +++-- cmd/sender/sender.go | 12 +++--- generated/sqlc/db_test.go | 4 +- internal/dkim/signer.go | 3 +- internal/mailbuilder/mailbuilder.go | 26 ++++++++++++- internal/mailbuilder/message.go | 46 +++++++--------------- internal/mailbuilder/message_test.go | 16 +++++++- internal/pool/pool.go | 1 - internal/smtp/sender.go | 11 +++++- internal/templates/templates_impl.go | 4 +- main.go | 9 ++--- 16 files changed, 179 insertions(+), 87 deletions(-) create mode 100644 .github/workflows/golang.yaml create mode 100644 .golangci.yaml rename cmd/api/{admin_api => adminapi}/api.go (81%) rename cmd/api/{mail_api => mailapi}/mailer.go (91%) diff --git a/.github/workflows/golang.yaml b/.github/workflows/golang.yaml new file mode 100644 index 0000000..fe22cfd --- /dev/null +++ b/.github/workflows/golang.yaml @@ -0,0 +1,18 @@ +name: golangci-lint +on: + push: + tags: + - v* + branches: + - master + pull_request: +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: golangci-lint + uses: golangci/golangci-lint-action@v2 + with: + version: v1.29 diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..54338f7 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,51 @@ +linters-settings: + golint: + min-confidence: 0 + goimports: + local-prefixes: github.com/clastix/capsule + dupl: + threshold: 100 + goconst: + min-len: 2 + min-occurrences: 2 +linters: + disable-all: true + enable: + - bodyclose + - deadcode + - depguard + - dogsled + - dupl + - errcheck + - goconst + - gocritic + - gofmt + - goimports + - golint + - goprintffuncname + - gosimple + - govet + - ineffassign + - misspell + - nolintlint + - rowserrcheck + - staticcheck + - structcheck + - stylecheck + - typecheck + - unconvert + - unparam + - unused + - varcheck + - whitespace + +issues: + exclude: + - Using the variable on range scope .* in function literal + +service: + golangci-lint-version: 1.33.x + +run: + skip-files: + - "zz_.*\\.go$" diff --git a/cmd/api/admin_api/api.go b/cmd/api/adminapi/api.go similarity index 81% rename from cmd/api/admin_api/api.go rename to cmd/api/adminapi/api.go index fdecf4e..accccfb 100644 --- a/cmd/api/admin_api/api.go +++ b/cmd/api/adminapi/api.go @@ -1,4 +1,4 @@ -package admin_api +package adminapi import ( "context" @@ -11,11 +11,11 @@ import ( "kannon.gyozatech.dev/internal/domains" ) -type adminApiService struct { +type adminAPIService struct { dm domains.DomainManager } -func (s *adminApiService) GetDomains(ctx context.Context, in *emptypb.Empty) (*pb.GetDomainsResponse, error) { +func (s *adminAPIService) GetDomains(ctx context.Context, in *emptypb.Empty) (*pb.GetDomainsResponse, error) { domains, err := s.dm.GetAllDomains() if err != nil { return nil, err @@ -28,7 +28,7 @@ func (s *adminApiService) GetDomains(ctx context.Context, in *emptypb.Empty) (*p return &res, nil } -func (s *adminApiService) CreateDomain(ctx context.Context, in *pb.CreateDomainRequest) (*pb.Domain, error) { +func (s *adminAPIService) CreateDomain(ctx context.Context, in *pb.CreateDomainRequest) (*pb.Domain, error) { domain, err := s.dm.CreateDomain(in.Domain) if err != nil { return nil, err @@ -37,7 +37,7 @@ func (s *adminApiService) CreateDomain(ctx context.Context, in *pb.CreateDomainR return dbDomainToProtoDomain(domain), nil } -func (s *adminApiService) RegenerateDomainKey(ctx context.Context, in *pb.RegenerateDomainKeyRequest) (*pb.Domain, error) { +func (s *adminAPIService) RegenerateDomainKey(ctx context.Context, in *pb.RegenerateDomainKeyRequest) (*pb.Domain, error) { return nil, nil } @@ -47,7 +47,7 @@ func CreateAdminAPIService(db *sql.DB) (pb.ApiServer, error) { if err != nil { return nil, err } - api := adminApiService{ + api := adminAPIService{ dm: dm, } diff --git a/cmd/api/mail_api/mailer.go b/cmd/api/mailapi/mailer.go similarity index 91% rename from cmd/api/mail_api/mailer.go rename to cmd/api/mailapi/mailer.go index 2c7dd73..c089879 100644 --- a/cmd/api/mail_api/mailer.go +++ b/cmd/api/mailapi/mailer.go @@ -1,4 +1,4 @@ -package mail_api +package mailapi import ( "context" @@ -20,13 +20,13 @@ import ( "kannon.gyozatech.dev/internal/templates" ) -type mailApiService struct { +type mailAPIService struct { domains domains.DomainManager templates templates.Manager sendingPoll pool.SendingPoolManager } -func (s mailApiService) SendHTML(ctx context.Context, in *pb.SendHTMLRequest) (*pb.SendResponse, error) { +func (s mailAPIService) SendHTML(ctx context.Context, in *pb.SendHTMLRequest) (*pb.SendResponse, error) { domain, ok := s.getCallDomainFromContext(ctx) if !ok { logrus.Errorf("invalid login\n") @@ -59,7 +59,7 @@ func (s mailApiService) SendHTML(ctx context.Context, in *pb.SendHTMLRequest) (* return &response, nil } -func (s mailApiService) SendTemplate(ctx context.Context, in *pb.SendTemplateRequest) (*pb.SendResponse, error) { +func (s mailAPIService) SendTemplate(ctx context.Context, in *pb.SendTemplateRequest) (*pb.SendResponse, error) { domain, ok := s.getCallDomainFromContext(ctx) if !ok { logrus.Errorf("invalid login\n") @@ -92,11 +92,11 @@ func (s mailApiService) SendTemplate(ctx context.Context, in *pb.SendTemplateReq return &response, nil } -func (s mailApiService) Close() error { +func (s mailAPIService) Close() error { return s.domains.Close() } -func (s mailApiService) getCallDomainFromContext(ctx context.Context) (sqlc.Domain, bool) { +func (s mailAPIService) getCallDomainFromContext(ctx context.Context) (sqlc.Domain, bool) { m, ok := metadata.FromIncomingContext(ctx) if !ok { logrus.Debugf("Cannot find metatada\n") @@ -138,10 +138,9 @@ func (s mailApiService) getCallDomainFromContext(ctx context.Context) (sqlc.Doma } return domain, true - } -func NewMailApiService(dbi *sql.DB) (pb.MailerServer, error) { +func NewMailAPIService(dbi *sql.DB) (pb.MailerServer, error) { domainsCli, err := domains.NewDomainManager(dbi) if err != nil { return nil, err @@ -157,7 +156,7 @@ func NewMailApiService(dbi *sql.DB) (pb.MailerServer, error) { return nil, err } - return &mailApiService{ + return &mailAPIService{ domains: domainsCli, sendingPoll: sendingPoolCli, templates: templates, diff --git a/cmd/api/main.go b/cmd/api/main.go index 166ec5a..df8d904 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -8,21 +8,22 @@ import ( "sync" "github.com/joho/godotenv" - "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus" "google.golang.org/grpc" - "kannon.gyozatech.dev/cmd/api/admin_api" - "kannon.gyozatech.dev/cmd/api/mail_api" + "kannon.gyozatech.dev/cmd/api/adminapi" + "kannon.gyozatech.dev/cmd/api/mailapi" "kannon.gyozatech.dev/generated/pb" ) func main() { log.SetFormatter(&log.JSONFormatter{}) - runGrpcServer() + if err := runGrpcServer(); err != nil { + panic(err.Error()) + } } func runGrpcServer() error { - godotenv.Load() + _ = godotenv.Load() dbi, err := sql.Open("postgres", os.Getenv("DB_CONN")) if err != nil { @@ -30,29 +31,28 @@ func runGrpcServer() error { } defer dbi.Close() - adminApiService, err := admin_api.CreateAdminAPIService(dbi) + adminAPIService, err := adminapi.CreateAdminAPIService(dbi) if err != nil { - logrus.Fatalf("Cannot create Admin API service: %v\n", err) - return err + return fmt.Errorf("cannot create Admin API service: %w", err) } - mailApiService, err := mail_api.NewMailApiService(dbi) + mailAPIService, err := mailapi.NewMailAPIService(dbi) if err != nil { - logrus.Fatalf("Cannot create Mailer API service: %v\n", err) + return fmt.Errorf("cannot create Mailer API service: %w", err) } wg := sync.WaitGroup{} wg.Add(2) go func() { - err := startApiServer(50051, adminApiService) + err := startAPIServer(50051, adminAPIService) if err != nil { panic("Cannot run api server") } }() go func() { - err := startMailerServer(50052, mailApiService) + err := startMailerServer(50052, mailAPIService) if err != nil { panic("Cannot run mailer server") } @@ -63,7 +63,7 @@ func runGrpcServer() error { return nil } -func startApiServer(port uint16, srv pb.ApiServer) error { +func startAPIServer(port uint16, srv pb.ApiServer) error { addr := fmt.Sprintf("0.0.0.0:%d", port) lis, err := net.Listen("tcp", addr) if err != nil { diff --git a/cmd/dispatcher/dispatcher.go b/cmd/dispatcher/dispatcher.go index e24a824..5ddf3f6 100644 --- a/cmd/dispatcher/dispatcher.go +++ b/cmd/dispatcher/dispatcher.go @@ -26,7 +26,7 @@ type appConfig struct { } func main() { - godotenv.Load() + _ = godotenv.Load() var config appConfig err := envconfig.Process("app", &config) @@ -121,7 +121,9 @@ func handleErrors(mgr *jsm.Manager) { } else { logrus.Printf("[🛑 bump] %v %v - %v", errMsg.Email, errMsg.MessageId, errMsg.Msg) } - msg.Ack() + if err := msg.Ack(); err != nil { + logrus.Errorf("Cannot hack msg to nats: %v\n", err) + } } } @@ -142,6 +144,8 @@ func handleDelivereds(mgr *jsm.Manager) { } else { logrus.Printf("[🚀 delivered] %v %v", deliveredMsg.Email, deliveredMsg.MessageId) } - msg.Ack() + if err := msg.Ack(); err != nil { + logrus.Errorf("Cannot hack msg to nats: %v\n", err) + } } } diff --git a/cmd/sender/sender.go b/cmd/sender/sender.go index 72fd938..b24f536 100644 --- a/cmd/sender/sender.go +++ b/cmd/sender/sender.go @@ -4,25 +4,23 @@ import ( "context" "flag" - "github.com/golang/protobuf/proto" - "github.com/joho/godotenv" "github.com/nats-io/jsm.go" "github.com/nats-io/nats.go" "github.com/sirupsen/logrus" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" "kannon.gyozatech.dev/generated/pb" "kannon.gyozatech.dev/internal/smtp" ) func main() { - godotenv.Load() senderHost := flag.String("sender-host", "sender.kannon.io", "Sender hostname for SMTP presentation") - natsUrl := flag.String("nasts-url", "nats", "Nats url connection") + natsURL := flag.String("nasts-url", "nats", "Nats url connection") maxSendingJobs := flag.Uint("max-sending-jobs", 100, "Max Parallel Job for sending") flag.Parse() - nc, err := nats.Connect(*natsUrl, nats.UseOldRequestStyle()) + nc, err := nats.Connect(*natsURL, nats.UseOldRequestStyle()) if err != nil { logrus.Fatalf("Cannot connect to nats: %v\n", err) } @@ -55,7 +53,9 @@ func handleSend(sender smtp.Sender, con *jsm.Consumer, nc *nats.Conn, maxParalle if err != nil { logrus.Errorf("error in handling message: %v\n", err.Error()) } - msg.Ack() + if err := msg.Ack(); err != nil { + logrus.Errorf("cannot hack message: %v\n", err.Error()) + } <-ch }() } diff --git a/generated/sqlc/db_test.go b/generated/sqlc/db_test.go index 1f9f2b7..4970b54 100644 --- a/generated/sqlc/db_test.go +++ b/generated/sqlc/db_test.go @@ -57,7 +57,7 @@ func TestMain(m *testing.M) { if err = pool.Retry(func() error { var err error - db, err = initDb(resource.GetPort("5432/tcp")) + db, err = initDB(resource.GetPort("5432/tcp")) if err != nil { logrus.Warnf("connection error: %v", err) return err @@ -144,7 +144,7 @@ func TestTemplates(t *testing.T) { assert.Nil(t, err) } -func initDb(dbPort string) (*sql.DB, error) { +func initDB(dbPort string) (*sql.DB, error) { url := fmt.Sprintf("postgresql://test:test@localhost:%v/test", dbPort) db, err := conn(url) if err != nil { diff --git a/internal/dkim/signer.go b/internal/dkim/signer.go index a39f6d5..4bad02d 100644 --- a/internal/dkim/signer.go +++ b/internal/dkim/signer.go @@ -42,6 +42,5 @@ func decodeKey(dkimPrivateKey string) (*rsa.PrivateKey, error) { if err != nil { return nil, err } - rsak, err := x509.ParsePKCS1PrivateKey(dkimPrivateKeyInBytes) - return rsak, nil + return x509.ParsePKCS1PrivateKey(dkimPrivateKeyInBytes) } diff --git a/internal/mailbuilder/mailbuilder.go b/internal/mailbuilder/mailbuilder.go index cee81cf..ce68155 100644 --- a/internal/mailbuilder/mailbuilder.go +++ b/internal/mailbuilder/mailbuilder.go @@ -4,7 +4,10 @@ import ( "bytes" "context" "database/sql" + "time" + "github.com/sirupsen/logrus" + "gopkg.in/mail.v2" "kannon.gyozatech.dev/generated/pb" "kannon.gyozatech.dev/generated/sqlc" "kannon.gyozatech.dev/internal/dkim" @@ -60,8 +63,8 @@ func (m *mailBuilder) PerpareForSend(email sqlc.SendingPoolEmail) (pb.EmailToSen func prepareMessage(sender pool.Sender, subject string, to string, messageID string, html string, baseHeaders headers) ([]byte, error) { emailMessageID := buildEmailMessageID(to, messageID) - headers := buildHeaders(subject, sender, to, messageID, emailMessageID, baseHeaders) - return renderMsg(html, sender.Email, to, headers) + h := buildHeaders(subject, sender, to, messageID, emailMessageID, baseHeaders) + return renderMsg(html, h) } func signMessage(domain string, dkimPrivateKey string, msg []byte) ([]byte, error) { @@ -74,3 +77,22 @@ func signMessage(domain string, dkimPrivateKey string, msg []byte) ([]byte, erro return dkim.SignMessage(signData, bytes.NewReader(msg)) } + +// renderMsg render a MsgPayload to an SMTP message +func renderMsg(html string, headers headers) ([]byte, error) { + msg := mail.NewMessage() + + for key, value := range headers { + msg.SetHeader(key, value) + } + msg.SetDateHeader("Date", time.Now()) + msg.SetBody("text/html", html) + + var buff bytes.Buffer + if _, err := msg.WriteTo(&buff); err != nil { + logrus.Warnf("🤢 Error writing message: %v\n", err) + return nil, err + } + + return buff.Bytes(), nil +} diff --git a/internal/mailbuilder/message.go b/internal/mailbuilder/message.go index 151b4d2..4d54f0b 100644 --- a/internal/mailbuilder/message.go +++ b/internal/mailbuilder/message.go @@ -1,52 +1,32 @@ package mailbuilder import ( - "bytes" "encoding/base64" "fmt" - "time" - "github.com/sirupsen/logrus" - "gopkg.in/mail.v2" "kannon.gyozatech.dev/internal/pool" ) -func buildEmailMessageID(to string, messageId string) string { +func buildEmailMessageID(to string, messageID string) string { emailBase64 := base64.URLEncoding.EncodeToString([]byte(to)) - return fmt.Sprintf("<%v/%v>", emailBase64, messageId) + return fmt.Sprintf("<%v/%v>", emailBase64, messageID) } -func buildReturnPath(to string, messageId string) string { +func buildReturnPath(to string, messageID string) string { emailBase64 := base64.URLEncoding.EncodeToString([]byte(to)) - return fmt.Sprintf("bump_%v-%v", emailBase64, messageId) + return fmt.Sprintf("bump_%v-%v", emailBase64, messageID) } // buildHeaders for a message func buildHeaders(subject string, sender pool.Sender, to string, poolMessageID string, messageID string, baseHeaders headers) headers { - headers := headers(baseHeaders) - headers["Subject"] = subject - headers["From"] = fmt.Sprintf("%v <%v>", sender.Alias, sender.Email) - headers["To"] = to - headers["Message-ID"] = messageID - headers["X-Pool-Message-ID"] = poolMessageID - return headers -} - -// renderMsg render a MsgPayload to an SMTP message -func renderMsg(html string, from, to string, headers headers) ([]byte, error) { - msg := mail.NewMessage() - - for key, value := range headers { - msg.SetHeader(key, value) + h := make(headers) + for k, v := range baseHeaders { + h[k] = v } - msg.SetDateHeader("Date", time.Now()) - msg.SetBody("text/html", html) - - var buff bytes.Buffer - if _, err := msg.WriteTo(&buff); err != nil { - logrus.Warnf("🤢 Error writing message: %v\n", err) - return nil, err - } - - return buff.Bytes(), nil + h["Subject"] = subject + h["From"] = fmt.Sprintf("%v <%v>", sender.Alias, sender.Email) + h["To"] = to + h["Message-ID"] = messageID + h["X-Pool-Message-ID"] = poolMessageID + return h } diff --git a/internal/mailbuilder/message_test.go b/internal/mailbuilder/message_test.go index e1dfda1..b03a82b 100644 --- a/internal/mailbuilder/message_test.go +++ b/internal/mailbuilder/message_test.go @@ -8,7 +8,6 @@ import ( ) func TestBuildHeaders(t *testing.T) { - sender := pool.Sender{ Email: "from@email.com", Alias: "email", @@ -34,5 +33,20 @@ func TestBuildHeaders(t *testing.T) { if h["To"] != "to@email.com" { t.Errorf("From headers not correct: %v != %v", h["To"], "to@email.com") } +} + +func TestBuildHeadersShouldCopyBaseHeader(t *testing.T) { + baseHeaders := headers{ + "testH": "testH", + } + sender := pool.Sender{ + Email: "from@email.com", + Alias: "email", + } + + buildHeaders("test subject", sender, "to@email.com", "132@email.com", "", baseHeaders) + if len(baseHeaders) != 1 { + t.Errorf("base headers has changed") + } } diff --git a/internal/pool/pool.go b/internal/pool/pool.go index a217f52..5c17c0f 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -39,7 +39,6 @@ func (m *sendingPoolManager) AddPool( subject string, domain string, ) (sqlc.Message, error) { - msg, err := m.db.CreateMessage(context.Background(), sqlc.CreateMessageParams{ TemplateID: template.TemplateID, Domain: domain, diff --git a/internal/smtp/sender.go b/internal/smtp/sender.go index 0c30ec9..1dd4647 100644 --- a/internal/smtp/sender.go +++ b/internal/smtp/sender.go @@ -101,7 +101,11 @@ func deliver(from, to string, msg []byte, mx string, insecure bool, domain strin return newSMTPError(err, false, 111) } defer conn.Close() - conn.SetDeadline(time.Now().Add(smtpTotalTimeout)) + if err := conn.SetDeadline(time.Now().Add(smtpTotalTimeout)); err != nil { + log.Debugf("Cannot set deadline: %v", err) + // TODO: add error code + return newSMTPError(err, false, 111) + } c, err := smtp.NewClient(conn, mx) if err != nil { @@ -162,7 +166,10 @@ func deliver(from, to string, msg []byte, mx string, insecure bool, domain strin return newSMTPErrorFromSTMP(err) } - c.Quit() + if err := c.Quit(); err != nil { + log.Debugf("err: %v\n", err) + return newSMTPErrorFromSTMP(err) + } return nil } diff --git a/internal/templates/templates_impl.go b/internal/templates/templates_impl.go index c3692a2..f927d03 100644 --- a/internal/templates/templates_impl.go +++ b/internal/templates/templates_impl.go @@ -23,10 +23,10 @@ func (m *manager) FindTemplate(domain string, templateID string) (sqlc.Template, return template, nil } -func (m *manager) CreateTemplate(HTML string, domain string) (sqlc.Template, error) { +func (m *manager) CreateTemplate(html string, domain string) (sqlc.Template, error) { template, err := m.db.CreateTemplate(context.TODO(), sqlc.CreateTemplateParams{ TemplateID: fmt.Sprintf("template_%v@%v", cuid.New(), domain), - Html: HTML, + Html: html, Domain: domain, }) if err != nil { diff --git a/main.go b/main.go index dd575e7..14d91b6 100644 --- a/main.go +++ b/main.go @@ -11,13 +11,13 @@ import ( ) func main() { - ps_db, err := sql.Open("postgres", "host=localhost user=postgres dbname=test_cannon port=5432 sslmode=disable") + db, err := sql.Open("postgres", "host=localhost user=postgres dbname=test_cannon port=5432 sslmode=disable") if err != nil { panic(err) } - dbi := sqlc.New(ps_db) - msg, err := dbi.CreateMessage(context.Background(), sqlc.CreateMessageParams{ + queries := sqlc.New(db) + msg, err := queries.CreateMessage(context.Background(), sqlc.CreateMessageParams{ TemplateID: "pippo", Domain: "ciao@ciao.com", Subject: "ciao", @@ -29,7 +29,7 @@ func main() { fmt.Printf("%v", err) } - data, _ := dbi.CreatePool(context.Background(), sqlc.CreatePoolParams{ + data, _ := queries.CreatePool(context.Background(), sqlc.CreatePoolParams{ ScheduledTime: time.Now(), MessageID: msg.ID, Emails: []string{"email@1.com", "asdasd@email.com"}, @@ -38,5 +38,4 @@ func main() { fmt.Printf("%v", err) } fmt.Printf("geenrated %v", data) - }