Skip to content
This repository was archived by the owner on May 23, 2024. It is now read-only.

feat: allow to configure authentication for sentinel#104

Merged
christianhueserhzdr merged 1 commit intohifis-net:mainfrom
tobiashuste:103-allow-to-configure-a-password-for-redis-sentinel
Jul 28, 2023
Merged

feat: allow to configure authentication for sentinel#104
christianhueserhzdr merged 1 commit intohifis-net:mainfrom
tobiashuste:103-allow-to-configure-a-password-for-redis-sentinel

Conversation

@tobiashuste
Copy link
Copy Markdown
Member

@tobiashuste tobiashuste commented Jul 28, 2023 •
edited
Loading

Closes #103

@tobiashuste tobiashuste force-pushed the 103-allow-to-configure-a-password-for-redis-sentinel branch from b0bc751 to 67ea194 Compare July 28, 2023 07:07
@tobiashuste tobiashuste self-assigned this Jul 28, 2023
christianhueserhzdr
christianhueserhzdr suggested changes Jul 28, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@christianhueserhzdr christianhueserhzdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks fine, thanks. But the "t" in "sentinel" that needs to be shifted two times to the left. ;-) For some reason sentinel is not configured during the converge stage and no change is done. But it should be, because we are changing the config file, right?

Comment thread README.md Outdated
Password for Redis Sentinel. This is unset by default.

```yaml
redis_senintel_password: 'changeme'
Copy link
Copy Markdown
Member

@christianhueserhzdr christianhueserhzdr Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
redis_senintel_password: 'changeme'
redis_sentinel_password: 'changeme'

Copy link
Copy Markdown
Member Author

@tobiashuste tobiashuste Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least I was consistent 🙈

Comment thread molecule/default/molecule.yml Outdated
hosts:
all:
vars:
redis_senintel_password: "123456"
Copy link
Copy Markdown
Member

@christianhueserhzdr christianhueserhzdr Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
redis_senintel_password: "123456"
redis_sentinel_password: "123456"

Comment thread templates/sentinel.conf.j2 Outdated
Comment on lines +15 to +16
{% if (redis_senintel_password is defined) and (redis_senintel_password|length > 0) %}
requirepass "{{ redis_senintel_password }}"
Copy link
Copy Markdown
Member

@christianhueserhzdr christianhueserhzdr Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% if (redis_senintel_password is defined) and (redis_senintel_password|length > 0) %}
requirepass "{{ redis_senintel_password }}"
{% if (redis_sentinel_password is defined) and (redis_sentinel_password|length > 0) %}
requirepass "{{ redis_sentinel_password }}"

@tobiashuste
Copy link
Copy Markdown
Member Author

tobiashuste commented Jul 28, 2023

This change looks fine, thanks. But the "t" in "sentinel" that needs to be shifted two times to the left. ;-) For some reason sentinel is not configured during the converge stage and no change is done. But it should be, because we are changing the config file, right?

There is an issue about that: #6

@tobiashuste tobiashuste force-pushed the 103-allow-to-configure-a-password-for-redis-sentinel branch from 67ea194 to 5f365c3 Compare July 28, 2023 07:43
christianhueserhzdr
christianhueserhzdr approved these changes Jul 28, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@christianhueserhzdr christianhueserhzdr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all fine now, thanks! I'll approve and merge this PR now.

@christianhueserhzdr christianhueserhzdr merged commit 5d87fa1 into hifis-net:main Jul 28, 2023
@tobiashuste tobiashuste deleted the 103-allow-to-configure-a-password-for-redis-sentinel branch July 28, 2023 07:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@christianhueserhzdr christianhueserhzdr christianhueserhzdr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tobiashuste tobiashuste

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow to configure a password for Redis Sentinel

2 participants

@tobiashuste @christianhueserhzdr
⚡