Skip to content

Sentinel TLS: discover_master is not respect connection type. #3128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
garry-t opened this issue Jan 30, 2024 · 11 comments
Open

Sentinel TLS: discover_master is not respect connection type. #3128

garry-t opened this issue Jan 30, 2024 · 11 comments

Comments

@garry-t
Copy link

garry-t commented Jan 30, 2024

In case Sentinel configured with TLS , redis_master call fails with error: "Connection reset by peer", means it is not use TLS during function call.
I've checked all docs which I able to found and for me still not clear is it supported or no?
Version: 5.1.0b3

Platform: Python 3.11 MacOS

Description:
SSL were generated self signed.
My simple code:

 sentinel = Sentinel(
            sentinels=sentinel_addresses,
            sentinel_kwargs={'password': secret_file_data['password']},
            socket_timeout=0.1,
            ssl=True,
            ssl_keyfile=ssl_keyfile,
            ssl_certfile=ssl_certfile,
            ssl_ca_certs=ssl_ca_certs
        )
 host, port = sentinel.discover_master(master_name)
 master = sentinel.master_for(master_name, password=secret_file_data['password'])
 print(f"Current {'SSL' if use_ssl else 'Non-SSL'} Connection - Master IP: {host}")

In sentinel logs

Error accepting a client connection: error:1408F10B:SSL routines:ssl3_get_record:wrong version number (addr=IP:56828 laddr=IP:26380)

@erihu78
Copy link

erihu78 commented Sep 4, 2024

I have exactly the same issue!
Any news on this??

@garionphx
Copy link

I ran into a similar problem, only with authentication. I created a merge request that fixes my issue, and maybe yours also: #3376

@rad-pat
Copy link

rad-pat commented Oct 15, 2024

Don't you fix this by passing the necessary ssl kwargs into the sentinel_kwargs?

@tgckpg
Copy link

tgckpg commented Dec 10, 2024

Good lord this bug is hard to track down. Thankfully ChatGPT has guided me here so I'm posting my workaround for anyone who also have this issue.

First you need this class

from redis.connection import SSLConnection
from redis.sentinel import SentinelManagedConnection

class SentinelManagedSSLConnection( SentinelManagedConnection, SSLConnection ):

    def __init__( self, *args, **kwargs ):
        kwargs.pop( "ssl", None )
        super().__init__( *args, **kwargs )

Then you can use it like so

s = {
    "ssl": True
    , "ssl_keyfile": os.path.join( Env.REDIS_CERTS, "key" )
    , "ssl_certfile": os.path.join( Env.REDIS_CERTS, "cert" )
    , "ssl_ca_certs": os.path.join( Env.REDIS_CERTS, "ca" )
}

sentinelObj.master_for( "mymaster", connection_class=SentinelManagedSSLConnection, **s)

Basically SentinelManagedConnection was not an SSLConnection so we make it an SSLConnection.

@abdhx
Copy link

abdhx commented Mar 5, 2025

@tgckpg
don't you just need an "ssl":True inside your sentinel_kwargs dict?

@tgckpg
Copy link

tgckpg commented Mar 5, 2025

@tgckpg don't you just need an "ssl":True inside your sentinel_kwargs dict?

IIRC the current behavior is this ssl: True only refers to the connection to the sentinel instances. Not the redis instances returned by sentinels. So this SentinelManagedSSLConnection thingy works around that.

Wait I misread your comment. The ssl_keyfiles, etc are for my k8s deployments. That part is optional, ymmv.

@Manhar0911
Copy link

@tgckpg using SentinelManagedSSLConnection class as you mentioned above, getting this error

WARNING:redis-test:⚠️ Redis client initialized, but ping failed: No master found for 'master' : <redis.client.Redis(<redis.connection.ConnectionPool(<redis.connection.Connection(host=sentinel.redis-ptdev5-ns.svc.cluster.local,port=26379,db=0)>)>)> - ConnectionError("Error while reading from sentinel.redis-ptdev5-ns.svc.cluster.local:26379 : (104, 'Connection reset by peer')")

Can you please suggest what i may be missing here.

class SentinelManagedSSLConnection(SentinelManagedConnection, SSLConnection):
    def __init__(self, *args, **kwargs):
        kwargs.pop("ssl", None) 
        super().__init__(*args, **kwargs) 

use_tls = os.getenv("REDIS_USE_TLS", "true").lower() == "true"
cacert_path = os.getenv("REDIS_CACERT_PATH", "/etc/rediscert/ca.crt")
sentinel_addresses_raw = os.getenv("REDIS_SENTINEL_ADDRESSES", "sentinel.redis-ptdev5-ns.svc.cluster.local:26379")
sentinel_master_name = os.getenv("REDIS_SENTINEL_MASTER_NAME", "master")
read_timeout = int(os.getenv("REDIS_READ_TIMEOUT", "200")) / 1000

sentinel_addrs = []
for addr in sentinel_addresses_raw.strip("[]").split(","):
    addr = addr.strip()
    if addr:
        if ':' in addr:
            host, port = addr.rsplit(':', 1)
            sentinel_addrs.append((host.strip(), int(port.strip())))
        else:
            sentinel_addrs.append((addr, 26379))

try:
    sentinel = Sentinel(
        sentinel_addrs,
        sentinel_kwargs={"socket_connect_timeout": 5.0, "socket_timeout": 5.0},
        connection_class=SentinelManagedSSLConnection if use_tls else None,
        ssl=use_tls,
        ssl_ca_certs=cacert_path if use_tls else None
    )
    redis_client = sentinel.master_for(
        service_name=sentinel_master_name,
        socket_timeout=read_timeout,
    )
    try:
        redis_client.ping()
        logger.info("Successfully connected to Redis via Sentinel.")
    except redis.RedisError as e:
        logger.warning(f"Redis client initialized, but ping failed: {e}")
except Exception as e:
    logger.error(f"Failed to connect to Redis via Sentinel: {e}")
    raise

@tgckpg
Copy link

tgckpg commented Apr 15, 2025

If you are using the redis helm chart from bitnami like I did. Cert auth is probably enabled. If that's the case you should put the key and cert files into the kwargs.

You can also check by exec into your redis pod and run redis-cli --tls -h sentinel-endpoint.cluster.svc.local --cacert CA_FILE --cert CERT_FILE --key KEY_FILE -c PING and see if it works first.

EDIT: Also you haven't put the connection_class and kwargs args in master_for. So yeah this doesn't work.

@Manhar0911
Copy link

Verified the cli command, it works. Added the key and cert files into the kwargs but still getting error. Please help.

❌ Redis connection failed: No master found for 'mymaster' : (104, 'Connection reset by peer')")

sentinel = Sentinel(
    [('sentinel.redis-ptdev5-ns.svc.cluster.local', 26379)],
    sentinel_kwargs={"socket_timeout": 5},
    connection_class=SentinelManagedSSLConnection,
    ssl=use_tls,
    ssl_ca_certs=cacert_path,
    ssl_certfile=cert_path,
    ssl_keyfile=key_path
)

# Get Redis master using Sentinel
try:
    redis_client = sentinel.master_for(
        "master",  # ← confirmed master name
        connection_class=SentinelManagedSSLConnection,
        ssl=True,
        ssl_ca_certs=cacert_path,
        ssl_certfile=cert_path,
        ssl_keyfile=key_path,
        socket_timeout=5
    )
    redis_client.ping()
    print("✅ Connected to Redis via Sentinel!")
except redis.RedisError as e:
    print("❌ Redis connection failed:", e)

@tgckpg
Copy link

tgckpg commented Apr 15, 2025

Try this. This is my working code. You should modify it to suite your needs.

redis==3.5.3

from django.conf import settings
from redis.sentinel import Sentinel
k = settings.SESSION_REDIS_SENTINEL_KWARGS
s = Sentinel( settings.SESSION_REDIS_SENTINEL_LIST,
    socket_timeout=30,
    retry_on_timeout=30,
    db=getattr(settings, 'SESSION_REDIS_DB', 0),
    password=getattr(settings, 'SESSION_REDIS_PASSWORD', None),
    sentinel_kwargs=k,
)

from website.injections.redis_sentinel import SentinelManagedSSLConnection
m = s.master_for(settings.SESSION_REDIS_SENTINEL_MASTER_ALIAS, connection_class=SentinelManagedSSLConnection, **k)
m.ping()

@Manhar0911
Copy link

This code worked. Thanks @tgckpg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants