X Tutup
Skip to content

Commit d1d04c9

Browse files
beautifulentropybeautifulentropy
authored
GRCP: Replace CountByNames_MapElement with a real proto map (letsencrypt#5621)
Fixes letsencrypt#5614
1 parent 8f4c105 commit d1d04c9

File tree

6 files changed

+392
-479
lines changed

6 files changed

+392
-479
lines changed

‎ra/ra.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,14 +1398,14 @@ func (ra *RegistrationAuthorityImpl) enforceNameCounts(ctx context.Context, name
13981398
return nil, err
13991399
}
14001400

1401-
if len(response.CountByNames) == 0 {
1401+
if len(response.Counts) == 0 {
14021402
return nil, errIncompleteGRPCResponse
14031403
}
14041404

14051405
var badNames []string
1406-
for _, entry := range response.CountByNames {
1407-
if entry.Count >= limit.GetThreshold(entry.Name, regID) {
1408-
badNames = append(badNames, entry.Name)
1406+
for name, count := range response.Counts {
1407+
if count >= limit.GetThreshold(name, regID) {
1408+
badNames = append(badNames, name)
14091409
}
14101410
}
14111411
return badNames, nil

‎ra/ra_test.go‎

Lines changed: 43 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,7 +1446,7 @@ func TestRateLimitLiveReload(t *testing.T) {
14461446

14471447
type mockSAWithNameCounts struct {
14481448
mocks.StorageAuthority
1449-
nameCounts map[string]*sapb.CountByNames_MapElement
1449+
nameCounts *sapb.CountByNames
14501450
t *testing.T
14511451
clk clock.FakeClock
14521452
}
@@ -1460,21 +1460,13 @@ func (m mockSAWithNameCounts) CountCertificatesByNames(ctx context.Context, req
14601460
if req.Range.Earliest != expectedEarliest {
14611461
m.t.Errorf("incorrect earliest: got '%d', expected '%d'", req.Range.Earliest, expectedEarliest)
14621462
}
1463-
var results []*sapb.CountByNames_MapElement
1463+
counts := make(map[string]int64)
14641464
for _, name := range req.Names {
1465-
if entry, ok := m.nameCounts[name]; ok {
1466-
results = append(results, entry)
1465+
if count, ok := m.nameCounts.Counts[name]; ok {
1466+
counts[name] = count
14671467
}
14681468
}
1469-
return &sapb.CountByNames{CountByNames: results}, nil
1470-
}
1471-
1472-
func nameCount(domain string, count int) *sapb.CountByNames_MapElement {
1473-
pbInt := int64(count)
1474-
return &sapb.CountByNames_MapElement{
1475-
Name: domain,
1476-
Count: pbInt,
1477-
}
1469+
return &sapb.CountByNames{Counts: counts}, nil
14781470
}
14791471

14801472
func TestCheckCertificatesPerNameLimit(t *testing.T) {
@@ -1491,11 +1483,9 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
14911483
}
14921484

14931485
mockSA := &mockSAWithNameCounts{
1494-
nameCounts: map[string]*sapb.CountByNames_MapElement{
1495-
"example.com": nameCount("example.com", 1),
1496-
},
1497-
clk: fc,
1498-
t: t,
1486+
nameCounts: &sapb.CountByNames{Counts: map[string]int64{"example.com": 1}},
1487+
clk: fc,
1488+
t: t,
14991489
}
15001490

15011491
ra.SA = mockSA
@@ -1505,8 +1495,8 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
15051495
test.AssertNotError(t, err, "rate limited example.com incorrectly")
15061496

15071497
// Two base domains, one above threshold, one below
1508-
mockSA.nameCounts["example.com"] = nameCount("example.com", 10)
1509-
mockSA.nameCounts["good-example.com"] = nameCount("good-example.com", 1)
1498+
mockSA.nameCounts.Counts["example.com"] = 10
1499+
mockSA.nameCounts.Counts["good-example.com"] = 1
15101500
err = ra.checkCertificatesPerNameLimit(ctx, []string{"www.example.com", "example.com", "good-example.com"}, rlp, 99)
15111501
test.AssertError(t, err, "incorrectly failed to rate limit example.com")
15121502
test.AssertErrorIs(t, err, berrors.RateLimit)
@@ -1517,9 +1507,9 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
15171507
test.AssertEquals(t, len(bErr.SubErrors), 0)
15181508

15191509
// Three base domains, two above threshold, one below
1520-
mockSA.nameCounts["example.com"] = nameCount("example.com", 10)
1521-
mockSA.nameCounts["other-example.com"] = nameCount("other-example.com", 10)
1522-
mockSA.nameCounts["good-example.com"] = nameCount("good-example.com", 1)
1510+
mockSA.nameCounts.Counts["example.com"] = 10
1511+
mockSA.nameCounts.Counts["other-example.com"] = 10
1512+
mockSA.nameCounts.Counts["good-example.com"] = 1
15231513
err = ra.checkCertificatesPerNameLimit(ctx, []string{"example.com", "other-example.com", "good-example.com"}, rlp, 99)
15241514
test.AssertError(t, err, "incorrectly failed to rate limit example.com, other-example.com")
15251515
test.AssertErrorIs(t, err, berrors.RateLimit)
@@ -1533,20 +1523,20 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) {
15331523
test.AssertError(t, err, "incorrectly failed to error on misbehaving SA")
15341524

15351525
// Two base domains, one above threshold but with an override.
1536-
mockSA.nameCounts["example.com"] = nameCount("example.com", 0)
1537-
mockSA.nameCounts["bigissuer.com"] = nameCount("bigissuer.com", 50)
1526+
mockSA.nameCounts.Counts["example.com"] = 0
1527+
mockSA.nameCounts.Counts["bigissuer.com"] = 50
15381528
err = ra.checkCertificatesPerNameLimit(ctx, []string{"www.example.com", "subdomain.bigissuer.com"}, rlp, 99)
15391529
test.AssertNotError(t, err, "incorrectly rate limited bigissuer")
15401530

15411531
// Two base domains, one above its override
1542-
mockSA.nameCounts["example.com"] = nameCount("example.com", 0)
1543-
mockSA.nameCounts["bigissuer.com"] = nameCount("bigissuer.com", 100)
1532+
mockSA.nameCounts.Counts["example.com"] = 10
1533+
mockSA.nameCounts.Counts["bigissuer.com"] = 100
15441534
err = ra.checkCertificatesPerNameLimit(ctx, []string{"www.example.com", "subdomain.bigissuer.com"}, rlp, 99)
15451535
test.AssertError(t, err, "incorrectly failed to rate limit bigissuer")
15461536
test.AssertErrorIs(t, err, berrors.RateLimit)
15471537

15481538
// One base domain, above its override (which is below threshold)
1549-
mockSA.nameCounts["smallissuer.co.uk"] = nameCount("smallissuer.co.uk", 1)
1539+
mockSA.nameCounts.Counts["smallissuer.co.uk"] = 1
15501540
err = ra.checkCertificatesPerNameLimit(ctx, []string{"www.smallissuer.co.uk"}, rlp, 99)
15511541
test.AssertError(t, err, "incorrectly failed to rate limit smallissuer")
15521542
test.AssertErrorIs(t, err, berrors.RateLimit)
@@ -1568,10 +1558,12 @@ func TestCheckExactCertificateLimit(t *testing.T) {
15681558
// Create a mock SA that has a count of already issued certificates for some
15691559
// test names
15701560
mockSA := &mockSAWithFQDNSet{
1571-
nameCounts: map[string]*sapb.CountByNames_MapElement{
1572-
"under.example.com": nameCount("under.example.com", dupeCertLimit-1),
1573-
"equal.example.com": nameCount("equal.example.com", dupeCertLimit),
1574-
"over.example.com": nameCount("over.example.com", dupeCertLimit+1),
1561+
nameCounts: &sapb.CountByNames{
1562+
Counts: map[string]int64{
1563+
"under.example.com": dupeCertLimit - 1,
1564+
"equal.example.com": dupeCertLimit,
1565+
"over.example.com": dupeCertLimit + 1,
1566+
},
15751567
},
15761568
t: t,
15771569
}
@@ -1709,7 +1701,7 @@ func TestRegistrationKeyUpdate(t *testing.T) {
17091701
type mockSAWithFQDNSet struct {
17101702
mocks.StorageAuthority
17111703
fqdnSet map[string]bool
1712-
nameCounts map[string]*sapb.CountByNames_MapElement
1704+
nameCounts *sapb.CountByNames
17131705
t *testing.T
17141706
}
17151707

@@ -1738,23 +1730,23 @@ func (m mockSAWithFQDNSet) FQDNSetExists(_ context.Context, req *sapb.FQDNSetExi
17381730

17391731
// Return a map of domain -> certificate count.
17401732
func (m mockSAWithFQDNSet) CountCertificatesByNames(ctx context.Context, req *sapb.CountCertificatesByNamesRequest) (*sapb.CountByNames, error) {
1741-
var results []*sapb.CountByNames_MapElement
1733+
counts := make(map[string]int64)
17421734
for _, name := range req.Names {
1743-
if entry, ok := m.nameCounts[name]; ok {
1744-
results = append(results, entry)
1735+
if count, ok := m.nameCounts.Counts[name]; ok {
1736+
counts[name] = count
17451737
}
17461738
}
1747-
return &sapb.CountByNames{CountByNames: results}, nil
1739+
return &sapb.CountByNames{Counts: counts}, nil
17481740
}
17491741

17501742
func (m mockSAWithFQDNSet) CountFQDNSets(_ context.Context, req *sapb.CountFQDNSetsRequest) (*sapb.Count, error) {
1751-
var count int64
1743+
var total int64
17521744
for _, name := range req.Domains {
1753-
if entry, ok := m.nameCounts[name]; ok {
1754-
count += entry.Count
1745+
if count, ok := m.nameCounts.Counts[name]; ok {
1746+
total += count
17551747
}
17561748
}
1757-
return &sapb.Count{Count: count}, nil
1749+
return &sapb.Count{Count: total}, nil
17581750
}
17591751

17601752
// Tests for boulder issue 1925[0] - that the `checkCertificatesPerNameLimit`
@@ -1776,9 +1768,8 @@ func TestCheckFQDNSetRateLimitOverride(t *testing.T) {
17761768

17771769
// Create a mock SA that has both name counts and an FQDN set
17781770
mockSA := &mockSAWithFQDNSet{
1779-
nameCounts: map[string]*sapb.CountByNames_MapElement{
1780-
"example.com": nameCount("example.com", 100),
1781-
"zombo.com": nameCount("zombo.com", 100),
1771+
nameCounts: &sapb.CountByNames{
1772+
Counts: map[string]int64{"example.com": 100, "zombo.com": 100},
17821773
},
17831774
fqdnSet: map[string]bool{},
17841775
t: t,
@@ -1828,12 +1819,14 @@ func TestExactPublicSuffixCertLimit(t *testing.T) {
18281819
// - test2.dedyn.io (once)
18291820
// - dynv6.net (twice)
18301821
mockSA := &mockSAWithNameCounts{
1831-
nameCounts: map[string]*sapb.CountByNames_MapElement{
1832-
"test.dedyn.io": nameCount("test.dedyn.io", 1),
1833-
"test2.dedyn.io": nameCount("test2.dedyn.io", 1),
1834-
"test3.dedyn.io": nameCount("test3.dedyn.io", 0),
1835-
"dedyn.io": nameCount("dedyn.io", 0),
1836-
"dynv6.net": nameCount("dynv6.net", 2),
1822+
nameCounts: &sapb.CountByNames{
1823+
Counts: map[string]int64{
1824+
"test.dedyn.io": 1,
1825+
"test2.dedyn.io": 1,
1826+
"test3.dedyn.io": 0,
1827+
"dedyn.io": 0,
1828+
"dynv6.net": 2,
1829+
},
18371830
},
18381831
clk: fc,
18391832
t: t,

0 commit comments

Comments
 (0)
X Tutup