[TT-12378] Spike/rate limits on endpoints

[titpetric]

PR Type

Enhancement, Documentation

---

Description

• Added RateLimitMeta struct to handle rate limiting metadata.
• Integrated RateLimitMeta into ExtendedPathsSet and URLSpec.
• Added compileRateLimitPathSpec function to compile rate limiting paths.
• Simplified LimiterKey function by removing allowanceScope parameter.
• Updated various compile functions to use correct naming conventions.
• Added comprehensive documentation for rate limiting, including Redis key construction and SessionLimiter function details.

---

Changes walkthrough 📝

	Relevant files
Enhancement	api_definitions.go Add RateLimitMeta struct and integrate with ExtendedPathsSet apidef/api_definitions.go Added RateLimitMeta struct for rate limiting metadata. Included RateLimitMeta in ExtendedPathsSet. +10/-0    api_definition.go Integrate rate limiting into API definition and path compilation gateway/api_definition.go Added RateLimit constant for rate limiting. Integrated RateLimitMeta into URLSpec. Added compileRateLimitPathSpec function for rate limiting paths. Updated various compile functions to use correct naming conventions. +36/-12  limiter.go Simplify LimiterKey function in rate limiter                          internal/rate/limiter.go Simplified LimiterKey function by removing allowanceScope parameter. +1/-6
Documentation	rate-limit.md Add documentation for rate limiting implementation              docs/rate-limit.md Added documentation for rate limiting implementation. Explained Redis key construction for rate limiting. Provided details on SessionLimiter and related functions. +85/-0

---

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions[bot]]

API Changes

no api changes detected

[github-actions[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review **Possible Bug:** The removal of `allowanceScope` in `LimiterKey` function might lead to unexpected behavior if the function relies on this parameter to generate unique keys for rate limiting. Ensure that this change does not affect the uniqueness and correctness of the generated keys. **Code Consistency:** The renaming of functions to correct naming conventions (e.g., `compileVirtualPathSpec` from `compileVirtualPathspathSpec`) is good for consistency. However, ensure all references to these functions are updated across the codebase to avoid runtime errors. **Documentation:** Ensure the new documentation added in `docs/rate-limit.md` is accurate and matches the implemented logic in the code. This includes the details on Redis key construction and session management.

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add nil pointer checks in compileRateLimitPathSpec to prevent runtime panics ___ **Consider checking for nil pointers in compileRateLimitPathSpec when accessing properties of paths items to avoid potential runtime panics.** [gateway/api_definition.go [1349-1350]](https://github.com/TykTechnologies/tyk/pull/6389/files#diff-0cf80174bbafb36f6d4f4308ebbd971b2833b76a936bad568220aa1a4ba0ee8bR1349-R1350) ```diff func (a APIDefinitionLoader) compileRateLimitPathSpec(paths []apidef.RateLimitMeta, stat URLStatus, apiSpec *APISpec, conf config.Config) []URLSpec { urlSpec := []URLSpec{} for _, stringSpec := range paths { - if stringSpec.Disabled { + if stringSpec == nil || stringSpec.Disabled { continue } ``` Suggestion importance[1-10]: 10 Why: Adding nil pointer checks is crucial to prevent potential runtime panics, ensuring the stability and reliability of the function. This is a critical improvement for robustness.	10
Enhancement	Add a validation method to ensure positive, non-zero values for Rate and Per ___ **Consider adding a validation method for RateLimitMeta to ensure that the Rate and Per values are positive and non-zero. This will prevent runtime errors or logical bugs due to incorrect configurations.** [apidef/api_definitions.go [216-222]](https://github.com/TykTechnologies/tyk/pull/6389/files#diff-9961ccc89a48d32db5b47ba3006315ef52f6e5007fb4b09f8c5d6d299c669d67R216-R222) ```diff type RateLimitMeta struct { Disabled bool `bson:"disabled" json:"disabled"` Path string `bson:"path" json:"path"` Method string `bson:"method" json:"method"` Rate float64 `bson:"rate" json:"rate"` Per float64 `bson:"per" json:"per"` } +func (rl *RateLimitMeta) Validate() error { + if rl.Rate <= 0 || rl.Per <= 0 { + return fmt.Errorf("rate and per must be positive and non-zero") + } + return nil +} + ``` Suggestion importance[1-10]: 9 Why: Adding validation for `Rate` and `Per` ensures that invalid configurations are caught early, preventing potential runtime errors or logical bugs. This is a significant improvement for robustness.	9
Possible issue	Handle empty rateScope to avoid generating keys with double dashes ___ **Refactor the LimiterKey function to handle the case where rateScope might be empty, which could lead to generating keys with double dashes. This adjustment ensures cleaner and more predictable key formatting.** [internal/rate/limiter.go [35-36]](https://github.com/TykTechnologies/tyk/pull/6389/files#diff-84e3fda0965028c6e464ae0916aaef0af63d0367fde6f910b513a610f2a34ee5R35-R36) ```diff func LimiterKey(currentSession *user.SessionState, rateScope string, key string, useCustomKey bool) string { + if rateScope != "" { + rateScope += "-" + } if !useCustomKey && !currentSession.KeyHashEmpty() { return Prefix(LimiterKeyPrefix, rateScope, currentSession.KeyHash()) } ``` Suggestion importance[1-10]: 8 Why: Handling the case where `rateScope` might be empty ensures cleaner and more predictable key formatting, which is important for maintaining consistent and error-free key generation.	8
Maintainability	Add explanatory comments for the RateLimit field in the URLSpec struct ___ **Add a comment above the RateLimit field in the URLSpec struct to explain its purpose and usage within the system, enhancing code readability and maintainability.** [gateway/api_definition.go [162]](https://github.com/TykTechnologies/tyk/pull/6389/files#diff-0cf80174bbafb36f6d4f4308ebbd971b2833b76a936bad568220aa1a4ba0ee8bR162-R162) ```diff +// RateLimit holds the metadata for rate limiting on this specific URL path. RateLimit apidef.RateLimitMeta ``` Suggestion importance[1-10]: 6 Why: Adding comments improves code readability and maintainability, helping future developers understand the purpose of the `RateLimit` field. However, it is a minor enhancement.	6

[titpetric]

Only rebased to see how much we deviated. Not much.

[sonarcloud[bot]]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint