Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove legacy rate limits code #7671

Closed
11 tasks done
Tracked by #5545
beautifulentropy opened this issue Aug 16, 2024 · 0 comments · Fixed by #7932
Closed
11 tasks done
Tracked by #5545

Remove legacy rate limits code #7671

beautifulentropy opened this issue Aug 16, 2024 · 0 comments · Fixed by #7932
Assignees
@jprenken
Milestone
Sprint 2025-01-07

Comments

@beautifulentropy
Copy link
Member

beautifulentropy commented Aug 16, 2024 •
edited
Loading

  • Remove nearly all of the countRegistrationsByIP, except the supporting field InitialIP in core.Registration. Default this field to "0.0.0.0" in the WFE and the SA (ratelimit: Remove legacy registrations per IP implementation #7760)
  • Remove InitialIP from core.Registration
  • Remove all code referencing certificatesPerName table
  • Remove all code referencing newOrdersRL table
  • Drop certificatesPerName and newOrdersRL tables in db-next schema
  • Deprecate DisableLegacyLimitWrites feature flag and delete all code it conditions
  • Deprecate UseKvLimitsForNewOrder flag and delete all code it conditions
  • Remove if wfe.limiter == nil && wfe.txnBuilder == nil in checkNewAccountLimits and checkNewOrderLimits WFE methods.
  • Remove if ra.limiter == nil && ra.txnBuilder == nil in countFailedValidations() and countCertificateIssued() RA methods.
  • Remove limitsExempt from NewOrderRequest proto.
  • Remove isRenewal from NewOrderRequest proto.
@beautifulentropy beautifulentropy mentioned this issue Aug 16, 2024
Closed
18 tasks
@aarongable aarongable added this to the Sprint 2024-11-19 milestone Nov 19, 2024
@beautifulentropy beautifulentropy mentioned this issue Nov 19, 2024
Merged
beautifulentropy added a commit that referenced this issue Nov 19, 2024
@beautifulentropy
ratelimit: Remove legacy registrations per IP implementation (#7760)
a8cdaf8 
Part of #7671
@jprenken jprenken mentioned this issue Dec 7, 2024
Merged
@aarongable aarongable mentioned this issue Dec 18, 2024
Closed
2 tasks
jprenken added a commit that referenced this issue Jan 10, 2025
@jprenken
Deprecate DisableLegacyLimitWrites & UseKvLimitsForNewOrder flags; re…
e4668b4 
…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.
jprenken added a commit that referenced this issue Jan 11, 2025
@jprenken
ra: Remove isRenewal & isARIRenewal from NewOrderRequest proto
2ab6749 
Fixes #7671
@jprenken jprenken mentioned this issue Jan 11, 2025
Merged
beautifulentropy pushed a commit that referenced this issue Jan 13, 2025
@jprenken
ra: Remove isRenewal & isARIRenewal from NewOrderRequest proto (#7932)
274d446 
Fixes #7671 
Fixes #5545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jprenken jprenken

Labels
None yet
Projects
None yet
Milestone
Sprint 2025-01-07
Development

Successfully merging a pull request may close this issue.

ra: Remove isRenewal & isARIRenewal from NewOrderRequest proto letsencrypt/boulder
3 participants
@aarongable @jprenken @beautifulentropy