Skip to content

Commit 09ba859

Browse files
jshaDaniel McCarney
authored andcommitted
SA: Deprecate FasterRateLimit feature flag (letsencrypt#4210)
This makes the behavior behind that flag the default.
1 parent 276ce30 commit 09ba859

File tree

8 files changed

+19
-121
lines changed

8 files changed

+19
-121
lines changed

features/featureflag_string.go

Lines changed: 13 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

features/features.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const (
1818
TLSSNIRevalidation
1919
AllowRenewalFirstRL
2020
SetIssuedNamesRenewalBit
21+
FasterRateLimit
2122

2223
// Currently in-use features
2324
// Check CAA and respect validationmethods parameter.
@@ -46,9 +47,6 @@ const (
4647
// RemoveWFE2AccountID will remove the account ID from account objects returned
4748
// from the new-account endpoint if enabled.
4849
RemoveWFE2AccountID
49-
// FasterRateLimit enables use of a separate table for counting the
50-
// Certificates Per Name rate limit.
51-
FasterRateLimit
5250
// CheckRenewalFirst will check whether an issuance is a renewal before
5351
// checking the "certificates per name" rate limit.
5452
CheckRenewalFirst

sa/_db-next/migrations/20190416000000_AddRateLimitTable.sql renamed to sa/_db/migrations/20190101000000_AddRateLimitTable.sql

File renamed without changes.

sa/rate_limits.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"strings"
66
"time"
77

8-
"github.com/letsencrypt/boulder/features"
98
"github.com/weppos/publicsuffix-go/publicsuffix"
109
"golang.org/x/net/context"
1110
)
@@ -37,9 +36,6 @@ func (ssa *SQLStorageAuthority) addCertificatesPerName(
3736
names []string,
3837
timeToTheHour time.Time,
3938
) error {
40-
if !features.Enabled(features.FasterRateLimit) {
41-
return nil
42-
}
4339
// De-duplicate the base domains.
4440
baseDomainsMap := make(map[string]bool)
4541
var qmarks []string

sa/rate_limits_test.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,16 @@ package sa
22

33
import (
44
"fmt"
5-
"os"
6-
"strings"
75
"testing"
86
"time"
97

10-
"github.com/letsencrypt/boulder/features"
11-
128
"golang.org/x/net/context"
139
)
1410

1511
func TestFasterRateLimit(t *testing.T) {
1612
sa, _, cleanUp := initSA(t)
1713
defer cleanUp()
1814

19-
if !strings.HasSuffix(os.Getenv("BOULDER_CONFIG_DIR"), "config-next") {
20-
return
21-
}
22-
features.Set(map[string]bool{"FasterRateLimit": true})
23-
2415
aprilFirst, err := time.Parse(time.RFC3339, "2019-04-01T00:00:00Z")
2516
if err != nil {
2617
t.Fatal(err)

sa/sa.go

Lines changed: 2 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"github.com/letsencrypt/boulder/core"
2121
corepb "github.com/letsencrypt/boulder/core/proto"
2222
berrors "github.com/letsencrypt/boulder/errors"
23-
"github.com/letsencrypt/boulder/features"
2423
bgrpc "github.com/letsencrypt/boulder/grpc"
2524
blog "github.com/letsencrypt/boulder/log"
2625
"github.com/letsencrypt/boulder/metrics"
@@ -106,12 +105,8 @@ func NewSQLStorageAuthority(
106105
parallelismPerRPC: parallelismPerRPC,
107106
}
108107

109-
ssa.countCertificatesByName = ssa.countCertificatesByNameImpl
110-
ssa.countCertificatesByExactName = ssa.countCertificatesByExactNameImpl
111-
if features.Enabled(features.FasterRateLimit) {
112-
ssa.countCertificatesByName = ssa.countCertificatesFaster
113-
ssa.countCertificatesByExactName = ssa.countCertificatesFaster
114-
}
108+
ssa.countCertificatesByName = ssa.countCertificatesFaster
109+
ssa.countCertificatesByExactName = ssa.countCertificatesFaster
115110
ssa.getChallenges = ssa.getChallengesImpl
116111

117112
return ssa, nil
@@ -457,69 +452,6 @@ func ReverseName(domain string) string {
457452
return strings.Join(labels, ".")
458453
}
459454

460-
const countCertificatesSelect = `
461-
SELECT serial from issuedNames
462-
WHERE (reversedName = :reversedDomain OR
463-
reversedName LIKE CONCAT(:reversedDomain, ".%"))
464-
AND NOT renewal AND notBefore > :earliest AND notBefore <= :latest;`
465-
466-
const countCertificatesExactSelect = `
467-
SELECT serial from issuedNames
468-
WHERE reversedName = :reversedDomain
469-
AND NOT renewal AND notBefore > :earliest AND notBefore <= :latest;`
470-
471-
// countCertificatesByNames returns, for a single domain, the count of
472-
// certificates issued in the given time range for that domain and its
473-
// subdomains.
474-
func (ssa *SQLStorageAuthority) countCertificatesByNameImpl(
475-
db dbSelector,
476-
domain string,
477-
earliest,
478-
latest time.Time,
479-
) (int, error) {
480-
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesSelect)
481-
}
482-
483-
// countCertificatesByExactNames returns, for a single domain, the count of
484-
// certificates issued in the given time range for that domain. In contrast to
485-
// countCertificatesByNames subdomains are NOT considered.
486-
func (ssa *SQLStorageAuthority) countCertificatesByExactNameImpl(
487-
db dbSelector,
488-
domain string,
489-
earliest,
490-
latest time.Time,
491-
) (int, error) {
492-
return ssa.countCertificates(db, domain, earliest, latest, countCertificatesExactSelect)
493-
}
494-
495-
// countCertificates returns, for a single domain, the count of certificate
496-
// issuances in the given time range for that domain using the
497-
// provided query assumed to be either `countCertificatesExactSelect`,
498-
// or `countCertificatesSelect`.
499-
func (ssa *SQLStorageAuthority) countCertificates(db dbSelector, domain string, earliest, latest time.Time, query string) (int, error) {
500-
var serials []string
501-
_, err := db.Select(
502-
&serials,
503-
query,
504-
map[string]interface{}{
505-
"reversedDomain": ReverseName(domain),
506-
"earliest": earliest,
507-
"latest": latest,
508-
})
509-
if err == sql.ErrNoRows {
510-
return 0, nil
511-
} else if err != nil {
512-
return 0, err
513-
}
514-
515-
// Deduplicate serials returning a count of unique serials
516-
serialMap := make(map[string]struct{}, len(serials))
517-
for _, s := range serials {
518-
serialMap[s] = struct{}{}
519-
}
520-
return len(serialMap), nil
521-
}
522-
523455
// GetCertificate takes a serial number and returns the corresponding
524456
// certificate, or error if it does not exist.
525457
func (ssa *SQLStorageAuthority) GetCertificate(ctx context.Context, serial string) (core.Certificate, error) {

sa/sa_test.go

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,9 @@ func TestCountCertificatesByNames(t *testing.T) {
565565
cert, err := x509.ParseCertificate(certDER)
566566
test.AssertNotError(t, err, "Couldn't parse example cert DER")
567567

568-
// Set the test clock's time to the time from the test certificate
569-
clk.Add(-clk.Now().Sub(cert.NotBefore))
568+
// Set the test clock's time to the time from the test certificate, plus an
569+
// hour to account for rounding.
570+
clk.Add(time.Hour - clk.Now().Sub(cert.NotBefore))
570571
now := clk.Now()
571572
yesterday := clk.Now().Add(-24 * time.Hour)
572573
twoDaysAgo := clk.Now().Add(-48 * time.Hour)
@@ -2813,20 +2814,6 @@ func TestCountCertificatesRenewalBit(t *testing.T) {
28132814
}
28142815
return 0
28152816
}
2816-
countNameExact := func(t *testing.T, name string) int64 {
2817-
counts, err := sa.CountCertificatesByExactNames(
2818-
context.Background(),
2819-
[]string{name},
2820-
fc.Now().Add(-5*time.Hour),
2821-
fc.Now().Add(5*time.Hour))
2822-
test.AssertNotError(t, err, "Unexpected err from CountCertificatesByExactNames")
2823-
for _, elem := range counts {
2824-
if *elem.Name == name {
2825-
return *elem.Count
2826-
}
2827-
}
2828-
return 0
2829-
}
28302817

28312818
// Add the first certificate - it won't be considered a renewal.
28322819
issued := certA.NotBefore
@@ -2852,11 +2839,6 @@ func TestCountCertificatesRenewalBit(t *testing.T) {
28522839
// The count for the base domain should be 2 now: certA and certC.
28532840
// CertB should be ignored.
28542841
test.AssertEquals(t, countName(t, "not-example.com"), int64(2))
2855-
2856-
// The exact name count for the base domain should be 1: certA. CertB should
2857-
// be ignored as a renewal and CertC should be ignored because it isn't an
2858-
// exact match.
2859-
test.AssertEquals(t, countNameExact(t, "not-example.com"), int64(1))
28602842
}
28612843

28622844
func TestNewAuthorizations2(t *testing.T) {

test/config-next/sa.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
]
2626
},
2727
"features": {
28-
"FasterRateLimit": true
2928
}
3029
},
3130

0 commit comments

Comments
 (0)