Merging to release-5-lts: [TT-10856/TT-11593]fix quota limits not working with url rewrite to self (#6133) #6168
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…elf (#6133) fix an issue where quota limits were not applied when URL rewrite middleware target is using `tyk://self/<target-endpoint>/` Quota limits were not applied when URL rewrite middleware is enabled with target of format `tyk://self/<target-endpoint>/`. This was happening with keys created with `access_rights` specified for per API basis. Creating keys from policies also filled in `access_rights` in keys. Having `access_rights` filled in caused `quota_renews` to be set to 0 during URL rewrite. This caused quota limit resets everytime. Updating session before URL rewrite would fix this as the `quota_renews` is correctly applied. calling `mw.Base().UpdateRequestSession(r)` after every middleware execution is okay because it will update session only if a session update is scheduled. https://github.com/TykTechnologies/tyk/blob/53886d044c43930b332dbe4a73cf727069df0770/gateway/middleware.go#L329 The quota related values returned in API response headers `X-RateLimit-Limit`, `X-RateLimit-Remaining`, `X-RateLimit-Reset` were wrong since a session update was scheduled only during the first hit after a quota reset. This PR also fixes this behaviour to report the correct quota information in those headers. https://tyktech.atlassian.net/browse/TT-10856 https://tyktech.atlassian.net/browse/TT-11593 <!-- Why is this change required? What problem does it solve? --> <!-- Please describe in detail how you tested your changes --> <!-- Include details of your testing environment, and the tests --> <!-- you ran to see how your change affects other areas of the code, etc. --> <!-- This information is helpful for reviewers and QA. --> <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) <!-- Go over all the following points, and put an `x` in all the boxes that apply --> <!-- If there are no documentation updates required, mark the item as checked. --> <!-- Raise up any additional concerns not covered by the checklist. --> - [ ] I ensured that the documentation is up to date - [ ] I explained why this PR updates go.mod in detail with reasoning why it's required - [ ] I would like a code coverage CI quality gate exception and have explained why ___ bug_fix, tests ___ - Fixed an issue where quota limits were not applied when URL rewrite middleware target is using `tyk://self/<endpoint>/`. - Added a test case to ensure quota limits are correctly enforced when using URL rewrite to self. ___ <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>middleware.go</strong><dd><code>Ensure Session Updates After Middleware Execution</code> </dd></summary> <hr> gateway/middleware.go <li>Added call to <code>UpdateRequestSession</code> after middleware execution to <br>ensure session updates, particularly for quota limits, are applied.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6133/files#diff-703054910891a4db633eca0f42ed779d6b4fa75cd9b3aa4c503e681364201c1b">+1/-0</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>middleware_test.go</strong><dd><code>Add Test for Quota Limits with URL Rewrite to Self</code> </dd></summary> <hr> gateway/middleware_test.go <li>Imported <code>time</code> package for session creation.<br> <li> Added <code>createSession</code> helper function to simplify session creation for <br>tests.<br> <li> Added <code>TestQuotaNotAppliedWithURLRewrite</code> to validate quota limits are <br>correctly applied when using URL rewrite to self.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6133/files#diff-6a09a08e3f82cc5e9d8c6b5c8426d75ea1e5d85e15ab008fca1f512e7c49c1e6">+66/-0</a> </td> </tr> </table></td></tr></tr></tbody></table> ___ > ✨ **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions (cherry picked from commit 3a78b0d)
|
API Changes no api changes detected |
💥 CI tests failed 🙈git-stateall okPlease look at the run or in the Checks tab. |
|
jeffy-mathew
approved these changes
Mar 22, 2024
jeffy-mathew
deleted the
merge/release-5-lts/3a78b0d053d9f3309d9c51c79dc630b0828c793e
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[TT-10856/TT-11593]fix quota limits not working with url rewrite to self (#6133)
User description
fix an issue where quota limits were not applied when URL rewrite
middleware target is using
tyk://self/<target-endpoint>/Description
Quota limits were not applied when URL rewrite middleware is enabled
with target of format
tyk://self/<target-endpoint>/.This was happening with keys created with
access_rightsspecified forper API basis.
Creating keys from policies also filled in
access_rightsin keys.Having
access_rightsfilled in causedquota_renewsto be set to 0during URL rewrite. This caused quota limit resets everytime.
Updating session before URL rewrite would fix this as the
quota_renewsis correctly applied.
calling
mw.Base().UpdateRequestSession(r)after every middlewareexecution is okay because it will update session only if a session
update is scheduled.
tyk/gateway/middleware.go
Line 329 in 53886d0
The quota related values returned in API response headers
X-RateLimit-Limit,X-RateLimit-Remaining,X-RateLimit-Resetwerewrong since a session update was scheduled only during the first hit
after a quota reset. This PR also fixes this behaviour to report the
correct quota information in those headers.
Related Issue
https://tyktech.atlassian.net/browse/TT-10856
https://tyktech.atlassian.net/browse/TT-11593
Motivation and Context
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
functionality to change)
coverage to functionality)
Checklist
why it's required
explained why
Type
bug_fix, tests
Description
middleware target is using
tyk://self/<endpoint>/.using URL rewrite to self.
Changes walkthrough
middleware.go
Ensure Session Updates After Middleware Executiongateway/middleware.go
UpdateRequestSessionafter middleware execution toensure session updates, particularly for quota limits, are applied.
middleware_test.go
Add Test for Quota Limits with URL Rewrite to Selfgateway/middleware_test.go
timepackage for session creation.createSessionhelper function to simplify session creation fortests.
TestQuotaNotAppliedWithURLRewriteto validate quota limits arecorrectly applied when using URL rewrite to self.