[TT-12827] A gateway using a redis rate limiter panics if any Gateway sharing the same Redis is using the DRL

[MaciekMis]

Description

This pull request fixes an issue where a gateway that uses enable_redis_rolling_limiter, enable_sentinel_rate_limiter or enable_fixed_window_rate_limiter shares redis with a gateway which is sending DRL updates panics as soon as it receives a DRL update

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• 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

• 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

Ticket Details

TT-12827

Status	In Code Review
Summary	A gateway using a redis rate limiter panics if any Gateway sharing the same Redis is using the DRL

Generated at: 2025-11-25 16:31:44

[github-actions]

API Changes

no api changes detected

[probelabs]

This pull request resolves a panic in the Tyk Gateway that occurs when gateways with different Redis-based rate-limiting configurations share the same Redis instance. Specifically, a gateway using a non-DRL (Distributed Rate Limiter) Redis rate limiter (e.g., Sentinel, rolling window) would panic upon receiving a DRL update notification from another gateway.

The fix introduces a check within the Redis event handler to verify if DRL is disabled on the receiving gateway. If it is, the DRL notification is safely ignored, preventing the system from accessing an uninitialized DRL manager and thus averting the panic.

Files Changed Analysis

• gateway/server.go: The logic to determine if DRL is disabled has been refactored into a new helper function, isDRLDisabled(). This centralizes the check and improves code clarity.
• gateway/redis_signals.go: The core of the fix resides here. The handleRedisEvent function now calls isDRLDisabled() before processing a NoticeGatewayDRLNotification. If DRL is disabled, it returns early, which is the key change that prevents the panic.
• gateway/gateway_test.go: A new test suite, TestDistributedRateLimiterDisabledRedisEvents, has been added. It explicitly verifies that gateways configured with other Redis-based rate limiters correctly ignore DRL notifications, ensuring the fix is effective and preventing regressions.

Architecture & Impact Assessment

• What this PR accomplishes: It fixes a critical panic, enabling stable operation of Tyk Gateway clusters where different Redis-based rate-limiting strategies coexist on a shared Redis instance.
• Key technical changes: The primary change is the introduction of a guard clause in the Redis event handler to filter irrelevant DRL notifications on non-DRL nodes. This is controlled by the new isDRLDisabled() function.
• Affected system components: The changes are localized to the Gateway's rate limiting and Redis event handling subsystems.

graph TD
    A[Redis Receives DRL Notification] --> B{isDRLDisabled?};
    B -->|Yes| C[Ignore Notification & Prevent Panic];
    B -->|No| D[Process DRL Update as Normal];

Loading

Scope Discovery & Context Expansion

• The changes are well-contained within the gateway package, specifically targeting the interaction between the rate limiter configuration and the Redis pub/sub event handler.
• The impact is limited to gateway nodes that have DRL disabled but are part of a cluster where other nodes use DRL. The fix ensures these nodes can coexist safely without crashing.
• The added tests in gateway_test.go sufficiently cover the scenarios that caused the panic, confirming the fix is effective and isolated.

Metadata

• Review Effort: 2 / 5
• Primary Label: bug

---

Powered by Visor from Probelabs

Last updated: 2025-11-25T16:32:50.132Z | Triggered by: pr_updated | Commit: 6214b39

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity	Location	Issue
🟡 Warning	gateway/gateway_test.go:642-671	The test cases for different rate limiters are nearly identical, leading to code duplication. This makes the test harder to maintain and extend. Additionally, the test state (gateway configuration) is manually reset at the end of each sub-test, which is fragile; if a test panics, the cleanup code will not run, potentially affecting subsequent tests. 💡 Suggestion Refactor these sub-tests into a single table-driven test. This will eliminate duplication and make it easier to add new cases. Use `t.Cleanup` within each sub-test's loop to ensure the configuration is reliably restored, even if the test panics.

---

Powered by Visor from Probelabs

Last updated: 2025-11-25T16:32:55.455Z | Triggered by: pr_updated | Commit: 6214b39

💡 TIP: You can chat with Visor using /visor ask <your question>

[MaciekMis]

/visor ask handleRedisEvent function calls a callback once the event processing logic is done. In case of NoticeGatewayDRLNotification the callback is not called specifically for this event but after processing - handled(notif.Command). Due to this implementation callback is called when NoticeGatewayDRLNotification is processed but it is skipped when NoticeGatewayDRLNotification event occurs in non DRL Gateway. Please reevaluate your comment about tests.

[probelabs]

You are correct. Thank you for the clarification.

In handleRedisEvent, the handled callback is intentionally not called for a NoticeGatewayDRLNotification when DRL is disabled. The function returns early to prevent a non-DRL gateway from processing the event.

The new test, TestDistributedRateLimiterDisabledRedisEvents, validates exactly this behavior. It provides a callback that fails the test if it's invoked, confirming that the event is correctly skipped.

Therefore, the implementation and the tests are aligned.

References:

• gateway/redis_signals.go:142-147 – Early return for DRL notifications
• gateway/gateway_test.go:633-640 – Test asserting the callback is not called

---

Powered by Visor from Probelabs

Last updated: 2025-11-24T12:25:33.479Z | Triggered by: MaciekMis | Commit: 4618f45

💡 TIP: You can chat with Visor using /visor ask <your question>

[buraksezer]

Modifying the global config can lead to flaky tests, but this is already a known issue in the codebase.

[sonarqubecloud]

Quality Gate failed

Failed conditions
E Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

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

[MaciekMis]

SonarCube scanned every file that had any change in this PR - those errors are not related to introduced code

[MaciekMis]

/release to release-5.8

[probelabs]

✅ Cherry-pick successful. A PR was created: #7565