ratelimits: More compact overrides format (#7199)

Support a more compact format for supplying overrides to default rate
limits.

Fixes #7197

ratelimits/limit.go

@@ -70,8 +70,8 @@ func validateLimit(l limit) error {
 
 type limits map[string]limit
 
-// loadLimits marshals the YAML file at path into a map of limis.
-func loadLimits(path string) (limits, error) {
+// loadDefaults marshals the defaults YAML file at path into a map of limits.
+func loadDefaults(path string) (limits, error) {
 	lm := make(limits)
 	data, err := os.ReadFile(path)
 	if err != nil {
@@ -84,6 +84,28 @@ func loadLimits(path string) (limits, error) {
 	return lm, nil
 }
 
+type overrideYAML struct {
+	limit `yaml:",inline"`
+	// Ids is a list of ids that this override applies to.
+	Ids []string
+}
+
+type overridesYAML []map[string]overrideYAML
+
+// loadOverrides marshals the YAML file at path into a map of overrides.
+func loadOverrides(path string) (overridesYAML, error) {
+	ov := overridesYAML{}
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	err = strictyaml.Unmarshal(data, &ov)
+	if err != nil {
+		return nil, err
+	}
+	return ov, nil
+}
+
 // parseOverrideNameId is broken out for ease of testing.
 func parseOverrideNameId(key string) (Name, string, error) {
 	if !strings.Contains(key, ":") {
@@ -107,10 +129,12 @@ func parseOverrideNameId(key string) (Name, string, error) {
 	return name, id, nil
 }
 
-// loadAndParseOverrideLimits loads override limits from YAML, validates them,
-// and parses them into a map of limits keyed by 'Name:id'.
-func loadAndParseOverrideLimits(path string) (limits, error) {
-	fromFile, err := loadLimits(path)
+// loadAndParseOverrideLimitsDeprecated loads override limits from YAML,
+// validates them, and parses them into a map of limits keyed by 'Name:id'.
+//
+// TODO(#7198): Remove this.
+func loadAndParseOverrideLimitsDeprecated(path string) (limits, error) {
+	fromFile, err := loadDefaults(path)
 	if err != nil {
 		return nil, err
 	}
@@ -143,10 +167,53 @@ func loadAndParseOverrideLimits(path string) (limits, error) {
 	return parsed, nil
 }
 
+// loadAndParseOverrideLimits loads override limits from YAML. The YAML file
+// must be formatted as a list of maps, where each map has a single key
+// representing the limit name and a value that is a map containing the limit
+// fields and an additional 'ids' field that is a list of ids that this override
+// applies to.
+func loadAndParseOverrideLimits(path string) (limits, error) {
+	fromFile, err := loadOverrides(path)
+	if err != nil {
+		return nil, err
+	}
+	parsed := make(limits)
+
+	for _, ov := range fromFile {
+		for k, v := range ov {
+			err = validateLimit(v.limit)
+			if err != nil {
+				return nil, fmt.Errorf("validating override limit %q: %w", k, err)
+			}
+			name, ok := stringToName[k]
+			if !ok {
+				return nil, fmt.Errorf("unrecognized name %q in override limit, must be one of %v", k, limitNames)
+			}
+			v.limit.name = name
+			v.limit.isOverride = true
+			for _, id := range v.Ids {
+				err = validateIdForName(name, id)
+				if err != nil {
+					return nil, fmt.Errorf(
+						"validating name %s and id %q for override limit %q: %w", name, id, k, err)
+				}
+				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.
+					id = string(core.HashNames(strings.Split(id, ",")))
+				}
+				parsed[joinWithColon(name.EnumString(), id)] = precomputeLimit(v.limit)
+			}
+		}
+	}
+	return parsed, nil
+}
+
 // loadAndParseDefaultLimits loads default limits from YAML, validates them, and
 // parses them into a map of limits keyed by 'Name'.
 func loadAndParseDefaultLimits(path string) (limits, error) {
-	fromFile, err := loadLimits(path)
+	fromFile, err := loadDefaults(path)
 	if err != nil {
 		return nil, err
 	}
@@ -189,9 +256,13 @@ func newLimitRegistry(defaults, overrides string) (*limitRegistry, error) {
 		return registry, nil
 	}
 
-	registry.overrides, err = loadAndParseOverrideLimits(overrides)
+	registry.overrides, err = loadAndParseOverrideLimitsDeprecated(overrides)
 	if err != nil {
-		return nil, err
+		// TODO(#7198): Leave this, remove the call above.
+		registry.overrides, err = loadAndParseOverrideLimits(overrides)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	return registry, nil

ratelimits/limit_test.go

@@ -134,6 +134,99 @@ func TestValidateIdForName(t *testing.T) {
 	test.AssertError(t, err, "valid regId with empty domain")
 }
 
+// TODO(#7198): Remove this.
+func TestLoadAndParseOverrideLimitsDeprecated(t *testing.T) {
+	// Load a single valid override limit with Id formatted as 'enum:RegId'.
+	l, err := loadAndParseOverrideLimitsDeprecated("testdata/working_override_deprecated.yml")
+	test.AssertNotError(t, err, "valid single override limit")
+	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 = loadAndParseOverrideLimitsDeprecated("testdata/working_override_regid_domain_deprecated.yml")
+	test.AssertNotError(t, err, "valid single override limit with Id of regId:domain")
+	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 = loadAndParseOverrideLimitsDeprecated("testdata/working_overrides_deprecated.yml")
+	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 := 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 '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 := joinWithColon(CertificatesPerFQDNSet.EnumString(), firstEntryFQDNSetHash)
+	secondEntryKey := joinWithColon(CertificatesPerFQDNSet.EnumString(), secondEntryFQDNSetHash)
+	thirdEntryKey := joinWithColon(CertificatesPerFQDNSet.EnumString(), thirdEntryFQDNSetHash)
+	l, err = loadAndParseOverrideLimitsDeprecated("testdata/working_overrides_regid_fqdnset_deprecated.yml")
+	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)
+	test.AssertEquals(t, l[secondEntryKey].Burst, int64(50))
+	test.AssertEquals(t, l[secondEntryKey].Count, int64(50))
+	test.AssertEquals(t, l[secondEntryKey].Period.Duration, time.Second*2)
+	test.AssertEquals(t, l[thirdEntryKey].Burst, int64(60))
+	test.AssertEquals(t, l[thirdEntryKey].Count, int64(60))
+	test.AssertEquals(t, l[thirdEntryKey].Period.Duration, time.Second*3)
+
+	// Path is empty string.
+	_, err = loadAndParseOverrideLimitsDeprecated("")
+	test.AssertError(t, err, "path is empty string")
+	test.Assert(t, os.IsNotExist(err), "path is empty string")
+
+	// Path to file which does not exist.
+	_, err = loadAndParseOverrideLimitsDeprecated("testdata/file_does_not_exist_deprecated.yml")
+	test.AssertError(t, err, "a file that does not exist ")
+	test.Assert(t, os.IsNotExist(err), "test file should not exist")
+
+	// Burst cannot be 0.
+	_, err = loadAndParseOverrideLimitsDeprecated("testdata/busted_override_burst_0_deprecated.yml")
+	test.AssertError(t, err, "single override limit with burst=0")
+	test.Assert(t, !os.IsNotExist(err), "test file should exist")
+
+	// Id cannot be empty.
+	_, err = loadAndParseOverrideLimitsDeprecated("testdata/busted_override_empty_id_deprecated.yml")
+	test.AssertError(t, err, "single override limit with empty id")
+	test.Assert(t, !os.IsNotExist(err), "test file should exist")
+
+	// Name cannot be empty.
+	_, err = loadAndParseOverrideLimitsDeprecated("testdata/busted_override_empty_name_deprecated.yml")
+	test.AssertError(t, err, "single override limit with empty name")
+	test.Assert(t, !os.IsNotExist(err), "test file should exist")
+
+	// Name must be a string representation of a valid Name enumeration.
+	_, err = loadAndParseOverrideLimitsDeprecated("testdata/busted_override_invalid_name_deprecated.yml")
+	test.AssertError(t, err, "single override limit with invalid name")
+	test.Assert(t, !os.IsNotExist(err), "test file should exist")
+
+	// Multiple entries, second entry has a bad name.
+	_, err = loadAndParseOverrideLimitsDeprecated("testdata/busted_overrides_second_entry_bad_name_deprecated.yml")
+	test.AssertError(t, err, "multiple override limits, second entry is bad")
+	test.Assert(t, !os.IsNotExist(err), "test file should exist")
+
+	// Multiple entries, third entry has id of "lol", instead of an IPv4 address.
+	_, err = loadAndParseOverrideLimitsDeprecated("testdata/busted_overrides_third_entry_bad_id_deprecated.yml")
+	test.AssertError(t, err, "multiple override limits, third entry has bad Id value")
+	test.Assert(t, !os.IsNotExist(err), "test file should exist")
+}
+
 func TestLoadAndParseOverrideLimits(t *testing.T) {
 	// Load a single valid override limit with Id formatted as 'enum:RegId'.
 	l, err := loadAndParseOverrideLimits("testdata/working_override.yml")
@@ -143,18 +236,18 @@ func TestLoadAndParseOverrideLimits(t *testing.T) {
 	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'.
+	// Load single valid override limit with a 'domain' Id.
 	l, err = loadAndParseOverrideLimits("testdata/working_override_regid_domain.yml")
 	test.AssertNotError(t, err, "valid single override limit with Id of regId:domain")
 	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.
+	// Load multiple valid override limits with 'regId' Ids.
 	l, err = loadAndParseOverrideLimits("testdata/working_overrides.yml")
-	expectKey1 := joinWithColon(NewRegistrationsPerIPAddress.EnumString(), "10.0.0.2")
 	test.AssertNotError(t, err, "multiple valid override limits")
+	expectKey1 := joinWithColon(NewRegistrationsPerIPAddress.EnumString(), "10.0.0.2")
 	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)

ratelimits/testdata/busted_override_burst_0.yml

@@ -1,4 +1,5 @@
-NewRegistrationsPerIPAddress:10.0.0.2:
-  burst: 0
-  count: 40
-  period: 1s
+- NewRegistrationsPerIPAddress:
+    burst: 0
+    count: 40
+    period: 1s
+    ids: [10.0.0.2]

ratelimits/testdata/busted_override_burst_0_deprecated.yml

@@ -0,0 +1,4 @@
+NewRegistrationsPerIPAddress:10.0.0.2:
+  burst: 0
+  count: 40
+  period: 1s

ratelimits/testdata/busted_override_empty_id.yml

@@ -1,4 +1,5 @@
-"UsageRequestsPerIPv10Address:":
-  burst: 40
-  count: 40
-  period: 1s
+- UsageRequestsPerIPv10Address:
+    burst: 40
+    count: 40
+    period: 1s
+    ids: []

ratelimits/testdata/busted_override_empty_id_deprecated.yml

@@ -0,0 +1,4 @@
+"UsageRequestsPerIPv10Address:":
+  burst: 40
+  count: 40
+  period: 1s

ratelimits/testdata/busted_override_empty_name.yml

@@ -1,4 +1,5 @@
-":10.0.0.2":
-  burst: 40
-  count: 40
-  period: 1s
+- "":
+    burst: 40
+    count: 40
+    period: 1s
+    ids: [10.0.0.2]

ratelimits/testdata/busted_override_empty_name_deprecated.yml

@@ -0,0 +1,4 @@
+":10.0.0.2":
+  burst: 40
+  count: 40
+  period: 1s

ratelimits/testdata/busted_override_invalid_name.yml

@@ -1,4 +1,5 @@
-UsageRequestsPerIPv10Address:10.0.0.2:
-  burst: 40
-  count: 40
-  period: 1s
+- UsageRequestsPerIPv10Address:
+    burst: 40
+    count: 40
+    period: 1s
+    ids: [10.0.0.2]

ratelimits/testdata/busted_override_invalid_name_deprecated.yml

@@ -0,0 +1,4 @@
+UsageRequestsPerIPv10Address:10.0.0.2:
+  burst: 40
+  count: 40
+  period: 1s

ratelimits/testdata/busted_overrides_second_entry_bad_name.yml

@@ -1,8 +1,10 @@
-NewRegistrationsPerIPAddress:10.0.0.2:
-  burst: 40
-  count: 40
-  period: 1s
-UsageRequestsPerIPv10Address:10.0.0.5:
-  burst: 40
-  count: 40
-  period: 1s
+- NewRegistrationsPerIPAddress:
+    burst: 40
+    count: 40
+    period: 1s
+    ids: [10.0.0.2]
+- UsageRequestsPerIPv10Address:
+    burst: 40
+    count: 40
+    period: 1s
+    ids: [10.0.0.5]

ratelimits/testdata/busted_overrides_second_entry_bad_name_deprecated.yml

@@ -0,0 +1,8 @@
+NewRegistrationsPerIPAddress:10.0.0.2:
+  burst: 40
+  count: 40
+  period: 1s
+UsageRequestsPerIPv10Address:10.0.0.5:
+  burst: 40
+  count: 40
+  period: 1s

ratelimits/testdata/busted_overrides_third_entry_bad_id.yml

@@ -1,12 +1,5 @@
-NewRegistrationsPerIPAddress:10.0.0.2:
-  burst: 40
-  count: 40
-  period: 1s
-NewRegistrationsPerIPAddress:10.0.0.5:
-  burst: 40
-  count: 40
-  period: 1s
-NewRegistrationsPerIPAddress:lol:
-  burst: 40
-  count: 40
-  period: 1s
+- NewRegistrationsPerIPAddress:
+    burst: 40
+    count: 40
+    period: 1s
+    ids: [10.0.0.5, 10.0.0.2, lol]

ratelimits/testdata/busted_overrides_third_entry_bad_id_deprecated.yml

@@ -0,0 +1,12 @@
+NewRegistrationsPerIPAddress:10.0.0.2:
+  burst: 40
+  count: 40
+  period: 1s
+NewRegistrationsPerIPAddress:10.0.0.5:
+  burst: 40
+  count: 40
+  period: 1s
+NewRegistrationsPerIPAddress:lol:
+  burst: 40
+  count: 40
+  period: 1s

ratelimits/testdata/working_override.yml

@@ -1,4 +1,5 @@
-NewRegistrationsPerIPAddress:10.0.0.2:
-  burst: 40
-  count: 40
-  period: 1s
+- NewRegistrationsPerIPAddress:
+    burst: 40
+    count: 40
+    period: 1s
+    ids: [10.0.0.2]

ratelimits/testdata/working_override_deprecated.yml

@@ -0,0 +1,4 @@
+NewRegistrationsPerIPAddress:10.0.0.2:
+  burst: 40
+  count: 40
+  period: 1s

ratelimits/testdata/working_override_regid_domain.yml

@@ -1,4 +1,5 @@
-CertificatesPerDomain:example.com:
-  burst: 40
-  count: 40
-  period: 1s
+- CertificatesPerDomain:
+    burst: 40
+    count: 40
+    period: 1s
+    ids: [example.com]

ratelimits/testdata/working_override_regid_domain_deprecated.yml

@@ -0,0 +1,4 @@
+CertificatesPerDomain:example.com:
+  burst: 40
+  count: 40
+  period: 1s