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

feat: Add SSL Support for Redis and Redis Sentinel Connections #1605

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jopemachine
Copy link
Member

@jopemachine jopemachine commented Oct 6, 2023

A follow-up to #1586.

Add support for SSL/TLS encryption for Redis and Redis sentinel connections. It introduces a new option --enable-ssl in the install-dev.sh script, modifies the configuration files and the docker-compose files, and updates the redis_helper module and the related files.

@jopemachine jopemachine changed the title Support Redis SSL feat: Support Redis SSL Oct 6, 2023
@github-actions github-actions bot added comp:manager Related to Manager component size:L 100~500 LoC labels Oct 6, 2023
@jopemachine jopemachine changed the title feat: Support Redis SSL feat: Add SSL Support for Redis and Redis Sentinel Connections Oct 6, 2023
@jopemachine jopemachine force-pushed the feat/add-tls-support-to-redis branch from 0fdbaf3 to 9644099 Compare October 6, 2023 07:17
@jopemachine
Copy link
Member Author

jopemachine commented Oct 6, 2023

This PR stores the certificate files in the ./volumes/redis-data directory, so make sure you have sufficient permissions on that path. Otherwise, you will get Error 13 when connecting Redis (permission denied)

@jopemachine jopemachine marked this pull request as ready for review October 6, 2023 07:39
@achimnol
Copy link
Member

@jopemachine Due to #1620, it requires merge-conflict resolution. Please update the PR.

@achimnol achimnol added this to the 23.09 milestone Oct 16, 2023
Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

This PR stores the certificate files in the ./volumes/redis-data directory, so make sure you have sufficient permissions on that path. Otherwise, you will get Error 13 when connecting Redis (permission denied)

They should be moved to the fixtures directory. volumes should be left as owned by the halfstack containers and the container engine. Do not put anything there that are not created and managed by halfstack containers.

@jopemachine jopemachine force-pushed the feat/add-tls-support-to-redis branch from 9644099 to ab0bece Compare October 17, 2023 02:04
@jopemachine
Copy link
Member Author

They should be moved to the fixtures directory. volumes should be left as owned by the halfstack containers and the container engine. Do not put anything there that are not created and managed by halfstack containers.

Thanks for letting me know! I updated the PR.

@achimnol
Copy link
Member

achimnol commented Nov 3, 2023

Currently we are considering rewriting of redis-py entirely.
I'm markding this PR as "on-hold".

@achimnol achimnol added the action:on hold Hold it. Wait for the restart. label Nov 3, 2023
@jopemachine jopemachine marked this pull request as draft December 23, 2024 02:47
@jopemachine
Copy link
Member Author

I will mark this PR as draft cause this is on hold

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:on hold Hold it. Wait for the restart. comp:manager Related to Manager component size:L 100~500 LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants