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

fix: support TLS connections to Redis #1285

Merged
merged 2 commits into from
Aug 16, 2024
Merged

Conversation

VinozzZ
Copy link
Contributor

@VinozzZ VinozzZ commented Aug 16, 2024

Which problem is this PR solving?

Currently, we are not setting the tls config for go-redis even through we do have UseTLS configuration option.

Short description of the changes

  • log the pubsub publish error
  • configure redis client tls

@VinozzZ VinozzZ requested a review from a team as a code owner August 16, 2024 19:13
@VinozzZ VinozzZ changed the base branch from main to v2.7_work_branch August 16, 2024 19:13
@robbkidd robbkidd changed the title fix: configure redis client with tls when UseTLS is enabled fix: support TLS connections to Redis Aug 16, 2024
Copy link
Contributor

@kentquirk kentquirk left a comment

Choose a reason for hiding this comment

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

This has been tested and shown to work.

Copy link
Member

@robbkidd robbkidd left a comment

Choose a reason for hiding this comment

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

Tested in an isolated docker-compose setup with honeycombio/refinery:2.7.2-dev.1 Refinery image and a Redis running with TLS enabled.

encrypted

@VinozzZ VinozzZ merged commit 4d30964 into v2.7_work_branch Aug 16, 2024
8 checks passed
@VinozzZ VinozzZ deleted the yingrong.fix_tls branch August 16, 2024 21:01
VinozzZ added a commit that referenced this pull request Aug 19, 2024
<!--
Thank you for contributing to the project! 💜
Please make sure to:
- Chat with us first if this is a big change
  - Open a new issue (or comment on an existing one)
- We want to make sure you don't spend time implementing something we
might have to say No to
- Add unit tests
- Mention any relevant issues in the PR description (e.g. "Fixes #123")

Please see our [OSS process
document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#)
to get an idea of how we operate.
-->

Currently, we are not setting the tls config for go-redis even through
we do have `UseTLS` configuration option.

- log the pubsub publish error
- configure redis client tls
VinozzZ added a commit that referenced this pull request Aug 19, 2024
<!--
Thank you for contributing to the project! 💜
Please make sure to:
- Chat with us first if this is a big change
  - Open a new issue (or comment on an existing one)
- We want to make sure you don't spend time implementing something we
might have to say No to
- Add unit tests
- Mention any relevant issues in the PR description (e.g. "Fixes #123")

Please see our [OSS process
document](https://github.com/honeycombio/home/blob/main/honeycomb-oss-lifecycle-and-practices.md#)
to get an idea of how we operate.
-->

Currently, we are not setting the tls config for go-redis even through
we do have `UseTLS` configuration option.

- log the pubsub publish error
- configure redis client tls
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