Open mburumaxwell opened 1 year ago
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?
@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=
@adityamandaleeka this is done
Thanks @mburumaxwell
What steps follow to get this to be in the next version of AspNetCore?
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:
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.
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.
API Review Notes:
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
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);
+ }
+ }
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:
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 https://github.com/microsoft/reverse-proxy/pull/1967Proposed API
It would be preferred to something similar to
IAuthorizationPolicyProvider
implemented viaDefaultAuthorizationPolicyProvider
andICorsPolicyProvider
implemented viaDefaultCorsPolicyProvider
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