Merging to release-5.8: [TT-12827] A gateway using a redis rate limiter panics if any Gateway sharing the same Redis is using the DRL (#7558)

[probelabs]

User description

TT-12827 A gateway using a redis rate limiter panics if any Gateway sharing the same Redis is using the DRL (#7558)

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

---

PR Type

Bug fix, Tests

---

Description

• Skip DRL events when DRL disabled

• Centralize DRL disable check in helper

• Add tests for DRL skip behavior

---

Diagram Walkthrough

flowchart LR
  handleRedisEvent["handleRedisEvent: DRL notice"] -- "isDRLDisabled() true" --> skip["return (skip)"]
  handleRedisEvent -- "isDRLDisabled() false" --> onStatus["onServerStatusReceivedHandler"]

  startDRL["startDRL()"] -- "isDRLDisabled() true" --> noInit["Don't init DRL"]
  startDRL -- "isDRLDisabled() false" --> initDRL["Init DRL + notifications"]

Loading

File Walkthrough

	Relevant files
Tests	gateway_test.go Tests for skipping DRL notifications under other limiters gateway/gateway_test.go Add test to ensure DRL notices are skipped. Cover Sentinel, Redis Rolling, and Fixed Window limiters. Validate no handler invocation when alternate limiter enabled. +45/-0
Bug fix	redis_signals.go Guard DRL redis event handling with disable check                gateway/redis_signals.go Use centralized isDRLDisabled() gate for DRL notice handling. Prevent processing DRL notifications when DRL is disabled. +2/-4
Enhancement	server.go Centralize DRL disable logic and apply in startDRL              gateway/server.go Introduce isDRLDisabled() helper. Replace inline condition in startDRL with helper. Ensure DRL not initialized when disabled conditions met. +7/-3

---

[github-actions]

API Changes

no api changes detected

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 7558 - Partially compliant Compliant requirements: Prevent gateways that are not using DRL (or are Management Nodes) from handling DRL notifications to avoid panics when sharing Redis with DRL-enabled gateways. Ensure DRL initialization is skipped when DRL is disabled due to other rate limiters or management node mode. Add tests verifying that DRL notifications are ignored when alternative Redis-based rate limiters are enabled. Non-compliant requirements: Maintain existing behavior for other redis-driven notifications and flows. Requires further human verification: Maintain existing behavior for other redis-driven notifications and flows.

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Logic Gap Verify that skipping DRL notifications via isDRLDisabled() does not suppress any required side-effects (metrics/logging) and that Management Node behavior remains unchanged from previous intent. if gw.isDRLDisabled() { // DRL is disabled - other Rate Limiter is being used or this is a Management Node. return } gw.onServerStatusReceivedHandler(notif.Payload) Initialization Ordering Confirm that DRLManager and SessionLimiter initialization before the early return in startDRL() cannot cause unintended side effects when DRL is disabled, and that resources are not leaked. gw.drlOnce.Do(func() { drlManager := &drl.DRL{} gw.SessionLimiter = NewSessionLimiter(gw.ctx, &gwConfig, drlManager) gw.DRLManager = drlManager if gw.isDRLDisabled() { return } mainLog.Info("Initialising distributed rate limiter") Test Coverage Scope Tests cover sentinel/rolling/fixed-window cases; consider adding an explicit Management Node case to ensure DRL notifications are skipped there too. func TestDistributedRateLimiterDisabledRedisEvents(t *testing.T) { ts := StartTest(nil) t.Cleanup(ts.Close) drlNotificationCommand := &testMessageAdapter{ Msg: `{"Command": "NoticeGatewayDRLNotification"}`, } shouldNotHandleNotification := func(NotificationCommand) { assert.Fail(t, "should skip redis event") } globalConf := ts.Gw.GetConfig() t.Run("enabled Sentinel Rate Limiter - should skip DRL updates", func(*testing.T) { globalConf.EnableSentinelRateLimiter = true ts.Gw.SetConfig(globalConf) ts.Gw.handleRedisEvent(drlNotificationCommand, shouldNotHandleNotification, nil) globalConf.EnableSentinelRateLimiter = false ts.Gw.SetConfig(globalConf) }) t.Run("enabled Redis Rolling Limiter - should skip DRL updates", func(*testing.T) { globalConf.EnableRedisRollingLimiter = true ts.Gw.SetConfig(globalConf) ts.Gw.handleRedisEvent(drlNotificationCommand, shouldNotHandleNotification, nil) globalConf.EnableRedisRollingLimiter = false ts.Gw.SetConfig(globalConf) }) t.Run("enabled Fixed Window Rate Limiter - should skip DRL updates", func(*testing.T) { globalConf.EnableFixedWindowRateLimiter = true ts.Gw.SetConfig(globalConf) ts.Gw.handleRedisEvent(drlNotificationCommand, shouldNotHandleNotification, nil) globalConf.EnableFixedWindowRateLimiter = false ts.Gw.SetConfig(globalConf) }) }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Assert no-callback invocation reliably Failing inside the callback may not assert the callback was never invoked if the path returns early. Replace with a handler that records invocation and assert after handleRedisEvent returns, ensuring the negative condition is validated. gateway/gateway_test.go [640-642] +called := false shouldNotHandleNotification := func(NotificationCommand) { - assert.Fail(t, "should skip redis event") + called = true } +// ... +ts.Gw.handleRedisEvent(drlNotificationCommand, shouldNotHandleNotification, nil) +assert.False(t, called, "should skip redis event") Suggestion importance[1-10]: 7 __ Why: Recording and asserting post-call avoids immediate Fail inside callback and more robustly verifies the handler wasn't invoked; it slightly improves test reliability without altering logic.	Medium
	Isolate test config state Avoid reusing and mutating a single globalConf across subtests, which can leak state between runs and cause flakiness. Fetch a fresh config inside each subtest, mutate it, and restore the original at the end of that subtest. gateway/gateway_test.go [646-654] -globalConf := ts.Gw.GetConfig() - t.Run("enabled Sentinel Rate Limiter - should skip DRL updates", func(*testing.T) { - globalConf.EnableSentinelRateLimiter = true - ts.Gw.SetConfig(globalConf) + conf := ts.Gw.GetConfig() + original := conf + conf.EnableSentinelRateLimiter = true + ts.Gw.SetConfig(conf) ts.Gw.handleRedisEvent(drlNotificationCommand, shouldNotHandleNotification, nil) - globalConf.EnableSentinelRateLimiter = false - ts.Gw.SetConfig(globalConf) + ts.Gw.SetConfig(original) }) Suggestion importance[1-10]: 6 __ Why: Reusing a single config across subtests can leak state; fetching a fresh config per subtest improves test isolation and reduces flakiness. The change is accurate and beneficial but not critical.	Low
Possible issue	Prevent config read races Cache GetConfig() once per call site to avoid reading a moving target mid-decision, which can cause inconsistent behavior under concurrent config updates. Pass the already-fetched config into isDRLDisabled to make the check atomic with the caller's context. gateway/server.go [2066-2070] -func (gw *Gateway) isDRLDisabled() bool { - gwConfig := gw.GetConfig() - - return gwConfig.ManagementNode || gwConfig.EnableSentinelRateLimiter || gwConfig.EnableRedisRollingLimiter || gwConfig.EnableFixedWindowRateLimiter +func (gw *Gateway) isDRLDisabledWithConfig(cfg Config) bool { + return cfg.ManagementNode || cfg.EnableSentinelRateLimiter || cfg.EnableRedisRollingLimiter || cfg.EnableFixedWindowRateLimiter } +// call sites (examples): +// cfg := gw.GetConfig() +// if gw.isDRLDisabledWithConfig(cfg) { ... } + Suggestion importance[1-10]: 5 __ Why: The idea to avoid multiple GetConfig reads is reasonable for consistency, but it proposes new API and call-site changes not present in the PR and is not strictly necessary here; impact is moderate and the improved_code does not map directly to existing usage.	Low

[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