TT-11485, fix for global rate limit disabled flag not working#6120
Conversation
|
PR Description updated to latest commit (382153a) |
PR Review
Code feedback:
✨ Review tool usage guide:Overview: With a configuration file, use the following template:
See the review usage page for a comprehensive guide on using this tool. |
PR Code Suggestions
✨ Improve tool usage guide:Overview: With a configuration file, use the following template:
See the improve usage page for a more comprehensive guide on using this tool. |
|
API Changes no api changes detected |
|
API tests result - postgres15-sha256 env: success ✅ |
|
API tests result - mongo44-sha256 env: success ✅ |
💥 CI tests failed 🙈git-statediff --git a/gateway/mw_api_rate_limit_test.go b/gateway/mw_api_rate_limit_test.go
index 0b47c01..7150f0d 100644
--- a/gateway/mw_api_rate_limit_test.go
+++ b/gateway/mw_api_rate_limit_test.go
@@ -1,13 +1,14 @@
package gateway
import (
- "github.com/stretchr/testify/assert"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"
+ "github.com/stretchr/testify/assert"
+
"github.com/justinas/alice"
"github.com/TykTechnologies/tyk/internal/uuid"Please look at the run or in the Checks tab. |
1 similar comment
💥 CI tests failed 🙈git-statediff --git a/gateway/mw_api_rate_limit_test.go b/gateway/mw_api_rate_limit_test.go
index 0b47c01..7150f0d 100644
--- a/gateway/mw_api_rate_limit_test.go
+++ b/gateway/mw_api_rate_limit_test.go
@@ -1,13 +1,14 @@
package gateway
import (
- "github.com/stretchr/testify/assert"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"
+ "github.com/stretchr/testify/assert"
+
"github.com/justinas/alice"
"github.com/TykTechnologies/tyk/internal/uuid"Please look at the run or in the Checks tab. |
…-api-rate-limiter' into TT-11485-not-possible-to-disable-api-rate-limiter
|
💥 CI tests failed 🙈git-stateall okPlease look at the run or in the Checks tab. |
|
API tests result - postgres15-murmur64 env: success ✅ |
|
API tests result - mongo44-murmur64 env: success ✅ |
|
API tests result - mongo44-murmur64 env: success ✅ User descriptionDescriptionRelated IssueMotivation and ContextHow This Has Been TestedScreenshots (if appropriate)Types of changes
Checklist
TypeBug fix Description
Changes walkthrough
|
|
/release to release-5.3 |
|
/release to release-5.3.0 |
|
Working on it! Note that it can take a few minutes. |
|
Working on it! Note that it can take a few minutes. |
## **User description**
<!-- Provide a general summary of your changes in the Title above -->
## Description
<!-- Describe your changes in detail -->
## Related Issue
<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->
## Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
## How This Has Been Tested
<!-- 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. -->
## Screenshots (if appropriate)
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] 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)
## Checklist
<!-- 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
___
## **Type**
Bug fix
___
## **Description**
- Fixed an issue where the global rate limit disabled flag was not
properly checked, causing the rate limiter to be incorrectly enabled.
___
## **Changes walkthrough**
<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>mw_api_rate_limit.go</strong><dd><code>Fix Global Rate
Limit Disabled Flag Check</code>
</dd></summary>
<hr>
gateway/mw_api_rate_limit.go
<li>Added a condition to check <code>GlobalRateLimit.Disabled</code>
flag in <br><code>EnabledForSpec</code> function.<br>
</details>
</td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6120/files#diff-46326b04f936c839922e970db5c2924156cc797070948f3dc9c589d04661d6d2">+1/-1</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 53886d0)
…flag not working (#6120) TT-11485, fix for global rate limit disabled flag not working (#6120) ## **User description** <!-- Provide a general summary of your changes in the Title above --> ## Description <!-- Describe your changes in detail --> ## Related Issue <!-- This project only accepts pull requests related to open issues. --> <!-- If suggesting a new feature or change, please discuss it in an issue first. --> <!-- If fixing a bug, there should be an issue describing it with steps to reproduce. --> <!-- OSS: Please link to the issue here. Tyk: please create/link the JIRA ticket. --> ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## How This Has Been Tested <!-- 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. --> ## Screenshots (if appropriate) ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] 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) ## Checklist <!-- 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 ___ ## **Type** Bug fix ___ ## **Description** - Fixed an issue where the global rate limit disabled flag was not properly checked, causing the rate limiter to be incorrectly enabled. ___ ## **Changes walkthrough** <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>mw_api_rate_limit.go</strong><dd><code>Fix Global Rate Limit Disabled Flag Check</code> </dd></summary> <hr> gateway/mw_api_rate_limit.go <li>Added a condition to check <code>GlobalRateLimit.Disabled</code> flag in <br><code>EnabledForSpec</code> function.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6120/files#diff-46326b04f936c839922e970db5c2924156cc797070948f3dc9c589d04661d6d2">+1/-1</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
|
@andrei-tyk Succesfully merged PR |
## **User description**
<!-- Provide a general summary of your changes in the Title above -->
## Description
<!-- Describe your changes in detail -->
## Related Issue
<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->
## Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
## How This Has Been Tested
<!-- 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. -->
## Screenshots (if appropriate)
## Types of changes
<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->
- [ ] 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)
## Checklist
<!-- 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
___
## **Type**
Bug fix
___
## **Description**
- Fixed an issue where the global rate limit disabled flag was not
properly checked, causing the rate limiter to be incorrectly enabled.
___
## **Changes walkthrough**
<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>mw_api_rate_limit.go</strong><dd><code>Fix Global Rate
Limit Disabled Flag Check</code>
</dd></summary>
<hr>
gateway/mw_api_rate_limit.go
<li>Added a condition to check <code>GlobalRateLimit.Disabled</code>
flag in <br><code>EnabledForSpec</code> function.<br>
</details>
</td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6120/files#diff-46326b04f936c839922e970db5c2924156cc797070948f3dc9c589d04661d6d2">+1/-1</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 53886d0)
|
Still working... |
|
@andrei-tyk Seems like there is conflict and it require manual merge. |
…d flag not working (#6120) (#6128) ## **User description** TT-11485, fix for global rate limit disabled flag not working (#6120) ## **User description** <!-- Provide a general summary of your changes in the Title above --> ## Description <!-- Describe your changes in detail --> ## Related Issue <!-- This project only accepts pull requests related to open issues. --> <!-- If suggesting a new feature or change, please discuss it in an issue first. --> <!-- If fixing a bug, there should be an issue describing it with steps to reproduce. --> <!-- OSS: Please link to the issue here. Tyk: please create/link the JIRA ticket. --> ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## How This Has Been Tested <!-- 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. --> ## Screenshots (if appropriate) ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] 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) ## Checklist <!-- 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 ___ ## **Type** Bug fix ___ ## **Description** - Fixed an issue where the global rate limit disabled flag was not properly checked, causing the rate limiter to be incorrectly enabled. ___ ## **Changes walkthrough** <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>mw_api_rate_limit.go</strong><dd><code>Fix Global Rate Limit Disabled Flag Check</code> </dd></summary> <hr> gateway/mw_api_rate_limit.go <li>Added a condition to check <code>GlobalRateLimit.Disabled</code> flag in <br><code>EnabledForSpec</code> function.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6120/files#diff-46326b04f936c839922e970db5c2924156cc797070948f3dc9c589d04661d6d2">+1/-1</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 ___ ## **Type** Bug fix, Tests ___ ## **Description** - Added a missing condition to properly check if the global rate limit is disabled in the `EnabledForSpec` function. - Introduced a new test case to ensure the `EnabledForSpec` function respects the `GlobalRateLimit.Disabled` flag. ___ ## **Changes walkthrough** <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>mw_api_rate_limit.go</strong><dd><code>Fix Global Rate Limit Disabled Flag Check</code> </dd></summary> <hr> gateway/mw_api_rate_limit.go <li>Added a condition to check if <code>GlobalRateLimit.Disabled</code> is true in the <br><code>EnabledForSpec</code> function.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6128/files#diff-46326b04f936c839922e970db5c2924156cc797070948f3dc9c589d04661d6d2">+1/-1</a> </td> </tr> </table></td></tr><tr><td><strong>Tests </strong></td><td><table> <tr> <td> <details> <summary><strong>mw_api_rate_limit_test.go</strong><dd><code>Add Test for Global Rate Limit Disabled Flag</code> </dd></summary> <hr> gateway/mw_api_rate_limit_test.go <li>Imported <code>apidef</code> and <code>assert</code> packages for testing.<br> <li> Added a new test <code>TestRateLimitForAPI_EnabledForSpec</code> to verify the <br>correct behavior of the <code>EnabledForSpec</code> function when the global rate <br>limit is disabled.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6128/files#diff-8485978214ae83eb8aeeda3519a1e90350a2a84067de47462f40f0938861dc45">+11/-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 Co-authored-by: andrei-tyk <[email protected]>



User description
Description
Related Issue
Motivation and Context
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
Checklist
Type
Bug fix
Description
Changes walkthrough
mw_api_rate_limit.go
Fix Global Rate Limit Disabled Flag Checkgateway/mw_api_rate_limit.go
GlobalRateLimit.Disabledflag inEnabledForSpecfunction.