Skip to content

ratelimits: Check at NewOrder and SpendOnly later#7669

Merged
beautifulentropy merged 3 commits into
mainfrom
ratelimits-spend-only-later
Aug 15, 2024
Merged

ratelimits: Check at NewOrder and SpendOnly later#7669
beautifulentropy merged 3 commits into
mainfrom
ratelimits-spend-only-later

Conversation

@beautifulentropy

@beautifulentropy beautifulentropy commented Aug 15, 2024

Copy link
Copy Markdown
Member
  • Check CertificatesPerDomain at newOrder and spend at Finalize time.
  • Check CertificatesPerAccountPerDomain at newOrder and spend at Finalize time.
  • Check CertificatesPerFQDNSet at newOrder and spend at Finalize time.
  • Fix a bug inFailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction() which results in failed authorizations being spent for the exact FQDN, not the eTLD+1.
  • Remove redundant "max names" check at transaction construction time
  • Enable key-value rate limits in the RA

@beautifulentropy beautifulentropy force-pushed the ratelimits-spend-only-later branch 2 times, most recently from 73ed05b to 315ab1a Compare August 15, 2024 21:51
@beautifulentropy beautifulentropy marked this pull request as ready for review August 15, 2024 21:51
@beautifulentropy beautifulentropy requested a review from a team as a code owner August 15, 2024 21:51
@beautifulentropy beautifulentropy requested review from aarongable and jsha and removed request for jsha August 15, 2024 21:51
@beautifulentropy beautifulentropy force-pushed the ratelimits-spend-only-later branch from 315ab1a to e9ee493 Compare August 15, 2024 21:59
@beautifulentropy beautifulentropy force-pushed the ratelimits-spend-only-later branch from e9ee493 to 08170d7 Compare August 15, 2024 22:01
aarongable
aarongable previously approved these changes Aug 15, 2024
Comment thread ra/ra.go Outdated
aarongable
aarongable previously approved these changes Aug 15, 2024
@github-actions

Copy link
Copy Markdown
Contributor

@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

@aarongable aarongable left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % filing a ticket for the RA to be hooked up to the rate limits redis cluster

@beautifulentropy

Copy link
Copy Markdown
Member Author

@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

Filing a ticket with #7666 to ensure that the RA gets hooked up and I'll ensure that there's no dependence on it unless it's configured and the flag is flipped.

@beautifulentropy beautifulentropy merged commit 14c0b2c into main Aug 15, 2024
@beautifulentropy beautifulentropy deleted the ratelimits-spend-only-later branch August 15, 2024 23:08
beautifulentropy added a commit that referenced this pull request Aug 16, 2024
- Check `CertificatesPerDomain` at newOrder and spend at Finalize time.
- Check `CertificatesPerAccountPerDomain` at newOrder and spend at
Finalize time.
- Check `CertificatesPerFQDNSet` at newOrder and spend at Finalize time.
- Fix a bug
in`FailedAuthorizationsPerDomainPerAccountSpendOnlyTransaction()` which
results in failed authorizations being spent for the exact FQDN, not the
eTLD+1.
- Remove redundant "max names" check at transaction construction time
- Enable key-value rate limits in the RA
beautifulentropy added a commit that referenced this pull request Aug 21, 2024
This bug was introduced in
#7669.

Also, make calls to ra.countCertificateIssued() non-blocking like
ra.countFailedValidation().

Part of #7664
Blocks #7666
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants