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: pass redis SSL config to connection pool #5220

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

Conversation

tarvip
Copy link

@tarvip tarvip commented Oct 31, 2024

What this PR does

This fix will pass SSL config properly to redis connection pool.
django-redis passes CONNECTION_POOL_KWARGS and not CONNECTION_POOL_CLASS_KWARGS to connection pool class, related django-redis code can be seen here.

Also, without this fix other settings max_connections and timeout are also not passed to connection pool.

I had issues with external Redis with in-transit encryption enabled using self-signed certs (Google managed Redis).
After changing this I can properly pass self-signed CA cert to redis client using REDIS_SSL_CA_CERTS env variable.

Which issue(s) this PR closes

There is no issue created for this problem.

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

@tarvip tarvip requested a review from a team October 31, 2024 05:54
@CLAassistant
Copy link

CLAassistant commented Oct 31, 2024

CLA assistant check
All committers have signed the CLA.

@tarvip tarvip force-pushed the redis-cache-ssl-fix branch from d59c342 to 3535fca Compare November 10, 2024 15:40
@tarvip tarvip requested a review from a team as a code owner November 10, 2024 15:40
@JNKielmann
Copy link

We are running into the same problem trying to use oncall with a Redis on GCP with in-transit encryption enabled.

@tarvip tarvip force-pushed the redis-cache-ssl-fix branch from 3535fca to ba679eb Compare November 27, 2024 11:28
@tarvip
Copy link
Author

tarvip commented Nov 27, 2024

We are running into the same problem trying to use oncall with a Redis on GCP with in-transit encryption enabled.

We have Grafana OnCall deployed using a Helm chart. As a workaround, I added the following env variable:

env:
  - name: REDIS_URI
    value: "$(REDIS_PROTOCOL)://$(REDIS_USERNAME):$(REDIS_PASSWORD)@$(REDIS_HOST):$(REDIS_PORT)/$(REDIS_DATABASE)?ssl_ca_certs=/mnt/redis-tls/server_ca.pem"

Redis CA cert is mounted to /mnt/redis-tls/server_ca.pem.

It would be nice to get this merged. Created this PR almost a month ago, no feedback so far.

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