Skip to content
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

Add config sentinel auth-pass to entrypoint-sentinel.sh #48

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hoanbc
Copy link

@hoanbc hoanbc commented Sep 25, 2023

Add config "sentinel auth-pass" to entrypoint-sentinel.sh

Signed-off-by: hoanbc <hoanbc@outlook.com>
@shubham-cmyk
Copy link
Member

@hoanbc

I want to discuss this.

As per documentation.
https://redis.io/docs/management/sentinel/

In order for Sentinels to connect to Redis server instances when they are configured with requirepass, the Sentinel configuration must include the sentinel auth-pass directive, in the format:

sentinel auth-pass <master-name> <password>

So it is used when the redis server is password protected and sentinel wants to connect to it.

But if you want to set password on the.

Sentinel only you have to pass : requirepass "your_password_here"

Since the same env variable is used here. i.e.( REDIS_PASSWORD )

            echo requirepass "${REDIS_PASSWORD}"
	    echo sentinel auth-pass "${MASTER_GROUP_NAME} ${REDIS_PASSWORD}"

That means the now sentinel and redis servers can't have different passwords.

I would like you to somehow. Change the variable REDIS_PASSWORD > SENTINEL_PASSWORD
Over here to make that clear, but that could also require the change in the redis-operator code.

Here : https://github.com/OT-CONTAINER-KIT/redis-operator/blob/master/k8sutils/statefulset.go#L610

Check the role as done here : https://github.com/OT-CONTAINER-KIT/redis-operator/blob/master/k8sutils/statefulset.go#L568
and provide these 2 variables.

i.e. REDIS_PASSWORD & SENTINEL_PASSWORD

@shubham-cmyk
Copy link
Member

@drivebyer
Does this idea looks nice. ❓

@drivebyer
Copy link
Collaborator

drivebyer commented Sep 25, 2023

@drivebyer Does this idea looks nice. ❓

LGTM, We should let user set different password between redis and sentinel.

These PR seem fix a bug when replication password enabled.

@hoanbc
Copy link
Author

hoanbc commented Sep 25, 2023

i agree, redis and sentinel should have different password for security compliance.

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

Successfully merging this pull request may close these issues.

3 participants