allow redis connections over TLS #1687
Conversation
| @@ -6,4 +6,7 @@ REDIS_PORT=2002 | |||
|
|
|||
| LOG_LEVEL=DEBUG | |||
|
|
|||
|
|
|||
| REDIS_TLS_ENABLED=false | |||
There was a problem hiding this comment.
We don't need these configs here if not used.
Also please add these conifgs in the documentation so it will be easy to find out for other people also.
|
|
||
| if options.Password == "" { | ||
| options.Password = redisConfig.Password | ||
| if caCert := c.Get("REDIS_TLS_CA_CERT"); caCert != "" { |
There was a problem hiding this comment.
@Deeptanshu-sankhwar The Redis TLS implementation had a critical issue in the getRedisConfig function that prevented proper certificate loading. The code was treating certificate file paths as if they contained the certificate content directly, rather than reading the files. I will recommend to update the certificate handling to properly read file content before passing to certificate functions.
There was a problem hiding this comment.
This matches a feedback I made yesterday and I forgot to send
| // REDIS_TLS_KEY: PEM-encoded client private key (string or file path) | ||
| // | ||
| // If TLS is enabled, the function sets up the TLS config for the Redis client. | ||
| func getRedisConfig(c config.Config, logger datasource.Logger) *Config { |
There was a problem hiding this comment.
Why not updating the signature to something like
| func getRedisConfig(c config.Config, logger datasource.Logger) *Config { | |
| func getRedisConfig(c config.Config) (*Config, error) { |
I would prefer log to be in the caller, also I'm unsure why the code continues on error in the implementation in this PR.
I mean when code is facing this
logger.Errorf("failed to append CA cert to pool")
logger.Errorf("failed to load client cert/key pair: %v", err)
It should return an error, no?
Add TLS connection to Redis
Closes #1685
Description:
Checklist:
goimportandgolangci-lint.