From 6bbc6bd92050946b3eb50a52fde52c41e14f8770 Mon Sep 17 00:00:00 2001 From: Samantha Date: Mon, 6 Nov 2023 11:57:14 -0500 Subject: [PATCH] Count impending issuance towards utilization. --- ra/ra.go | 46 ++++++++++++++++++++++------------------------ ra/ra_test.go | 7 ++++--- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/ra/ra.go b/ra/ra.go index 4cf7af3b82ef..90195d2d38a8 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -394,16 +394,15 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationIPLimit(ctx context.Contex } threshold, overrideKey := limit.GetThreshold(ip.String(), noRegistrationID) + if count.Count >= threshold { + return berrors.RegistrationsPerIPError(0, "too many registrations for this IP") + } if overrideKey != "" { // We do not support overrides for the NewRegistrationsPerIPRange limit. - utilization := float64(count.Count) / float64(threshold) + utilization := float64(count.Count+1) / float64(threshold) ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.RegistrationsPerIP, overrideKey).Set(utilization) } - if count.Count >= threshold { - return berrors.RegistrationsPerIPError(0, "too many registrations for this IP") - } - return nil } @@ -600,14 +599,14 @@ func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(ctx context. if err != nil { return err } - if overrideKey != "" { - utilization := float64(countPB.Count) / float64(threshold) - ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.PendingAuthorizationsPerAccount, overrideKey).Set(utilization) - } if countPB.Count >= threshold { ra.log.Infof("Rate limit exceeded, PendingAuthorizationsByRegID, regID: %d", regID) return berrors.RateLimitError(0, "too many currently pending authorizations: %d", countPB.Count) } + if overrideKey != "" { + utilization := float64(countPB.Count+1) / float64(threshold) + ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.PendingAuthorizationsPerAccount, overrideKey).Set(utilization) + } return nil } @@ -653,14 +652,14 @@ func (ra *RegistrationAuthorityImpl) checkInvalidAuthorizationLimit(ctx context. // here. noKey := "" threshold, overrideKey := limit.GetThreshold(noKey, regID) - if overrideKey != "" { - utilization := float64(count.Count) / float64(threshold) - ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.InvalidAuthorizationsPerAccount, overrideKey).Set(utilization) - } if count.Count >= threshold { ra.log.Infof("Rate limit exceeded, InvalidAuthorizationsByRegID, regID: %d", regID) return berrors.FailedValidationError(0, "too many failed authorizations recently") } + if overrideKey != "" { + utilization := float64(count.Count+1) / float64(threshold) + ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.InvalidAuthorizationsPerAccount, overrideKey).Set(utilization) + } return nil } @@ -684,14 +683,13 @@ func (ra *RegistrationAuthorityImpl) checkNewOrdersPerAccountLimit(ctx context.C // There is no meaningful override key to use for this rate limit noKey := "" threshold, overrideKey := limit.GetThreshold(noKey, acctID) - if overrideKey != "" { - utilization := float64(count.Count) / float64(threshold) - ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.NewOrdersPerAccount, overrideKey).Set(utilization) - } - if count.Count >= threshold { return berrors.RateLimitError(0, "too many new orders recently") } + if overrideKey != "" { + utilization := float64(count.Count+1) / float64(threshold) + ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.NewOrdersPerAccount, overrideKey).Set(utilization) + } return nil } @@ -1435,7 +1433,8 @@ func (ra *RegistrationAuthorityImpl) enforceNameCounts(ctx context.Context, name threshold, overrideKey := limit.GetThreshold(name, regID) if response.Counts[name] >= threshold { badNames = append(badNames, name) - } else if overrideKey != "" { + } + if overrideKey != "" { // Name is under threshold due to an override. utilization := float64(response.Counts[name]+1) / float64(threshold) metricsData = append(metricsData, struct { @@ -1513,13 +1512,12 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerFQDNSetLimit(ctx contex return fmt.Errorf("checking duplicate certificate limit for %q: %s", names, err) } - if overrideKey != "" { - utilization := float64(len(prevIssuances.TimestampsNS)) / float64(threshold) - ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.CertificatesPerFQDNSet, overrideKey).Set(utilization) - } - if int64(len(prevIssuances.TimestampsNS)) < threshold { // Issuance in window is below the threshold, no need to limit. + if overrideKey != "" { + utilization := float64(len(prevIssuances.TimestampsNS)+1) / float64(threshold) + ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.CertificatesPerFQDNSet, overrideKey).Set(utilization) + } return nil } else { // Evaluate the rate limit using a leaky bucket algorithm. The bucket diff --git a/ra/ra_test.go b/ra/ra_test.go index c5fb1f433d6c..8da7088bf521 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -719,12 +719,13 @@ func TestRegistrationsPerIPOverrideUsage(t *testing.T) { } // No error expected, the count of existing registrations for "4.5.6.7" - // should be 1 below the threshold. + // should be 1 below the override threshold. err := ra.checkRegistrationIPLimit(ctx, rlp, regIP, mockCounterAlwaysTwo) test.AssertNotError(t, err, "Unexpected error checking RegistrationsPerIPRange limit") - // Expecting ~66% of the override for "4.5.6.7" to be utilized. - test.AssertMetricWithLabelsEquals(t, ra.rlOverrideUsageGauge, prometheus.Labels{"limit": ratelimit.RegistrationsPerIP, "override_key": regIP.String()}, 0.6666666666666666) + // Accounting for the anticipated issuance, we expect "4.5.6.7" to be at + // 100% of their override threshold. + test.AssertMetricWithLabelsEquals(t, ra.rlOverrideUsageGauge, prometheus.Labels{"limit": ratelimit.RegistrationsPerIP, "override_key": regIP.String()}, 1) mockCounterAlwaysThree := func(context.Context, *sapb.CountRegistrationsByIPRequest, ...grpc.CallOption) (*sapb.Count, error) { return &sapb.Count{Count: 3}, nil