diff --git a/cmd/notify-mailer/main.go b/cmd/notify-mailer/main.go index a511e4006..6b8ff52b3 100644 --- a/cmd/notify-mailer/main.go +++ b/cmd/notify-mailer/main.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "encoding/csv" "encoding/json" "errors" @@ -20,7 +19,6 @@ import ( "github.com/jmhodges/clock" "github.com/letsencrypt/boulder/cmd" "github.com/letsencrypt/boulder/db" - "github.com/letsencrypt/boulder/features" blog "github.com/letsencrypt/boulder/log" bmail "github.com/letsencrypt/boulder/mail" "github.com/letsencrypt/boulder/metrics" @@ -35,31 +33,33 @@ type mailer struct { mailer bmail.Mailer subject string emailTemplate *template.Template - destinations []recipient + recipients []recipient targetRange interval sleepInterval time.Duration } -// interval defines a range of email addresses to send to, alphabetically. -// The "start" field is inclusive and the "end" field is exclusive. -// To include everything, set "end" to "\xFF". +// interval defines a range of email addresses to send to in alphabetical order. +// The `start` field is inclusive and the `end` field is exclusive. To include +// everything, set `end` to \xFF. type interval struct { start string end string } -type contactJSON struct { - ID int +// contactQueryResult is a receiver for queries to the `registrations` table. +type contactQueryResult struct { + // ID is exported to receive the value of `id`. + ID int64 + + // Contact is exported to receive the value of `contact`. Contact []byte } func (i *interval) ok() error { if i.start > i.end { - return fmt.Errorf( - "interval start value (%s) is greater than end value (%s)", + return fmt.Errorf("interval start value (%s) is greater than end value (%s)", i.start, i.end) } - return nil } @@ -67,37 +67,35 @@ func (i *interval) includes(s string) bool { return s >= i.start && s < i.end } +// ok ensures that both the `targetRange` and `sleepInterval` are valid. func (m *mailer) ok() error { - // Make sure the checkpoint range is OK - if checkpointErr := m.targetRange.ok(); checkpointErr != nil { - return checkpointErr + if err := m.targetRange.ok(); err != nil { + return err } - // Do not allow a negative sleep interval if m.sleepInterval < 0 { return fmt.Errorf( "sleep interval (%d) is < 0", m.sleepInterval) } - return nil } -func (m *mailer) printStatus(to string, cur, total int, start time.Time) { - // Should never happen - if total <= 0 || cur < 1 || cur > total { - m.log.AuditErrf("invalid cur (%d) or total (%d)", cur, total) +func (m *mailer) logStatus(to string, current, total int, start time.Time) { + // Should never happen. + if total <= 0 || current < 1 || current > total { + m.log.AuditErrf("Invalid current (%d) or total (%d)", current, total) } - completion := (float32(cur) / float32(total)) * 100 + completion := (float32(current) / float32(total)) * 100 now := m.clk.Now() elapsed := now.Sub(start) - m.log.Infof("Sending to %q. Message %d of %d [%.2f%%]. Elapsed: %s", - to, cur, total, completion, elapsed) + m.log.Infof("Sending message (%d) of (%d) to address (%s) [%.2f%%] time elapsed (%s)", + current, total, to, completion, elapsed) } -func sortAddresses(input emailToRecipientMap) []string { +func sortAddresses(input addressToRecipientMap) []string { var addresses []string - for k := range input { - addresses = append(addresses, k) + for address := range input { + addresses = append(addresses, address) } sort.Strings(addresses) return addresses @@ -108,122 +106,128 @@ func (m *mailer) run() error { return err } - m.log.Infof("Resolving %d destination addresses", len(m.destinations)) - addressesToRecipients, err := m.resolveEmailAddresses() + totalRecipients := len(m.recipients) + m.log.Infof("Resolving addresses for (%d) recipients", totalRecipients) + + addressToRecipient, err := m.resolveAddresses() if err != nil { return err } - if len(addressesToRecipients) == 0 { - return fmt.Errorf("zero recipients after looking up addresses?") + + totalAddresses := len(addressToRecipient) + if totalAddresses == 0 { + return errors.New("0 recipients remained after resolving addresses") } - m.log.Infof("Resolved destination addresses. %d accounts became %d addresses.", - len(m.destinations), len(addressesToRecipients)) - var biggest int - var biggestAddress string - for k, v := range addressesToRecipients { - if len(v) > biggest { - biggest = len(v) - biggestAddress = k + + m.log.Infof("%d recipients were resolved to %d addresses", totalRecipients, totalAddresses) + + var mostRecipients string + var mostRecipientsLen int + for k, v := range addressToRecipient { + if len(v) > mostRecipientsLen { + mostRecipientsLen = len(v) + mostRecipients = k } } - m.log.Infof("Most frequent address %q had %d associated lines", biggestAddress, biggest) + + m.log.Infof("Address %q was associated with the most recipients (%d)", + mostRecipients, mostRecipientsLen) err = m.mailer.Connect() if err != nil { return err } - defer func() { - _ = m.mailer.Close() - }() + + defer func() { _ = m.mailer.Close() }() startTime := m.clk.Now() - - sortedAddresses := sortAddresses(addressesToRecipients) - numAddresses := len(addressesToRecipients) + sortedAddresses := sortAddresses(addressToRecipient) var sent int for i, address := range sortedAddresses { if !m.targetRange.includes(address) { - m.log.Debugf("skipping %q: out of target range") + m.log.Debugf("Address %q is outside of target range, skipping", address) continue } + if err := policy.ValidEmail(address); err != nil { - m.log.Infof("skipping %q: %s", address, err) + m.log.Infof("Skipping %q due to policy violation: %s", address, err) continue } - recipients := addressesToRecipients[address] - m.printStatus(address, i+1, numAddresses, startTime) - var mailBody bytes.Buffer - err = m.emailTemplate.Execute(&mailBody, recipients) + + recipients := addressToRecipient[address] + m.logStatus(address, i+1, totalAddresses, startTime) + + var messageBody strings.Builder + err = m.emailTemplate.Execute(&messageBody, recipients) if err != nil { return err } - if mailBody.Len() == 0 { - return fmt.Errorf("email body was empty after interpolation.") + + if messageBody.Len() == 0 { + return errors.New("message body was empty after interpolation") } - err := m.mailer.SendMail([]string{address}, m.subject, mailBody.String()) + + err := m.mailer.SendMail([]string{address}, m.subject, messageBody.String()) if err != nil { var recoverableSMTPErr bmail.RecoverableSMTPError if errors.As(err, &recoverableSMTPErr) { - m.log.Errf("address %q was rejected by server: %s", address, err) + m.log.Errf("Address %q was rejected by the server due to: %s", address, err) continue } - return fmt.Errorf("sending mail %d of %d to %q: %s", + return fmt.Errorf("while sending mail (%d) of (%d) to address %q: %s", i, len(sortedAddresses), address, err) } + sent++ m.clk.Sleep(m.sleepInterval) } + if sent == 0 { - return fmt.Errorf("sent zero messages. Check recipients and configured interval") + return errors.New("0 messages sent, check recipients or configured interval") } return nil } -// resolveEmailAddresses looks up the id of each recipient to find that -// account's email addresses, then adds that recipient to a map from address to -// recipient struct. -func (m *mailer) resolveEmailAddresses() (emailToRecipientMap, error) { - result := make(emailToRecipientMap, len(m.destinations)) - - for _, r := range m.destinations { - // Get the email address for the reg ID - emails, err := emailsForReg(r.id, m.dbMap) +// resolveAddresses creates a mapping of email addresses to (a list of) +// `recipient`s that resolve to that email address. +func (m *mailer) resolveAddresses() (addressToRecipientMap, error) { + result := make(addressToRecipientMap, len(m.recipients)) + for _, recipient := range m.recipients { + addresses, err := getAddressForID(recipient.id, m.dbMap) if err != nil { return nil, err } - for _, email := range emails { - parsedEmail, err := mail.ParseAddress(email) + for _, address := range addresses { + parsed, err := mail.ParseAddress(address) if err != nil { - m.log.Errf("unparsable email for reg ID %d : %q", r.id, email) + m.log.Errf("Unparsable address %q, skipping ID (%d)", address, recipient.id) continue } - addr := parsedEmail.Address - result[addr] = append(result[addr], r) + result[parsed.Address] = append(result[parsed.Address], recipient) } } return result, nil } -// Since the only thing we use from gorp is the SelectOne method on the -// gorp.DbMap object, we just define an interface with that method -// instead of importing all of gorp. This facilitates mock implementations for -// unit tests +// dbSelector abstracts over a subset of methods from `gorp.DbMap` objects to +// facilitate mocking in unit tests. type dbSelector interface { SelectOne(holder interface{}, query string, args ...interface{}) error } -// Finds the email addresses associated with a reg ID -func emailsForReg(id int, dbMap dbSelector) ([]string, error) { - var contact contactJSON - err := dbMap.SelectOne(&contact, - `SELECT id, contact +// getAddressForID queries the database for the email address associated with +// the provided registration ID. +func getAddressForID(id int64, dbMap dbSelector) ([]string, error) { + var result contactQueryResult + err := dbMap.SelectOne(&result, + `SELECT id, + contact FROM registrations - WHERE contact != 'null' AND id = :id;`, - map[string]interface{}{ - "id": id, - }) + WHERE contact != 'null' + AND id = :id;`, + map[string]interface{}{"id": id}) if err != nil { if db.IsNoRows(err) { return []string{}, nil @@ -231,86 +235,98 @@ func emailsForReg(id int, dbMap dbSelector) ([]string, error) { return nil, err } - var contactFields []string - var addresses []string - err = json.Unmarshal(contact.Contact, &contactFields) + var contacts []string + err = json.Unmarshal(result.Contact, &contacts) if err != nil { return nil, err } - for _, entry := range contactFields { - if strings.HasPrefix(entry, "mailto:") { - addresses = append(addresses, strings.TrimPrefix(entry, "mailto:")) + + var addresses []string + for _, contact := range contacts { + if strings.HasPrefix(contact, "mailto:") { + addresses = append(addresses, strings.TrimPrefix(contact, "mailto:")) } } return addresses, nil } -// recipient represents one line in the input CSV, containing an account and -// (optionally) some extra fields related to that account. +// recipient represents a single record from the recipient list file. type recipient struct { - id int - Extra map[string]string + id int64 + + // Data is exported so the contents can be referenced from message template. + Data map[string]string } -// emailToRecipientMap maps from an email address to a list of recipients with -// that email address. -type emailToRecipientMap map[string][]recipient +// addressToRecipientMap maps email addresses to a list of `recipient`s that +// resolve to that email address. +type addressToRecipientMap map[string][]recipient -// readRecipientsList reads a CSV filename and parses that file into a list of -// recipient structs. It puts any columns after the first into a per-recipient -// map from column name -> value. +// readRecipientsList parses the contents of a recipient list file into a list +// of `recipient` objects. func readRecipientsList(filename string) ([]recipient, error) { f, err := os.Open(filename) if err != nil { return nil, err } + reader := csv.NewReader(f) record, err := reader.Read() if err != nil { return nil, err } + if len(record) == 0 { - return nil, fmt.Errorf("no entries in CSV") - } - if record[0] != "id" { - return nil, fmt.Errorf("first field of CSV input must be an ID.") - } - var columnNames []string - for _, v := range record[1:] { - columnNames = append(columnNames, strings.TrimSpace(v)) + return nil, errors.New("no records in CSV") } - results := []recipient{} + if record[0] != "id" { + return nil, errors.New("first field of CSV input must be \"id\"") + } + + var dataColumns []string + for _, v := range record[1:] { + dataColumns = append(dataColumns, strings.TrimSpace(v)) + } + + var recipients []recipient for { record, err := reader.Read() - if err == io.EOF { - if len(results) == 0 { - return nil, fmt.Errorf("no entries after the header in CSV") + if errors.Is(err, io.EOF) { + // Finished parsing the file. + if len(recipients) == 0 { + return nil, fmt.Errorf("no records after the header in CSV") } - return results, nil - } - if err != nil { + return recipients, nil + } else if err != nil { return nil, err } + if len(record) == 0 { return nil, fmt.Errorf("empty line in CSV") } - if len(record) != len(columnNames)+1 { - return nil, fmt.Errorf("Number of columns in CSV line didn't match header columns."+ - " Got %d, expected %d. Line: %v", len(record), len(columnNames)+1, record) + + if len(record) != len(dataColumns)+1 { + return nil, fmt.Errorf("got (%d) columns, for (%d) header columns, for line %q", + len(record), len(dataColumns)+1, record) } - id, err := strconv.Atoi(record[0]) + + // Ensure the ID in the record can be parsed as a valid ID. + recordID := record[0] + id, err := strconv.ParseInt(recordID, 10, 64) if err != nil { - return nil, err - } - recip := recipient{ - id: id, - Extra: make(map[string]string), + return nil, fmt.Errorf( + "%q couldn't be parsed as a registration ID due to: %s", recordID, err) } + + // Create a mapping of column names to extra data columns (anything + // after `id`). + data := make(map[string]string) for i, v := range record[1:] { - recip.Extra[columnNames[i]] = v + data[dataColumns[i]] = v } - results = append(results, recip) + + recipients = append(recipients, recipient{id, data}) } } @@ -404,15 +420,6 @@ func main() { end := flag.String("end", "\xFF", "Alphabetically highest email address (exclusive).") reconnBase := flag.Duration("reconnectBase", 1*time.Second, "Base sleep duration between reconnect attempts") reconnMax := flag.Duration("reconnectMax", 5*60*time.Second, "Max sleep duration between reconnect attempts after exponential backoff") - type config struct { - NotifyMailer struct { - DB cmd.DBConfig - cmd.PasswordConfig - cmd.SMTPConfig - Features map[string]bool - } - Syslog cmd.SyslogConfig - } configFile := flag.String("config", "", "File containing a JSON config.") flag.Usage = func() { @@ -421,6 +428,7 @@ func main() { flag.PrintDefaults() } + // Validate required args. flag.Parse() if *from == "" || *subject == "" || *bodyFile == "" || *configFile == "" || *recipientListFile == "" { @@ -429,48 +437,50 @@ func main() { } configData, err := ioutil.ReadFile(*configFile) - cmd.FailOnError(err, fmt.Sprintf("Reading %q", *configFile)) + cmd.FailOnError(err, "Couldn't load JSON config file") + + type config struct { + NotifyMailer struct { + DB cmd.DBConfig + cmd.SMTPConfig + } + Syslog cmd.SyslogConfig + } + + // Parse JSON config. var cfg config err = json.Unmarshal(configData, &cfg) - cmd.FailOnError(err, "Unmarshaling config") - err = features.Set(cfg.NotifyMailer.Features) - cmd.FailOnError(err, "Failed to set feature flags") + cmd.FailOnError(err, "Couldn't unmarshal JSON config file") log := cmd.NewLogger(cfg.Syslog) defer log.AuditPanic() + // Setup database client. dbURL, err := cfg.NotifyMailer.DB.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbSettings := sa.DbSettings{ - MaxOpenConns: 10, - } - dbMap, err := sa.NewDbMap(dbURL, dbSettings) - cmd.FailOnError(err, "Could not connect to database") - // Load email body - body, err := ioutil.ReadFile(*bodyFile) - cmd.FailOnError(err, fmt.Sprintf("Reading %q", *bodyFile)) - template, err := template.New("email").Parse(string(body)) - cmd.FailOnError(err, fmt.Sprintf("Parsing template %q", *bodyFile)) + dbMap, err := sa.NewDbMap(dbURL, sa.DbSettings{MaxOpenConns: 10}) + cmd.FailOnError(err, "Couldn't create database connection") + + // Load and parse message body. + template, err := template.New("email").ParseFiles(*bodyFile) + cmd.FailOnError(err, "Couldn't parse message template") address, err := mail.ParseAddress(*from) - cmd.FailOnError(err, fmt.Sprintf("Parsing %q", *from)) + cmd.FailOnError(err, fmt.Sprintf("Couldn't parse %q to address", *from)) recipients, err := readRecipientsList(*recipientListFile) - cmd.FailOnError(err, fmt.Sprintf("Reading %q", *recipientListFile)) - - targetRange := interval{ - start: *start, - end: *end, - } + cmd.FailOnError(err, "Couldn't populate recipients") var mailClient bmail.Mailer if *dryRun { - log.Infof("Doing a dry run.") + log.Infof("Starting %s in dry-run mode", cmd.VersionString()) mailClient = bmail.NewDryRun(*address, log) } else { + log.Infof("Starting %s", cmd.VersionString()) smtpPassword, err := cfg.NotifyMailer.PasswordConfig.Pass() - cmd.FailOnError(err, "Failed to load SMTP password") + cmd.FailOnError(err, "Couldn't load SMTP password from file") + mailClient = bmail.New( cfg.NotifyMailer.Server, cfg.NotifyMailer.Port, @@ -490,12 +500,17 @@ func main() { dbMap: dbMap, mailer: mailClient, subject: *subject, - destinations: recipients, + recipients: recipients, emailTemplate: template, - targetRange: targetRange, + targetRange: interval{ + start: *start, + end: *end, + }, sleepInterval: *sleep, } err = m.run() - cmd.FailOnError(err, "mailer.send returned error") + cmd.FailOnError(err, "Couldn't complete") + + log.Info("Completed successfully") } diff --git a/cmd/notify-mailer/main_test.go b/cmd/notify-mailer/main_test.go index 808642f24..c6449c5b6 100644 --- a/cmd/notify-mailer/main_test.go +++ b/cmd/notify-mailer/main_test.go @@ -86,7 +86,7 @@ func TestSleepInterval(t *testing.T) { sleepInterval: sleepLen * time.Second, targetRange: interval{start: "", end: "\xFF"}, clk: newFakeClock(t), - destinations: recipients, + recipients: recipients, dbMap: dbMap, } @@ -107,7 +107,7 @@ func TestSleepInterval(t *testing.T) { sleepInterval: 0, targetRange: interval{end: "\xFF"}, clk: newFakeClock(t), - destinations: recipients, + recipients: recipients, dbMap: dbMap, } @@ -135,7 +135,7 @@ func TestMailIntervals(t *testing.T) { mailer: mc, dbMap: dbMap, subject: testSubject, - destinations: recipients, + recipients: recipients, emailTemplate: tmpl, targetRange: interval{start: "\xFF", end: "\xFF\xFF"}, sleepInterval: 0, @@ -154,7 +154,7 @@ func TestMailIntervals(t *testing.T) { mailer: mc, dbMap: dbMap, subject: testSubject, - destinations: recipients, + recipients: recipients, emailTemplate: tmpl, targetRange: interval{}, sleepInterval: -10, @@ -174,7 +174,7 @@ func TestMailIntervals(t *testing.T) { mailer: mc, dbMap: dbMap, subject: testSubject, - destinations: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}}, + recipients: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}}, emailTemplate: tmpl, targetRange: interval{start: "test-example-updated@letsencrypt.org", end: "\xFF"}, sleepInterval: 0, @@ -206,7 +206,7 @@ func TestMailIntervals(t *testing.T) { mailer: mc, dbMap: dbMap, subject: testSubject, - destinations: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}}, + recipients: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}}, emailTemplate: tmpl, targetRange: interval{end: "test-example-updated@letsencrypt.org"}, sleepInterval: 0, @@ -243,7 +243,7 @@ func TestMessageContentStatic(t *testing.T) { mailer: mc, dbMap: dbMap, subject: testSubject, - destinations: []recipient{{id: 1}}, + recipients: []recipient{{id: 1}}, emailTemplate: template.Must(template.New("letter").Parse("an email body")), targetRange: interval{end: "\xFF"}, sleepInterval: 0, @@ -267,7 +267,7 @@ func TestMessageContentInterpolated(t *testing.T) { recipients := []recipient{ { id: 1, - Extra: map[string]string{ + Data: map[string]string{ "validationMethod": "eyeballing it", }, }, @@ -275,13 +275,13 @@ func TestMessageContentInterpolated(t *testing.T) { dbMap := mockEmailResolver{} mc := &mocks.Mailer{} m := &mailer{ - log: blog.UseMock(), - mailer: mc, - dbMap: dbMap, - subject: "Test Subject", - destinations: recipients, + log: blog.UseMock(), + mailer: mc, + dbMap: dbMap, + subject: "Test Subject", + recipients: recipients, emailTemplate: template.Must(template.New("letter").Parse( - `issued by {{range .}}{{ .Extra.validationMethod }}{{end}}`)), + `issued by {{range .}}{{ .Data.validationMethod }}{{end}}`)), targetRange: interval{end: "\xFF"}, sleepInterval: 0, clk: newFakeClock(t), @@ -305,25 +305,25 @@ func TestMessageContentInterpolatedMultiple(t *testing.T) { recipients := []recipient{ { id: 200, - Extra: map[string]string{ + Data: map[string]string{ "domain": "blog.example.com", }, }, { id: 201, - Extra: map[string]string{ + Data: map[string]string{ "domain": "nas.example.net", }, }, { id: 202, - Extra: map[string]string{ + Data: map[string]string{ "domain": "mail.example.org", }, }, { id: 203, - Extra: map[string]string{ + Data: map[string]string{ "domain": "panel.example.net", }, }, @@ -331,14 +331,14 @@ func TestMessageContentInterpolatedMultiple(t *testing.T) { dbMap := mockEmailResolver{} mc := &mocks.Mailer{} m := &mailer{ - log: blog.UseMock(), - mailer: mc, - dbMap: dbMap, - subject: "Test Subject", - destinations: recipients, + log: blog.UseMock(), + mailer: mc, + dbMap: dbMap, + subject: "Test Subject", + recipients: recipients, emailTemplate: template.Must(template.New("letter").Parse( `issued for: -{{range .}}{{ .Extra.domain }} +{{range .}}{{ .Data.domain }} {{end}}Thanks`)), targetRange: interval{end: "\xFF"}, sleepInterval: 0, @@ -371,7 +371,7 @@ type mockEmailResolver struct{} // into a list of anonymous structs func (bs mockEmailResolver) SelectOne(output interface{}, _ string, args ...interface{}) error { // The "dbList" is just a list of contact records in memory - dbList := []contactJSON{ + dbList := []contactQueryResult{ { ID: 1, Contact: []byte(`["mailto:example@letsencrypt.org"]`), @@ -423,21 +423,21 @@ func (bs mockEmailResolver) SelectOne(output interface{}, _ string, args ...inte } // Play the type cast game so that we can dig into the arguments map and get - // out an integer "id" parameter + // out an int64 `id` parameter. argsRaw := args[0] argsMap, ok := argsRaw.(map[string]interface{}) if !ok { return fmt.Errorf("incorrect args type %T", args) } idRaw := argsMap["id"] - id, ok := idRaw.(int) + id, ok := idRaw.(int64) if !ok { return fmt.Errorf("incorrect args ID type %T", id) } - // Play the type cast game to get a pointer to the output `contactJSON` - // pointer so we can write the result from the db list - outputPtr, ok := output.(*contactJSON) + // Play the type cast game to get a `*contactQueryResult` so we can write + // the result from the db list. + outputPtr, ok := output.(*contactQueryResult) if !ok { return fmt.Errorf("incorrect output type %T", output) } @@ -508,14 +508,14 @@ func TestResolveEmails(t *testing.T) { mailer: mc, dbMap: dbMap, subject: "Test", - destinations: recipients, + recipients: recipients, emailTemplate: tmpl, targetRange: interval{end: "\xFF"}, sleepInterval: 0, clk: newFakeClock(t), } - addressesToRecipients, err := m.resolveEmailAddresses() + addressesToRecipients, err := m.resolveAddresses() test.AssertNotError(t, err, "failed to resolveEmailAddresses") expected := []string{