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

ratelimits: API improvements necessary for batches and limit fixes #7117

Merged
merged 8 commits into from
Nov 8, 2023
Merged

ratelimits: API improvements necessary for batches and limit fixes #7117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
76a020a
ratelimits: API improvements necessary for batches and limit fixes
beautifulentropy Oct 13, 2023
3ce9716
Revert accidental removal.
beautifulentropy Oct 25, 2023
af12f17
Fix merge issue.
beautifulentropy Oct 25, 2023
376f822
Addressing second round comments.
beautifulentropy Oct 26, 2023
8276711
Fix typo.
beautifulentropy Oct 27, 2023
81df1d7
Apply suggestions from code review
beautifulentropy Oct 27, 2023
45d1849
Addressing comments.
beautifulentropy Nov 6, 2023
8269947
Remove newline that somehow got added.
beautifulentropy Nov 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions ratelimits/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,21 @@ Example: `NewRegistrationsPerIPv6Range:2001:0db8:0000::/48`

#### regId

The registration ID of the account.
An ACME account registration ID.

Example: `NewOrdersPerAccount:12345678`

#### regId:domain
#### domain

A combination of registration ID and domain, formatted 'regId:domain'.
A valid eTLD+1 domain name.

Example: `CertificatesPerDomainPerAccount:12345678:example.com`
Example: `CertificatesPerDomain:example.com`

#### regId:fqdnSet
#### fqdnSet

A combination of registration ID and a comma-separated list of domain names,
formatted 'regId:fqdnSet'.
A comma-separated list of domain names.

Example: `CertificatesPerFQDNSetPerAccount:12345678:example.com,example.org`
Example: `CertificatesPerFQDNSet:example.com,example.org`

## Bucket Key Definitions

Expand Down
52 changes: 52 additions & 0 deletions ratelimits/bucket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package ratelimits

import (
"fmt"
"net"
)

// BucketId should only be created using the New*BucketId functions. It is used
// by the Limiter to look up the bucket and limit overrides for a specific
// subscriber and limit.
type BucketId struct {
// limit is the name of the associated rate limit. It is used for looking up
// default limits.
limit Name

// bucketKey is the limit Name enum (e.g. "1") concatenated with the
// subscriber identifier specific to the associate limit Name type.
bucketKey string
}

// NewRegistrationsPerIPAddressBucketId returns a BucketId for the provided IP
// address.
func NewRegistrationsPerIPAddressBucketId(ip net.IP) (BucketId, error) {
id := ip.String()
err := validateIdForName(NewRegistrationsPerIPAddress, id)
if err != nil {
return BucketId{}, err
}
return BucketId{
limit: NewRegistrationsPerIPAddress,
bucketKey: joinWithColon(NewRegistrationsPerIPAddress.EnumString(), id),
}, nil
}

// NewRegistrationsPerIPv6RangeBucketId returns a BucketId for the /48 IPv6
// range containing the provided IPv6 address.
func NewRegistrationsPerIPv6RangeBucketId(ip net.IP) (BucketId, error) {
if ip.To4() != nil {
return BucketId{}, fmt.Errorf("invalid IPv6 address, %q must be an IPv6 address", ip.String())
}
ipMask := net.CIDRMask(48, 128)
ipNet := &net.IPNet{IP: ip.Mask(ipMask), Mask: ipMask}
id := ipNet.String()
err := validateIdForName(NewRegistrationsPerIPv6Range, id)
if err != nil {
return BucketId{}, err
}
return BucketId{
limit: NewRegistrationsPerIPv6Range,
bucketKey: joinWithColon(NewRegistrationsPerIPv6Range.EnumString(), id),
}, nil
}
16 changes: 4 additions & 12 deletions ratelimits/limit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,14 @@ func loadAndParseOverrideLimits(path string) (limits, error) {
return nil, fmt.Errorf(
"validating name %s and id %q for override limit %q: %w", name, id, k, err)
}
if name == CertificatesPerFQDNSetPerAccount {
if name == CertificatesPerFQDNSet {
// FQDNSet hashes are not a nice thing to ask for in a config file,
// so we allow the user to specify a comma-separated list of FQDNs
// and compute the hash here.
regIdDomains := strings.SplitN(id, ":", 2)
if len(regIdDomains) != 2 {
// Should never happen, the Id format was validated above.
return nil, fmt.Errorf("invalid override limit %q, must be formatted 'name:id'", k)
}
regId := regIdDomains[0]
domains := strings.Split(regIdDomains[1], ",")
fqdnSet := core.HashNames(domains)
id = fmt.Sprintf("%s:%s", regId, fqdnSet)
id = string(core.HashNames(strings.Split(id, ",")))
}
v.isOverride = true
parsed[bucketKey(name, id)] = precomputeLimit(v)
parsed[joinWithColon(name.EnumString(), id)] = precomputeLimit(v)
}
return parsed, nil
}
Expand All @@ -159,7 +151,7 @@ func loadAndParseDefaultLimits(path string) (limits, error) {
if !ok {
return nil, fmt.Errorf("unrecognized name %q in default limit, must be one of %v", k, limitNames)
}
parsed[nameToEnumString(name)] = precomputeLimit(v)
parsed[name.EnumString()] = precomputeLimit(v)
}
return parsed, nil
}
120 changes: 33 additions & 87 deletions ratelimits/limit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,19 @@ func Test_validateIdForName(t *testing.T) {
err = validateIdForName(NewOrdersPerAccount, "1234567890")
test.AssertNotError(t, err, "valid regId")

// 'enum:regId:domain'
// 'enum:domain'
// Valid regId and domain.
err = validateIdForName(CertificatesPerDomainPerAccount, "1234567890:example.com")
err = validateIdForName(CertificatesPerDomain, "example.com")
test.AssertNotError(t, err, "valid regId and domain")

// 'enum:regId:fqdnSet'
// Valid regId and FQDN set containing a single domain.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "1234567890:example.com")
// 'enum:fqdnSet'
// Valid fqdnSet containing a single domain.
err = validateIdForName(CertificatesPerFQDNSet, "example.com")
test.AssertNotError(t, err, "valid regId and FQDN set containing a single domain")

// 'enum:regId:fqdnSet'
// Valid regId and FQDN set containing multiple domains.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "1234567890:example.com,example.org")
// 'enum:fqdnSet'
// Valid fqdnSet containing multiple domains.
err = validateIdForName(CertificatesPerFQDNSet, "example.com,example.org")
test.AssertNotError(t, err, "valid regId and FQDN set containing multiple domains")

// Empty string.
Expand Down Expand Up @@ -125,107 +125,56 @@ func Test_validateIdForName(t *testing.T) {
err = validateIdForName(NewOrdersPerAccount, "lol")
test.AssertError(t, err, "invalid regId")

// Invalid regId with good domain.
err = validateIdForName(CertificatesPerDomainPerAccount, "lol:example.com")
test.AssertError(t, err, "invalid regId with good domain")

// Valid regId with bad domain.
err = validateIdForName(CertificatesPerDomainPerAccount, "1234567890:lol")
test.AssertError(t, err, "valid regId with bad domain")

// Empty regId with good domain.
err = validateIdForName(CertificatesPerDomainPerAccount, ":lol")
// Invalid domain, malformed.
err = validateIdForName(CertificatesPerDomain, "example:.com")
test.AssertError(t, err, "valid regId with bad domain")

// Valid regId with empty domain.
err = validateIdForName(CertificatesPerDomainPerAccount, "1234567890:")
// Invalid domain, empty.
err = validateIdForName(CertificatesPerDomain, "")
test.AssertError(t, err, "valid regId with empty domain")

// Empty regId with empty domain, no separator.
err = validateIdForName(CertificatesPerDomainPerAccount, "")
test.AssertError(t, err, "empty regId with empty domain, no separator")

// Instead of anything we would expect, we get lol.
err = validateIdForName(CertificatesPerDomainPerAccount, "lol")
test.AssertError(t, err, "instead of anything we would expect, just lol")

// Valid regId with good domain and a secret third separator.
err = validateIdForName(CertificatesPerDomainPerAccount, "1234567890:example.com:lol")
test.AssertError(t, err, "valid regId with good domain and a secret third separator")

// Valid regId with bad FQDN set.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "1234567890:lol..99")
test.AssertError(t, err, "valid regId with bad FQDN set")

// Bad regId with good FQDN set.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "lol:example.com,example.org")
test.AssertError(t, err, "bad regId with good FQDN set")

// Empty regId with good FQDN set.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, ":example.com,example.org")
test.AssertError(t, err, "empty regId with good FQDN set")

// Good regId with empty FQDN set.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "1234567890:")
test.AssertError(t, err, "good regId with empty FQDN set")

// Empty regId with empty FQDN set, no separator.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "")
test.AssertError(t, err, "empty regId with empty FQDN set, no separator")

// Instead of anything we would expect, just lol.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "lol")
test.AssertError(t, err, "instead of anything we would expect, just lol")

// Valid regId with good FQDN set and a secret third separator.
err = validateIdForName(CertificatesPerFQDNSetPerAccount, "1234567890:example.com,example.org:lol")
test.AssertError(t, err, "valid regId with good FQDN set and a secret third separator")
}

func Test_loadAndParseOverrideLimits(t *testing.T) {
newRegistrationsPerIPAddressEnumStr := nameToEnumString(NewRegistrationsPerIPAddress)
newRegistrationsPerIPv6RangeEnumStr := nameToEnumString(NewRegistrationsPerIPv6Range)

// Load a single valid override limit with Id formatted as 'enum:RegId'.
l, err := loadAndParseOverrideLimits("testdata/working_override.yml")
test.AssertNotError(t, err, "valid single override limit")
expectKey := newRegistrationsPerIPAddressEnumStr + ":" + "10.0.0.2"
expectKey := joinWithColon(NewRegistrationsPerIPAddress.EnumString(), "10.0.0.2")
test.AssertEquals(t, l[expectKey].Burst, int64(40))
test.AssertEquals(t, l[expectKey].Count, int64(40))
test.AssertEquals(t, l[expectKey].Period.Duration, time.Second)

// Load single valid override limit with Id formatted as 'regId:domain'.
l, err = loadAndParseOverrideLimits("testdata/working_override_regid_domain.yml")
test.AssertNotError(t, err, "valid single override limit with Id of regId:domain")
expectKey = nameToEnumString(CertificatesPerDomainPerAccount) + ":" + "12345678:example.com"
expectKey = joinWithColon(CertificatesPerDomain.EnumString(), "example.com")
test.AssertEquals(t, l[expectKey].Burst, int64(40))
test.AssertEquals(t, l[expectKey].Count, int64(40))
test.AssertEquals(t, l[expectKey].Period.Duration, time.Second)

// Load multiple valid override limits with 'enum:RegId' Ids.
l, err = loadAndParseOverrideLimits("testdata/working_overrides.yml")
expectKey1 := newRegistrationsPerIPAddressEnumStr + ":" + "10.0.0.2"
expectKey1 := joinWithColon(NewRegistrationsPerIPAddress.EnumString(), "10.0.0.2")
test.AssertNotError(t, err, "multiple valid override limits")
test.AssertEquals(t, l[expectKey1].Burst, int64(40))
test.AssertEquals(t, l[expectKey1].Count, int64(40))
test.AssertEquals(t, l[expectKey1].Period.Duration, time.Second)
expectKey2 := newRegistrationsPerIPv6RangeEnumStr + ":" + "2001:0db8:0000::/48"
expectKey2 := joinWithColon(NewRegistrationsPerIPv6Range.EnumString(), "2001:0db8:0000::/48")
test.AssertEquals(t, l[expectKey2].Burst, int64(50))
test.AssertEquals(t, l[expectKey2].Count, int64(50))
test.AssertEquals(t, l[expectKey2].Period.Duration, time.Second*2)

// Load multiple valid override limits with 'regID:fqdnSet' Ids as follows:
// - CertificatesPerFQDNSetPerAccount:12345678:example.com
// - CertificatesPerFQDNSetPerAccount:12345678:example.com,example.net
// - CertificatesPerFQDNSetPerAccount:12345678:example.com,example.net,example.org
// Load multiple valid override limits with 'fqdnSet' Ids, as follows:
// - CertificatesPerFQDNSet:example.com
// - CertificatesPerFQDNSet:example.com,example.net
// - CertificatesPerFQDNSet:example.com,example.net,example.org
firstEntryFQDNSetHash := string(core.HashNames([]string{"example.com"}))
secondEntryFQDNSetHash := string(core.HashNames([]string{"example.com", "example.net"}))
thirdEntryFQDNSetHash := string(core.HashNames([]string{"example.com", "example.net", "example.org"}))
firstEntryKey := nameToEnumString(CertificatesPerFQDNSetPerAccount) + ":" + "12345678:" + firstEntryFQDNSetHash
secondEntryKey := nameToEnumString(CertificatesPerFQDNSetPerAccount) + ":" + "12345678:" + secondEntryFQDNSetHash
thirdEntryKey := nameToEnumString(CertificatesPerFQDNSetPerAccount) + ":" + "12345678:" + thirdEntryFQDNSetHash
firstEntryKey := joinWithColon(CertificatesPerFQDNSet.EnumString(), firstEntryFQDNSetHash)
secondEntryKey := joinWithColon(CertificatesPerFQDNSet.EnumString(), secondEntryFQDNSetHash)
thirdEntryKey := joinWithColon(CertificatesPerFQDNSet.EnumString(), thirdEntryFQDNSetHash)
l, err = loadAndParseOverrideLimits("testdata/working_overrides_regid_fqdnset.yml")
test.AssertNotError(t, err, "multiple valid override limits with Id of regId:fqdnSets")
test.AssertNotError(t, err, "multiple valid override limits with 'fqdnSet' Ids")
test.AssertEquals(t, l[firstEntryKey].Burst, int64(40))
test.AssertEquals(t, l[firstEntryKey].Count, int64(40))
test.AssertEquals(t, l[firstEntryKey].Period.Duration, time.Second)
Expand Down Expand Up @@ -278,25 +227,22 @@ func Test_loadAndParseOverrideLimits(t *testing.T) {
}

func Test_loadAndParseDefaultLimits(t *testing.T) {
newRestistrationsPerIPv4AddressEnumStr := nameToEnumString(NewRegistrationsPerIPAddress)
newRegistrationsPerIPv6RangeEnumStr := nameToEnumString(NewRegistrationsPerIPv6Range)

// Load a single valid default limit.
l, err := loadAndParseDefaultLimits("testdata/working_default.yml")
test.AssertNotError(t, err, "valid single default limit")
test.AssertEquals(t, l[newRestistrationsPerIPv4AddressEnumStr].Burst, int64(20))
test.AssertEquals(t, l[newRestistrationsPerIPv4AddressEnumStr].Count, int64(20))
test.AssertEquals(t, l[newRestistrationsPerIPv4AddressEnumStr].Period.Duration, time.Second)
test.AssertEquals(t, l[NewRegistrationsPerIPAddress.EnumString()].Burst, int64(20))
test.AssertEquals(t, l[NewRegistrationsPerIPAddress.EnumString()].Count, int64(20))
test.AssertEquals(t, l[NewRegistrationsPerIPAddress.EnumString()].Period.Duration, time.Second)

// Load multiple valid default limits.
l, err = loadAndParseDefaultLimits("testdata/working_defaults.yml")
test.AssertNotError(t, err, "multiple valid default limits")
test.AssertEquals(t, l[newRestistrationsPerIPv4AddressEnumStr].Burst, int64(20))
test.AssertEquals(t, l[newRestistrationsPerIPv4AddressEnumStr].Count, int64(20))
test.AssertEquals(t, l[newRestistrationsPerIPv4AddressEnumStr].Period.Duration, time.Second)
test.AssertEquals(t, l[newRegistrationsPerIPv6RangeEnumStr].Burst, int64(30))
test.AssertEquals(t, l[newRegistrationsPerIPv6RangeEnumStr].Count, int64(30))
test.AssertEquals(t, l[newRegistrationsPerIPv6RangeEnumStr].Period.Duration, time.Second*2)
test.AssertEquals(t, l[NewRegistrationsPerIPAddress.EnumString()].Burst, int64(20))
test.AssertEquals(t, l[NewRegistrationsPerIPAddress.EnumString()].Count, int64(20))
test.AssertEquals(t, l[NewRegistrationsPerIPAddress.EnumString()].Period.Duration, time.Second)
test.AssertEquals(t, l[NewRegistrationsPerIPv6Range.EnumString()].Burst, int64(30))
test.AssertEquals(t, l[NewRegistrationsPerIPv6Range.EnumString()].Count, int64(30))
test.AssertEquals(t, l[NewRegistrationsPerIPv6Range.EnumString()].Period.Duration, time.Second*2)

// Path is empty string.
_, err = loadAndParseDefaultLimits("")
Expand Down
Loading