Skip to content

Commit

Permalink
Deprecate DisableLegacyLimitWrites & UseKvLimitsForNewOrder flags; re…
Browse files Browse the repository at this point in the history
…move code using certificatesPerName & newOrdersRL tables (#7858)

Remove code using `certificatesPerName` & `newOrdersRL` tables.

Deprecate `DisableLegacyLimitWrites` & `UseKvLimitsForNewOrder` flags.

Remove legacy `ratelimit` package.

Delete these RA test cases:

- `TestAuthzFailedRateLimitingNewOrder` (rl:
`FailedAuthorizationsPerDomainPerAccount`)
- `TestCheckCertificatesPerNameLimit` (rl: `CertificatesPerDomain`)
- `TestCheckExactCertificateLimit` (rl: `CertificatesPerFQDNSet`)
- `TestExactPublicSuffixCertLimit` (rl: `CertificatesPerDomain`)

Rate limits in NewOrder are now enforced by the WFE, starting here:
https://github.com/letsencrypt/boulder/blob/5a9b4c4b18fd0aa670bc6332bdd59701ff7d6186/wfe2/wfe.go#L781

We collect a batch of transactions to check limits, check them all at
once, go through and find which one(s) failed, and serve the failure
with the Retry-After that's furthest in the future. All this code
doesn't really need to be tested again; what needs to be tested is that
we're returning the correct failure. That code is
`NewOrderLimitTransactions`, and the `ratelimits` package's tests cover
this.

The public suffix handling behavior is tested by
`TestFQDNsToETLDsPlusOne`:
https://github.com/letsencrypt/boulder/blob/5a9b4c4b18fd0aa670bc6332bdd59701ff7d6186/ratelimits/utilities_test.go#L9

Some other RA rate limit tests were deleted earlier, in #7869.

Part of #7671.
  • Loading branch information
jprenken authored Jan 10, 2025
1 parent f37c362 commit e4668b4
Show file tree
Hide file tree
Showing 31 changed files with 1,189 additions and 3,412 deletions.
5 changes: 2 additions & 3 deletions cmd/boulder-ra/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ type Config struct {
cmd.ServiceConfig
cmd.HostnamePolicyConfig

RateLimitPoliciesFilename string `validate:"required"`
// RateLimitPoliciesFilename is deprecated.
RateLimitPoliciesFilename string

MaxContactsPerRegistration int

Expand Down Expand Up @@ -297,8 +298,6 @@ func main() {
)
defer rai.Drain()

policyErr := rai.LoadRateLimitPoliciesFile(c.RA.RateLimitPoliciesFilename)
cmd.FailOnError(policyErr, "Couldn't load rate limit policies file")
rai.PA = pa

rai.VA = va.RemoteClients{
Expand Down
5 changes: 1 addition & 4 deletions db/map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/letsencrypt/borp"

"github.com/go-sql-driver/mysql"

"github.com/letsencrypt/boulder/core"
"github.com/letsencrypt/boulder/test"
"github.com/letsencrypt/boulder/test/vars"
Expand Down Expand Up @@ -185,10 +186,6 @@ func TestTableFromQuery(t *testing.T) {
query: "insert into `certificates` (`registrationID`,`serial`,`digest`,`der`,`issued`,`expires`) values (?,?,?,?,?,?);",
expectedTable: "`certificates`",
},
{
query: "INSERT INTO certificatesPerName (eTLDPlusOne, time, count) VALUES (?, ?, ?) ON DUPLICATE KEY UPDATE count=count+1;",
expectedTable: "certificatesPerName",
},
{
query: "insert into `fqdnSets` (`ID`,`SetHash`,`Serial`,`Issued`,`Expires`) values (null,?,?,?,?);",
expectedTable: "`fqdnSets`",
Expand Down
19 changes: 3 additions & 16 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import (
// package's global Config.
type Config struct {
// Deprecated flags.
IncrementRateLimits bool
IncrementRateLimits bool
UseKvLimitsForNewOrder bool
DisableLegacyLimitWrites bool

// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
// GET requests. WARNING: This feature is a draft and highly unstable.
Expand Down Expand Up @@ -67,21 +69,6 @@ type Config struct {
// until the paused identifiers are unpaused and the order is resubmitted.
CheckIdentifiersPaused bool

// UseKvLimitsForNewOrder when enabled, causes the key-value rate limiter to
// be the authoritative source of rate limiting information for new-order
// callers and disables the legacy rate limiting checks.
//
// Note: this flag does not disable writes to the certificatesPerName or
// fqdnSets tables at Finalize time.
UseKvLimitsForNewOrder bool

// DisableLegacyLimitWrites when enabled, disables writes to:
// - the newOrdersRL table at new-order time, and
// - the certificatesPerName table at finalize time.
//
// This flag should only be used in conjunction with UseKvLimitsForNewOrder.
DisableLegacyLimitWrites bool

// PropagateCancels controls whether the WFE and ocsp-responder allows
// cancellation of an inbound request to cancel downstream gRPC and other
// queries. In practice, cancellation of an inbound request is achieved by
Expand Down
10 changes: 0 additions & 10 deletions mocks/sa.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,16 +311,6 @@ func (sa *StorageAuthorityReadOnly) FQDNSetExists(_ context.Context, _ *sapb.FQD
return &sapb.Exists{Exists: false}, nil
}

// CountCertificatesByNames is a mock
func (sa *StorageAuthorityReadOnly) CountCertificatesByNames(_ context.Context, _ *sapb.CountCertificatesByNamesRequest, _ ...grpc.CallOption) (*sapb.CountByNames, error) {
return &sapb.CountByNames{}, nil
}

// CountOrders is a mock
func (sa *StorageAuthorityReadOnly) CountOrders(_ context.Context, _ *sapb.CountOrdersRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
return &sapb.Count{}, nil
}

// DeactivateRegistration is a mock
func (sa *StorageAuthority) DeactivateRegistration(_ context.Context, _ *sapb.RegistrationID, _ ...grpc.CallOption) (*emptypb.Empty, error) {
return &emptypb.Empty{}, nil
Expand Down
Loading

0 comments on commit e4668b4

Please sign in to comment.