ratelimits: Support new Comment field for each Id entry (#7480)

Fixes #7478
letsencrypt · May 13, 2024 · 16d55ef · 16d55ef
1 parent 10b7e63
commit 16d55ef
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 36 deletions.
diff --git a/ratelimits/limit.go b/ratelimits/limit.go
@@ -87,7 +87,12 @@ func loadDefaults(path string) (limits, error) {
 type overrideYAML struct {
 	limit `yaml:",inline"`
 	// Ids is a list of ids that this override applies to.
-	Ids []string
+	Ids []struct {
+		Id string `yaml:"id"`
+		// Comment is an optional field that can be used to provide additional
+		// context for the override.
+		Comment string `yaml:"comment,omitempty"`
+	} `yaml:"ids"`
 }
 
 type overridesYAML []map[string]overrideYAML
@@ -153,7 +158,8 @@ func loadAndParseOverrideLimits(path string) (limits, error) {
 			}
 			v.limit.name = name
 			v.limit.isOverride = true
-			for _, id := range v.Ids {
+			for _, entry := range v.Ids {
+				id := entry.Id
 				err = validateIdForName(name, id)
 				if err != nil {
 					return nil, fmt.Errorf(

diff --git a/ratelimits/testdata/busted_override_burst_0.yml b/ratelimits/testdata/busted_override_burst_0.yml
@@ -2,4 +2,6 @@
     burst: 0
     count: 40
     period: 1s
-    ids: [10.0.0.2]
+    ids:
+      - id: 10.0.0.2
+        comment: Foo
diff --git a/ratelimits/testdata/busted_override_empty_name.yml b/ratelimits/testdata/busted_override_empty_name.yml
@@ -2,4 +2,6 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [10.0.0.2]
+    ids:
+      - id: 10.0.0.2
+        comment: Foo
diff --git a/ratelimits/testdata/busted_override_invalid_name.yml b/ratelimits/testdata/busted_override_invalid_name.yml
@@ -2,4 +2,6 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [10.0.0.2]
+    ids:
+      - id: 10.0.0.2
+        comment: Foo
diff --git a/ratelimits/testdata/busted_overrides_second_entry_bad_name.yml b/ratelimits/testdata/busted_overrides_second_entry_bad_name.yml
@@ -2,9 +2,13 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [10.0.0.2]
+    ids:
+      - id: 10.0.0.2
+        comment: Foo
 - UsageRequestsPerIPv10Address:
     burst: 40
     count: 40
     period: 1s
-    ids: [10.0.0.5]
+    ids:
+      - id: 10.0.0.5
+        comment: Bar
diff --git a/ratelimits/testdata/busted_overrides_third_entry_bad_id.yml b/ratelimits/testdata/busted_overrides_third_entry_bad_id.yml
@@ -2,4 +2,10 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [10.0.0.5, 10.0.0.2, lol]
+    ids:
+      - id: 10.0.0.5
+        comment: Foo
+      - id: 10.0.0.2
+        comment: Bar
+      - id: lol
+        comment: Baz
diff --git a/ratelimits/testdata/working_override.yml b/ratelimits/testdata/working_override.yml
@@ -2,4 +2,6 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [10.0.0.2]
+    ids:
+      - id: 10.0.0.2
+        comment: Foo
diff --git a/ratelimits/testdata/working_override_regid_domain.yml b/ratelimits/testdata/working_override_regid_domain.yml
@@ -2,4 +2,6 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [example.com]
+    ids:
+      - id: example.com
+        comment: Foo
diff --git a/ratelimits/testdata/working_overrides.yml b/ratelimits/testdata/working_overrides.yml
@@ -2,15 +2,23 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [10.0.0.2]
+    ids:
+      - id: 10.0.0.2
+        comment: Foo
 - NewRegistrationsPerIPv6Range:
     burst: 50
     count: 50
     period: 2s
-    ids: [2001:0db8:0000::/48]
+    ids:
+      - id: 2001:0db8:0000::/48
+        comment: Foo
 - FailedAuthorizationsPerDomainPerAccount:
     burst: 60
     count: 60
     period: 3s
-    ids: [1234, 5678]
+    ids:
+      - id: 1234
+        comment: Foo
+      - id: 5678
+        comment: Foo
 
diff --git a/ratelimits/testdata/working_overrides_regid_fqdnset.yml b/ratelimits/testdata/working_overrides_regid_fqdnset.yml
@@ -2,14 +2,20 @@
     burst: 40
     count: 40
     period: 1s
-    ids: [example.com]
+    ids:
+      - id: example.com
+        comment: Foo
 - CertificatesPerFQDNSet:
     burst: 50
     count: 50
     period: 2s
-    ids: ["example.com,example.net"]
+    ids:
+      - id: "example.com,example.net"
+        comment: Foo
 - CertificatesPerFQDNSet:
     burst: 60
     count: 60
     period: 3s
-    ids: ["example.com,example.net,example.org"]
+    ids:
+      - id: "example.com,example.net,example.org"
+        comment: Foo
diff --git a/test/config-next/wfe2-ratelimit-overrides.yml b/test/config-next/wfe2-ratelimit-overrides.yml
@@ -3,38 +3,58 @@
     count: 1000000
     period: 168h
     ids:
-      - 127.0.0.1
+      - id: 127.0.0.1
+        comment: localhost
 - CertificatesPerDomain:
     burst: 1
     count: 1
     period: 2160h
     ids:
-      - ratelimit.me
+      - id: ratelimit.me
+        comment: Rate Limit Test Domain
 - CertificatesPerDomain:
     burst: 10000
     count: 10000
     period: 2160h
     ids:
-      - le.wtf
-      - le1.wtf
-      - le2.wtf
-      - le3.wtf
-      - nginx.wtf
-      - good-caa-reserved.com
-      - bad-caa-reserved.com
-      - ecdsa.le.wtf
-      - must-staple.le.wtf
+      - id: le.wtf
+        comment: Let's Encrypt Test Domain
+      - id: le1.wtf
+        comment: Let's Encrypt Test Domain 1
+      - id: le2.wtf
+        comment: Let's Encrypt Test Domain 2
+      - id: le3.wtf
+        comment: Let's Encrypt Test Domain 3
+      - id: nginx.wtf
+        comment: Nginx Test Domain
+      - id: good-caa-reserved.com
+        comment: Good CAA Reserved Domain
+      - id: bad-caa-reserved.com
+        comment: Bad CAA Reserved Domain
+      - id: ecdsa.le.wtf
+        comment: ECDSA Let's Encrypt Test Domain
+      - id: must-staple.le.wtf
+        comment: Must-Staple Let's Encrypt Test Domain
 - CertificatesPerFQDNSet:
     burst: 10000
     count: 10000
     period: 168h
     ids:
-      - le.wtf
-      - le1.wtf
-      - le2.wtf
-      - le3.wtf
-      - le.wtf,le1.wtf
-      - good-caa-reserved.com
-      - nginx.wtf
-      - ecdsa.le.wtf
-      - must-staple.le.wtf
+      - id: le.wtf
+        comment: Let's Encrypt Test Domain
+      - id: le1.wtf
+        comment: Let's Encrypt Test Domain 1
+      - id: le2.wtf
+        comment: Let's Encrypt Test Domain 2
+      - id: le3.wtf
+        comment: Let's Encrypt Test Domain 3
+      - id: le.wtf,le1.wtf
+        comment: Let's Encrypt Test Domain, Let's Encrypt Test Domain 1
+      - id: good-caa-reserved.com
+        comment: Good CAA Reserved Domain
+      - id: nginx.wtf
+        comment: Nginx Test Domain
+      - id: ecdsa.le.wtf
+        comment: ECDSA Let's Encrypt Test Domain
+      - id: must-staple.le.wtf
+        comment: Must-Staple Let's Encrypt Test Domain