fix(redis): RedisClusterBroker silently drops TLS config from BaseSecurity#2935
Merged
Conversation
BaseSecurity(use_ssl=True) conveys TLS via connection_class, which CLUSTER_INCOMPATIBLE_PARAMS filters out of the cluster connection options — so RedisClusterBroker silently connected over plaintext. Translate it to RedisCluster's native `ssl` flag instead (also for rediss:// URLs), and warn when a custom ssl_context is provided since RedisCluster cannot accept one. Also fix _parse_base_security to fall back to ssl=True when no explicit ssl_context is given — previously it passed ssl=None, disabling TLS for the plain RedisBroker as well. Fixes #2909 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
powersemmi
approved these changes
Jul 14, 2026
This file contains hidden or 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
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.
Description
RedisClusterBrokersilently dropped TLS when configured withBaseSecurity(use_ssl=True):parse_security()conveys TLS viaconnection_class: SSLConnection, butCLUSTER_INCOMPATIBLE_PARAMSfiltersconnection_classout in_resolve_url_options(), soRedisCluster(**opts)received zero SSL config and connected over plaintext (timing out against TLS-only clusters such as AWS MemoryDB). Introduced in #2854.Fixes #2909
What changed
RedisClusterBroker._resolve_url_optionsnow translates TLS intent intoRedisCluster's nativessl=Trueflag after the incompatible-params filter. This covers bothBaseSecurity(use_ssl=True)andrediss://URLs (which hit the same silent-drop path viaparse_url'sconnection_class).setdefaultis used so explicit userssl/ssl_*kwargs are preserved, and the injectedssloption also flows into the sync Pub/Sub cluster client, which already reads it.ssl_contexton cluster: the asyncRedisClusterconstructor has nossl_context/connection_classparameter (verified against redis-py 7.4), so the fix proposed in the issue would raiseTypeErrorat connect time. Instead aRuntimeWarningis emitted telling the user to use thessl_ca_certs/ssl_certfile/ssl_keyfile/ssl_*options, whilessl=Trueis still applied — no more silent plaintext._parse_base_security(plainRedisBroker): the generatedSSLConnection._connection_argumentspassedssl: Nonewhen no explicitssl_contextwas given, which disables TLS inasyncio.open_connection— the same bug in the non-cluster broker. It now falls back tossl: True(default SSL context).Verified end-to-end that the options produced by the broker construct a
RedisClusterwhoseconnection_kwargs["connection_class"]is redis-py's ownSSLConnection, and that the issue's repro assertions are resolved.Type of change
Checklist
just lintshows no errors)just static-analysis🤖 Generated with Claude Code