Skip to content

Conversation

@FlashBlack
Copy link

@FlashBlack FlashBlack commented Oct 29, 2025

Q A
Branch? 7.3
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

When getting the master server in redis transport via sentinel, auth was omitted. Used code similar to Cache/Traits/RedisTrait.php#L243
This is still not an ideal solution, and to some extent it duplicates the logic from Symfony/Component/Cache/Traits/RedisTrait.php, so it would be good to unify this in the future.

To cover the integration test I had to tweak the integration services a bit:

  • used one image for all redis services
  • correctly pass authorisation settings

@FlashBlack FlashBlack force-pushed the messenger-redis-sentinel-auth branch from 6053b59 to 74ad55b Compare October 29, 2025 17:23
@nicolas-grekas
Copy link
Member

This is very related to #61783
Can you please have a look there, code and discussion, and let us know if you're ok to continue this?

@symfony symfony deleted a comment from carsonbot Oct 31, 2025
@FlashBlack FlashBlack force-pushed the messenger-redis-sentinel-auth branch from b412d97 to 0748661 Compare November 17, 2025 22:48
@FlashBlack FlashBlack force-pushed the messenger-redis-sentinel-auth branch from 0748661 to 1e663d1 Compare November 17, 2025 23:10
@FlashBlack
Copy link
Author

FlashBlack commented Nov 17, 2025

@nicolas-grekas I'm agreed that's current auth way a bit inconsistent.
and I want to bring this to some kind of unified format, or something close to it.
Moreover, there are some issues with DSN backward compatibility. I left one failed test with the following example:

Connection::fromDsn('redis://password1@localhost/queue', ['auth' => 'password2'], $redis);

which password should be used in this case? (origin expectation was at password2 but why?)

Regarding auth per sentinel instance - it's often recommended to separate master and sentinel auth but all sentinel instances have a single password, which is confirmed by the official documentation - https://redis.io/docs/latest/operate/oss_and_stack/management/sentinel/#sentinel-password-only-authentication and docker image as well - https://hub.docker.com/r/bitnami/redis-sentinel#configuration

In my implementation, I suggest explicitly passing auth for master and sentinel in the options e.g.

REDIS_DSN=redis:?host[localhost:1]&host[localhost:2]&master_auth=123&sentinel_auth=456

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants