From 14700671af7b39238149650edd0e562d3cd8af43 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 27 Jul 2015 15:05:43 -0700 Subject: [PATCH] Review fixes, the sequel --- cmd/expiration-mailer/main.go | 53 ++++++++++++++++++++---------- cmd/expiration-mailer/main_test.go | 12 +++---- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index b70c225ab..c306d92cc 100644 --- a/cmd/expiration-mailer/main.go +++ b/cmd/expiration-mailer/main.go @@ -10,6 +10,7 @@ import ( "crypto/x509" "fmt" "io/ioutil" + "sort" "strings" "text/template" "time" @@ -35,10 +36,10 @@ type mailer struct { stats statsd.Statter log *blog.AuditLogger dbMap *gorp.DbMap - Mailer mail.Mailer - EmailTemplate *template.Template - NagTimes []time.Duration - Limit int + mailer mail.Mailer + emailTemplate *template.Template + nagTimes []time.Duration + limit int } func (m *mailer) sendNags(parsedCert *x509.Certificate, contacts []core.AcmeURL) error { @@ -56,12 +57,12 @@ func (m *mailer) sendNags(parsedCert *x509.Certificate, contacts []core.AcmeURL) DNSNames: strings.Join(parsedCert.DNSNames, ", "), } msgBuf := new(bytes.Buffer) - err := m.EmailTemplate.Execute(msgBuf, email) + err := m.emailTemplate.Execute(msgBuf, email) if err != nil { m.stats.Inc("Mailer.Errors.SendingNag.TemplateFailure", 1, 1.0) return err } - err = m.Mailer.SendMail(emails, msgBuf.String()) + err = m.mailer.SendMail(emails, msgBuf.String()) if err != nil { m.stats.Inc("Mailer.Errors.SendingNag.SendFailure", 1, 1.0) return err @@ -142,10 +143,10 @@ func (m *mailer) processCerts(certs []core.Certificate) { func (m *mailer) findExpiringCertificates() error { now := time.Now() // E.g. m.NagTimes = [1, 3, 7, 14] days from expiration - for i, expiresIn := range m.NagTimes { + for i, expiresIn := range m.nagTimes { left := now if i > 0 { - left = left.Add(m.NagTimes[i-1]) + left = left.Add(m.nagTimes[i-1]) } right := now.Add(expiresIn) @@ -156,7 +157,9 @@ func (m *mailer) findExpiringCertificates() error { `SELECT cert.* FROM certificates AS cert JOIN certificateStatus AS cs ON cs.serial = cert.serial - WHERE cert.expires > :cutoffA AND cert.expires < :cutoffB AND cert.status != "revoked" + WHERE cert.expires > :cutoffA + AND cert.expires < :cutoffB + AND cert.status != "revoked" AND cs.lastExpirationNagSent <= :nagCutoff ORDER BY cert.expires ASC LIMIT :limit`, @@ -164,7 +167,7 @@ func (m *mailer) findExpiringCertificates() error { "cutoffA": left, "cutoffB": right, "nagCutoff": time.Now().Add(-expiresIn), - "limit": m.Limit, + "limit": m.limit, }, ) if err != nil { @@ -179,18 +182,32 @@ func (m *mailer) findExpiringCertificates() error { return nil } +type durationSlice []time.Duration + +func (ds durationSlice) Len() int { + return len(ds) +} + +func (ds durationSlice) Less(a, b int) bool { + return ds[a] < ds[b] +} + +func (ds durationSlice) Swap(a, b int) { + ds[a], ds[b] = ds[b], ds[a] +} + func main() { app := cmd.NewAppShell("expiration-mailer") app.App.Flags = append(app.App.Flags, cli.IntFlag{ Name: "cert_limit", Value: 100, - EnvVar: "cert_LIMIT", + EnvVar: "CERT_LIMIT", Usage: "Count of certificates to process per expiration period", }) app.Config = func(c *cli.Context, config cmd.Config) cmd.Config { - if c.GlobalInt("email_limit") > 0 { + if c.GlobalInt("cert_limit") > 0 { config.Mailer.CertLimit = c.GlobalInt("cert_limit") } return config @@ -225,7 +242,7 @@ func main() { mailClient := mail.New(c.Mailer.Server, c.Mailer.Port, c.Mailer.Username, c.Mailer.Password) - var nags []time.Duration + var nags durationSlice for _, nagDuration := range c.Mailer.NagTimes { dur, err := time.ParseDuration(nagDuration) if err != nil { @@ -234,15 +251,17 @@ func main() { } nags = append(nags, dur) } + // Make sure durations are sorted in increasing order + sort.Sort(nags) m := mailer{ stats: stats, log: auditlogger, dbMap: dbMap, - Mailer: &mailClient, - EmailTemplate: tmpl, - NagTimes: nags, - Limit: c.Mailer.CertLimit, + mailer: &mailClient, + emailTemplate: tmpl, + nagTimes: nags, + limit: c.Mailer.CertLimit, } auditlogger.Info("expiration-mailer: Starting") diff --git a/cmd/expiration-mailer/main_test.go b/cmd/expiration-mailer/main_test.go index eb01f9951..40c84100a 100644 --- a/cmd/expiration-mailer/main_test.go +++ b/cmd/expiration-mailer/main_test.go @@ -77,8 +77,8 @@ func TestSendNags(t *testing.T) { mc := mockMail{} m := mailer{ stats: stats, - Mailer: &mc, - EmailTemplate: tmpl, + mailer: &mc, + emailTemplate: tmpl, } cert := &x509.Certificate{ @@ -134,11 +134,11 @@ func TestFindExpiringCertificates(t *testing.T) { m := mailer{ log: blog.GetAuditLogger(), stats: stats, - Mailer: &mc, - EmailTemplate: tmpl, + mailer: &mc, + emailTemplate: tmpl, dbMap: dbMap, - NagTimes: []time.Duration{time.Hour * 24, time.Hour * 24 * 4, time.Hour * 24 * 7}, - Limit: 100, + nagTimes: []time.Duration{time.Hour * 24, time.Hour * 24 * 4, time.Hour * 24 * 7}, + limit: 100, } log.Clear()