Expiry mailer: fetch certificates in bulk (letsencrypt#5607)

aarongable · web-flow · commit e3ce8164253d · 2021-10-28T09:33:20.000-07:00
Use `sa.SelectCertificates` instead of `sa.SelectCertificate` to
fetch the entire batch of certificates all at once, instead of doing
up to 10k individual certificate selections in serial.
diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go
@@ -13,6 +13,7 @@ import (
 	netmail "net/mail"
 	"net/url"
 	"os"
+	"regexp"
 	"sort"
 	"strings"
 	"text/template"
@@ -334,24 +335,34 @@ func (m *mailer) findExpiringCertificates() error {
 		}
 		m.stats.nagsAtCapacity.With(prometheus.Labels{"nag_group": expiresIn.String()}).Set(atCapacity)
 
-		// Now we can sequentially retrieve the certificate details for each of the
-		// certificate status rows
-		var certs []core.Certificate
-		for _, serial := range serials {
-			var cert core.Certificate
-			cert, err := sa.SelectCertificate(m.dbMap, serial)
-			if err != nil {
-				// We can get a NoRowsErr when processing a serial number corresponding
-				// to a precertificate with no final certificate. Since this certificate
-				// is not being used by a subscriber, we don't send expiration email about
-				// it.
-				if db.IsNoRows(err) {
-					continue
-				}
-				m.log.AuditErrf("expiration-mailer: Error loading cert %q: %s", cert.Serial, err)
-				return err
+		if len(serials) == 0 {
+			continue // nothing to do
+		}
+
+		// Wrap every serial in quotes so they can be interpolated into the query.
+		quotedSerials := make([]string, len(serials))
+		serialRegexp := regexp.MustCompile("^[0-9a-f]+$")
+		for i, s := range serials {
+			if !serialRegexp.MatchString(s) {
+				return fmt.Errorf("encountered malformed serial %q", s)
 			}
-			certs = append(certs, cert)
+			quotedSerials[i] = fmt.Sprintf("'%s'", s)
+		}
+
+		// Now we can retrieve the certificate details for all of the status rows.
+		certWithIDs, err := sa.SelectCertificates(
+			m.dbMap,
+			fmt.Sprintf("WHERE serial IN (%s)", strings.Join(quotedSerials, ",")),
+			nil,
+		)
+		if err != nil {
+			m.log.AuditErrf("expiration-mailer: error retrieving certs: %s", err)
+			return err
+		}
+
+		certs := make([]core.Certificate, len(certWithIDs))
+		for i, c := range certWithIDs {
+			certs[i] = c.Certificate
 		}
 
 		m.log.Infof("Found %d certificates expiring between %s and %s", len(certs),
diff --git a/cmd/expiration-mailer/main_test.go b/cmd/expiration-mailer/main_test.go
@@ -724,12 +724,12 @@ func TestDedupOnRegistration(t *testing.T) {
 	}
 	regA, err = testCtx.ssa.NewRegistration(ctx, regA)
 	test.AssertNotError(t, err, "Couldn't store regA")
+
 	rawCertA := newX509Cert("happy A",
 		testCtx.fc.Now().Add(72*time.Hour),
 		[]string{"example-a.com", "shared-example.com"},
 		serial1,
 	)
-
 	certDerA, _ := x509.CreateCertificate(rand.Reader, rawCertA, rawCertA, &testKey.PublicKey, &testKey)
 	certA := &core.Certificate{
 		RegistrationID: regA.Id,
@@ -764,11 +764,8 @@ func TestDedupOnRegistration(t *testing.T) {
 
 	err = testCtx.m.findExpiringCertificates()
 	test.AssertNotError(t, err, "error calling findExpiringCertificates")
-	if len(testCtx.mc.Messages) > 1 {
-		t.Errorf("num of messages, want %d, got %d", 1, len(testCtx.mc.Messages))
-	}
-	if len(testCtx.mc.Messages) == 0 {
-		t.Fatalf("no messages sent")
+	if len(testCtx.mc.Messages) != 1 {
+		t.Fatalf("wrong num of messages, want 1, got %d", len(testCtx.mc.Messages))
 	}
 	domains := "example-a.com\nexample-b.com\nshared-example.com"
 	expected := mocks.MailerMessage{
diff --git a/test/v1_integration.py b/test/v1_integration.py
@@ -402,9 +402,13 @@ def test_expiration_mailer():
 
     requests.post("http://localhost:9381/clear", data='')
     for time in (no_reminder, first_reminder, last_reminder):
-        print(get_future_output(
-            ["./bin/expiration-mailer", "--config", "%s/expiration-mailer.json" % config_dir],
-            time))
+        try:
+            print(get_future_output(
+                ["./bin/expiration-mailer", "--config", "%s/expiration-mailer.json" % config_dir],
+                time))
+        except subprocess.CalledProcessError as e:
+            print(e.output.decode("unicode-escape"))
+            raise
     resp = requests.get("http://localhost:9381/count?to=%s" % email_addr)
     mailcount = int(resp.text)
     if mailcount != 2:
