Rate Limiting configuration - policy validation

[mburumaxwell]

Background and Motivation

The ASP.NET Core rate limiting middleware is great, but "limited" in terms of policy validation. Let's start with some code that you can write today in .NET 7:

builder.Services.AddRateLimiter(options =>
{
    options.AddFixedWindowLimiter("customPolicy", opt =>
    {
        opt.PermitLimit = 4;
        opt.Window = TimeSpan.FromSeconds(12);
        opt.QueueProcessingOrder = QueueProcessingOrder.OldestFirst;
        opt.QueueLimit = 2;
    });
    // ...
});

There is no way to validate that customPolicy actually exists. This is useful when configuring multiple routes from configuration such as is the case for YARP. See microsoft/reverse-proxy#1967

Proposed API

It would be preferred to something similar to IAuthorizationPolicyProvider implemented via DefaultAuthorizationPolicyProvider and ICorsPolicyProvider implemented via DefaultCorsPolicyProvider

namespace Microsoft.AspNetCore.RateLimiting;

-  internal struct DefaultKeyType 
+  public struct DefaultKeyType 
{
// omitted ...
}
+
+ public interface IRateLimiterPolicyProvider
+ {
+     ValueTask<IRateLimiterPolicy<DefaultKeyType>?> GetDefaultPolicyAsync();
+     ValueTask<IRateLimiterPolicy<DefaultKeyType>?> GetPolicyAsync(string policyName);
+ }
+
+ public class DefaultRateLimiterPolicyProvider : IRateLimiterPolicyProvider
+ {
+     private readonly RateLimiterOptions _options;
+     
+     public DefaultRateLimiterPolicyProvider(IOptions<RateLimiterOptions> options)
+     {
+     
+     }
+     
+     public ValueTask<IRateLimiterPolicy<DefaultKeyType>?> GetPolicyAsync(string policyName)
+     {
+         options.PolicyMap[policyName] ?? options.UnactivatedPolicyMap[policyName];
+     }
+ }

RateLimiterOptions.PolicyMap is internal hence this feature cannot be added in another library or the final application.

Usage Examples

See YARP: https://github.com/microsoft/reverse-proxy/blob/26ce1d15f868cb8da1891d65db1e59a20fd6ecbf/src/ReverseProxy/Configuration/ConfigValidator.cs#L312-L318

Alternative Designs

None

Risks

None

[adityamandaleeka]

Triage: this seems like a reasonable suggestion (a way to find out about these issues at load-time rather than run-time).

Would an API that returned a bool (indicating whether the policy exists) be sufficient?

[adityamandaleeka]

@mburumaxwell Can you update your comment to make it follow the API proposal template here? https://github.com/dotnet/aspnetcore/issues/new?assignees=&labels=api-suggestion&template=30_api_proposal.md&title=

[mburumaxwell]

@adityamandaleeka this is done

[adityamandaleeka]

Thanks @mburumaxwell

[mburumaxwell]

What steps follow to get this to be in the next version of AspNetCore?

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[adityamandaleeka]

This proposal will be discussed by our team in an upcoming API review meeting, after which we'll provide feedback/suggestions.

Once a proposal gets to the api-approved state, we'll be ready to take a PR to implement the change.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[halter73]

API Review Notes:

• We don't like making the DefaultKeyType public if possible.
• It's not clear what you'd use the IRateLimiterPolicy<DefaultKeyType> for other than just checking that it exists like YARP does.

We think the API needs work. Maybe it could be combined with the rate limit feature proposal. #45658

[mburumaxwell]

The proposal in #45658 cannot work because the validation would only be available where a HttpContext is yet YARP needs validation elsewhere.

If I understand correctly, the main issue is making DefaultKeyType public. What if instead we just have boolean values?

namespace Microsoft.AspNetCore.RateLimiting;

+ public interface IRateLimiterPolicyProvider
+ {
+     bool DefaultPolicyExists();
+     bool PolicyExists(string policyName);
+ }
+
+ public class DefaultRateLimiterPolicyProvider : IRateLimiterPolicyProvider
+ {
+     private readonly RateLimiterOptions _options;
+     
+     public DefaultRateLimiterPolicyProvider(IOptions<RateLimiterOptions> options)
+     {
+     
+     }
+     
+     public bool PolicyExists(string policyName)
+     {
+         options.PolicyMap.ContainsKey(policyName) || options.UnactivatedPolicyMap.ContainsKey(policyName);
+     }
+ }