Skip to content

Commit 8aafb31

Browse files
beautifulentropybeautifulentropy
authored
ratelimits: Small cleanup in transaction.go (#8275)
1 parent 30eac83 commit 8aafb31

File tree

3 files changed

+26
-33
lines changed

3 files changed

+26
-33
lines changed

‎ra/ra.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1521,7 +1521,7 @@ func (ra *RegistrationAuthorityImpl) countFailedValidations(ctx context.Context,
15211521
// resetAccountPausingLimit resets bucket to maximum capacity for given account.
15221522
// There is no reason to surface errors from this function to the Subscriber.
15231523
func (ra *RegistrationAuthorityImpl) resetAccountPausingLimit(ctx context.Context, regId int64, ident identifier.ACMEIdentifier) {
1524-
bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, regId, ident)
1524+
bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, regId, ident.Value)
15251525
err := ra.limiter.Reset(ctx, bucketKey)
15261526
if err != nil {
15271527
ra.log.Warningf("resetting bucket for regID=[%d] identifier=[%s]: %s", regId, ident.Value, err)

‎ra/ra_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ func TestPerformValidation_FailedValidationsTriggerPauseIdentifiersRatelimit(t *
726726
domain := randomDomain()
727727
ident := identifier.NewDNS(domain)
728728
authzPB := createPendingAuthorization(t, sa, ident, fc.Now().Add(12*time.Hour))
729-
bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident)
729+
bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident.Value)
730730

731731
// Set the stored TAT to indicate that this bucket has exhausted its quota.
732732
err = rl.BatchSet(context.Background(), map[string]time.Time{
@@ -802,7 +802,7 @@ func TestPerformValidation_FailedThenSuccessfulValidationResetsPauseIdentifiersR
802802
domain := randomDomain()
803803
ident := identifier.NewDNS(domain)
804804
authzPB := createPendingAuthorization(t, sa, ident, fc.Now().Add(12*time.Hour))
805-
bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident)
805+
bucketKey := ratelimits.NewRegIdIdentValueBucketKey(ratelimits.FailedAuthorizationsForPausingPerDomainPerAccount, authzPB.RegistrationID, ident.Value)
806806

807807
// Set a stored TAT so that we can tell when it's been reset.
808808
err = rl.BatchSet(context.Background(), map[string]time.Time{

‎ratelimits/transaction.go‎

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,26 +41,17 @@ func newRegIdBucketKey(name Name, regId int64) string {
4141
return joinWithColon(name.EnumString(), strconv.FormatInt(regId, 10))
4242
}
4343

44-
// newStringBucketKey validates and returns a bucketKey for limits that use
45-
// the 'enum:identValue' or 'enum:domainOrCIDR' bucket key formats.
46-
func newStringBucketKey(name Name, value string) string {
47-
return joinWithColon(name.EnumString(), value)
44+
// newDomainOrCIDRBucketKey validates and returns a bucketKey for limits that use
45+
// the 'enum:domainOrCIDR' bucket key formats.
46+
func newDomainOrCIDRBucketKey(name Name, domainOrCIDR string) string {
47+
return joinWithColon(name.EnumString(), domainOrCIDR)
4848
}
4949

5050
// NewRegIdIdentValueBucketKey returns a bucketKey for limits that use the
5151
// 'enum:regId:identValue' bucket key format. This function is exported for use
5252
// by the RA when resetting the account pausing limit.
53-
func NewRegIdIdentValueBucketKey(name Name, regId int64, orderIdent identifier.ACMEIdentifier) string {
54-
return newRegIdStringBucketKey(name, regId, orderIdent.Value)
55-
}
56-
57-
// newRegIdStringBucketKey returns a bucketKey for limits that use the
58-
// 'enum:regId:identValue' or 'enum:regId:domainOrCIDR' bucket key formats.
59-
//
60-
// This is split out from NewRegIdIdentValueBucketKey so that we can handle an
61-
// IP prefix in CIDR notation, which is not a valid identifier value.
62-
func newRegIdStringBucketKey(name Name, regId int64, value string) string {
63-
return joinWithColon(name.EnumString(), strconv.FormatInt(regId, 10), value)
53+
func NewRegIdIdentValueBucketKey(name Name, regId int64, orderIdent string) string {
54+
return joinWithColon(name.EnumString(), strconv.FormatInt(regId, 10), orderIdent)
6455
}
6556

6657
// newFQDNSetBucketKey validates and returns a bucketKey for limits that use the
@@ -256,7 +247,7 @@ func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountCheckO
256247
for _, ident := range orderIdents {
257248
// FailedAuthorizationsPerDomainPerAccount limit uses the
258249
// 'enum:regId:identValue' bucket key format for transactions.
259-
perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, ident)
250+
perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, ident.Value)
260251

261252
// Add a check-only transaction for each per identValue per account
262253
// bucket.
@@ -287,7 +278,7 @@ func (builder *TransactionBuilder) FailedAuthorizationsPerDomainPerAccountSpendO
287278

288279
// FailedAuthorizationsPerDomainPerAccount limit uses the
289280
// 'enum:regId:identValue' bucket key format for transactions.
290-
perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, orderIdent)
281+
perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsPerDomainPerAccount, regId, orderIdent.Value)
291282
txn, err := newSpendOnlyTransaction(limit, perIdentValuePerAccountBucketKey, 1)
292283
if err != nil {
293284
return Transaction{}, err
@@ -314,7 +305,7 @@ func (builder *TransactionBuilder) FailedAuthorizationsForPausingPerDomainPerAcc
314305

315306
// FailedAuthorizationsForPausingPerDomainPerAccount limit uses the
316307
// 'enum:regId:identValue' bucket key format for transactions.
317-
perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsForPausingPerDomainPerAccount, regId, orderIdent)
308+
perIdentValuePerAccountBucketKey := NewRegIdIdentValueBucketKey(FailedAuthorizationsForPausingPerDomainPerAccount, regId, orderIdent.Value)
318309
txn, err := newTransaction(limit, perIdentValuePerAccountBucketKey, 1)
319310
if err != nil {
320311
return Transaction{}, err
@@ -354,21 +345,22 @@ func (builder *TransactionBuilder) certificatesPerDomainCheckOnlyTransactions(re
354345
}
355346
}
356347

357-
var txns []Transaction
358-
covers, err := coveringIdentifiers(orderIdents)
348+
coveringIdents, err := coveringIdentifiers(orderIdents)
359349
if err != nil {
360350
return nil, err
361351
}
362-
for _, name := range covers {
363-
perDomainOrCIDRBucketKey := newStringBucketKey(CertificatesPerDomain, name)
352+
353+
var txns []Transaction
354+
for _, ident := range coveringIdents {
355+
perDomainOrCIDRBucketKey := newDomainOrCIDRBucketKey(CertificatesPerDomain, ident)
364356
if accountOverride {
365357
if !perAccountLimit.isOverride {
366358
return nil, fmt.Errorf("shouldn't happen: CertificatesPerDomainPerAccount limit is not an override")
367359
}
368-
perAccountPerDomainOrCIDRKey := newRegIdStringBucketKey(CertificatesPerDomainPerAccount, regId, name)
360+
perAccountPerDomainOrCIDRBucketKey := NewRegIdIdentValueBucketKey(CertificatesPerDomainPerAccount, regId, ident)
369361
// Add a check-only transaction for each per account per identValue
370362
// bucket.
371-
txn, err := newCheckOnlyTransaction(perAccountLimit, perAccountPerDomainOrCIDRKey, 1)
363+
txn, err := newCheckOnlyTransaction(perAccountLimit, perAccountPerDomainOrCIDRBucketKey, 1)
372364
if err != nil {
373365
if errors.Is(err, errLimitDisabled) {
374366
continue
@@ -432,21 +424,22 @@ func (builder *TransactionBuilder) CertificatesPerDomainSpendOnlyTransactions(re
432424
}
433425
}
434426

435-
var txns []Transaction
436-
covers, err := coveringIdentifiers(orderIdents)
427+
coveringIdents, err := coveringIdentifiers(orderIdents)
437428
if err != nil {
438429
return nil, err
439430
}
440-
for _, name := range covers {
441-
perDomainOrCIDRBucketKey := newStringBucketKey(CertificatesPerDomain, name)
431+
432+
var txns []Transaction
433+
for _, ident := range coveringIdents {
434+
perDomainOrCIDRBucketKey := newDomainOrCIDRBucketKey(CertificatesPerDomain, ident)
442435
if accountOverride {
443436
if !perAccountLimit.isOverride {
444437
return nil, fmt.Errorf("shouldn't happen: CertificatesPerDomainPerAccount limit is not an override")
445438
}
446-
perAccountPerDomainOrCIDRKey := newRegIdStringBucketKey(CertificatesPerDomainPerAccount, regId, name)
439+
perAccountPerDomainOrCIDRBucketKey := NewRegIdIdentValueBucketKey(CertificatesPerDomainPerAccount, regId, ident)
447440
// Add a spend-only transaction for each per account per
448441
// domainOrCIDR bucket.
449-
txn, err := newSpendOnlyTransaction(perAccountLimit, perAccountPerDomainOrCIDRKey, 1)
442+
txn, err := newSpendOnlyTransaction(perAccountLimit, perAccountPerDomainOrCIDRBucketKey, 1)
450443
if err != nil {
451444
return nil, err
452445
}

0 commit comments

Comments
 (0)