Use the same pattern as was recently implemented in
expiration-mailer to parallelize notify-mailer. This should
significantly increase throughput when sending emails
to all subscribers.
- Break message body construction out into a testable method.
- Ensure that in the event of a missing key, an informative error is returned instead
of allowing the message to be populated with the zero value of the key.
- Add message body construction tests for success, empty map, and missing key.
- Comment the `recipient` struct and it's `Data` field to make it clear that SRE
must be informed of any modifications.
Fixes#5921
We have decided that we don't like the if err := call(); err != nil
syntax, because it creates confusing scopes, but we have not cleaned up
all existing instances of that syntax. However, we have now found a
case where that syntax enables a bug: It caused readers to believe that
a later err = call() statement was assigning to an already-declared err
in the local scope, when in fact it was assigning to an
already-declared err in the parent scope of a closure. This caused our
ineffassign and staticcheck linters to be unable to analyze the
lifetime of the err variable, and so they did not complain when we
never checked the actual value of that error.
This change standardizes on the two-line error checking syntax
everywhere, so that we can more easily ensure that our linters are
correctly analyzing all error assignments.
The resulting `boulder` binary can be invoked by different names to
trigger the behavior of the relevant subcommand. For instance, symlinking
and invoking as `boulder-ca` acts as the CA. Symlinking and invoking as
`boulder-va` acts as the VA.
This reduces the .deb file size from about 200MB to about 20MB.
This works by creating a registry that maps subcommand names to `main`
functions. Each subcommand registers itself in an `init()` function. The
monolithic `boulder` binary then checks what name it was invoked with
(`os.Args[0]`), looks it up in the registry, and invokes the appropriate
`main`. To avoid conflicts, all of the old `package main` are replaced
with `package notmain`.
To get the list of registered subcommands, run `boulder --list`. This
is used when symlinking all the variants into place, to ensure the set
of symlinked names matches the entries in the registry.
Fixes#5692
- Parse recipient list file as a TSV when flag `-tsv` is provided
- Log recipient list records that contain empty columns
- Log and skip recipient list records that contain the same `id` as previously
read records
- Remove unnecessary check for mismatched header and record column length, this
is already handled by the `encoding/csv` package
- Remove unnecessary check for empty line, this is already handled by the
`encoding/csv` package
Part of #5420
### Improve consistency
- Make registration `id` an `int64`
- Use `address`, `recipient`, and `record` terminology
- Use `errors.New()` in place of `fmt.Errorf()`
- Use `strings.Builder` in place of `bytes.Buffer`
- Use `errors.Is()` when checking for sentinel errors
- Remove unused (duplicate) `cmd.PasswordFile` in `config`
- Remove unused `cmd.Features` in `config`
### Improve readability
- Use godoc standard comments
- Replace multiple calls to `len(someVariable)` with `totalSomeVariable`
Part of #5420
This required a refactoring: Move validateEmail from the RA to ValidEmail
in the `policy` package. I also moved `ValidDomain` from a method on
PolicyAuthority to a standalone function so that ValidEmail can call it.
notify-mailer will now log invalid addresses and skip them without
attempting to send mail. Since @example.com addresses are invalid,
I updated the notify-mailer test, which used a lot of such addresses.
Also, now when notify-mailer receives an unrecoverable error sending
mail, it logs the email address and what offset within the list it was.
New types and related infrastructure are added to the `db` package to allow
wrapping gorp DbMaps and Transactions.
The wrapped versions return a special `db.ErrDatabaseOp` error type when errors
occur. The new error type includes additional information such as the operation
that failed and the related table.
Where possible we determine the table based on the types of the gorp function
arguments. Where that isn't possible (e.g. with raw SQL queries) we try to use
a simple regexp approach to find the table name. This isn't great for general
SQL but works well enough for Boulder's existing SQL queries.
To get additional confidence my regexps work for all of Boulder's queries
I temporarily changed the `db` package's `tableFromQuery` function to panic if
the table couldn't be determined. I re-ran the full unit and integration test
suites with this configuration and saw no panics.
Resolves https://github.com/letsencrypt/boulder/issues/4559
Fixes#4018
This rearranges notify-mailer so we can give it CSV input and interpolate fields from that CSV.
It removes the old-style JSON input so we don't have to support two different input styles.
When multiple accounts have the same email address, their recipient data is consolidated under
that address so they only receive a single email. The CSV data can be interpolated using
the `range` operator in Golang templates.
Because we're now operating on the resolved email addresses instead of purely on accounts,
this PR also changes the checkpointing mode. Instead of a numeric start and end, it takes
a pair of strings, and only sends to email addresses between those two strings.
* Fixes `mockEmailResolver` to return `sql.ErrNoRows`.
This commit reproduces the error observed in #2183 where a registration
ID is provided that doesn't match a row with a valid contact.
First a bug is fixed in the ID range check done by the
`mockEmailResolver` - it was using a `||` where it should have been
using a `&&` and also had a `> 0` where it needed `>= 0`, oops! slipped
past review!
Second the `mockEmailResolver` is modified to return `sql.ErrNoRows`
when the index is out of bounds for the mock data.
Lastly a ID of `999` is added to the `TestResolveEmails` function to
elicit the "mailer.send returned error: sql: no rows in result set"
error.
* Handles `sql.ErrNoRows` in `emailsForReg`.
This commit fixes#2183 (and the failing unit test introduced in the
prior commit) by handling `sql.ErrNoRows` in `emailsForReg` gracefully.
* Clarfies mockEmailResolver comment
This PR adds a `printStatus` function that is called every iteration of the mailer's `run()` loop. The status output is logged at the `info` level and includes the destination email, the current message being sent, the total number of messages to send, and the elapsed time since `run()` started.
The status output can be disabled by lowering the default syslog level in the `notify-mailer` config.
Additionally, this PR adds stats support for the mailer package. Three new stats are
published during the `MailerImpl`'s `SendMail` function (called in a loop by the mailer utilities):
`Mailer.SendMail.Attempts`
`Mailer.SendMail.Successes`
`Mailer.SendMail.Errors`
This PR removes two stats from the `expiration-mailer` that are redundant copies of the above:
`Mailer.Expiration.Errors.SendingNag.SendFailure`
`Mailer.Expiration.Sent`
This resolves#2026.
Presently the `contact-exporter` dumps a list of email addresses to an intermediate file. That file can be provided as input to `notify-mailer` to send messages to the listed email addresses. This has the downside that if a user updates their registration contact address between the time the export is run and the notification emails are sent the message will go to the wrong address.
This PR addresses this by adding a new default output mode to the `contact-exporter` that writes JSON serialized objects containing both a registration ID and an email address to an intermediate file. The `notify-mailer` is updated to support reading this file, deserializing the JSON, and resolving the reg IDs to fresh email addresses before sending messages.
The "classic" behaviour of using bare email addresses as the intermediate form is supported with both the `contact-exporter` and the `notify-mailer` by providing the `--emails` bool flag at the command line.
Resolves#1951
This commit adds a new notify-mailer command. Outside of the new command, this PR also:
Adds a new SMTPConfig to cmd/config.go that is shared between the expiration mailer and the notify mailer.
Modifies mail/mailer.go to add an smtpClient interface.
Adds a dryRunClient to mail/mailer.go that implements the smtpClient interface.
Modifies the mail/mailer.go MailerImpl and constructor to use the SMTPConfig and a dialer. The missing functions from the smtpClient interface are added.
The notify-mailer command supports checkpointing through --start and --end parameters. It supports dry runs by using the new dryRunClient from the mail package when given the --dryRun flag. The speed at which emails are sent can be tweaked using the --sleep flag.
Unit tests for notify-mailer's checkpointing behaviour, the checkpoint interval/sleep parameter sanity, the sleep behaviour, and the message content construction are included in main_test.go.
Future work:
A separate command to generate the list of destination emails provided to notify-mailer
Support for using registration IDs as input and resolving the email address at runtime.
Resolves#1928. Credit to @jsha for the initial work - I'm just completing the branch he started.
* Adds `notify-mailer` command.
* Adds a new SMTPConfig to `cmd/config.go` that is shared between the
expiration mailer and the notify mailer.
* Modifies `mail/mailer.go` to add an `smtpClient` interface.
* Adds a `dryRunClient` to `mail/mailer.go` that implements the
`smtpClient` interface.
* Modifies the `mail/mailer.go` `MailerImpl` and constructor to use the
SMTPConfig and a dialer. The missing functions from the `smtpClient`
interface are added.
* Fix errcheck warnings
* Review feedback
* Review feedback pt2
* Fixes#1446 - invalid message-id generation.
* Change -configFile to -config
* Test message ID with friendly email
https://github.com/letsencrypt/boulder/pull/1936
Aaron Gable