Redis In-Transit Encryption (TLS) and password support

[Vlaaaaaaad]

When using Redis for peer discovery, if Redis has in-transit encryption enabled, Samproxy will fail with an i/o timeout:

time="..." level=error msg="registration failed" err="read tcp 172.16.145.223:44362->172.16.4.75:6379: i/o timeout" name="http://ip-000-00-000-000.eu-west-1.compute.internal:8081" timeoutSec=10
time="..." level=error msg="failed to register self with peer store" error="read tcp 172.16.145.223:44362->172.16.4.75:6379: i/o timeout"

This is due to the Redis client used not having the option enabled: https://github.com/honeycombio/samproxy/blob/7ad0d4372207be9e09d43678d5a7549a76e0ea8b/internal/peer/redis.go#L60-L65

As per Redislabs' "Redis Enterprise and Go" which also uses github.com/garyburd/redigo/redis, this requires specific parameters to be sent when initializing the client:

conn, err := redis.Dial("tcp","<endpoint>:<port>",
    redis.DialPassword("<password>"), // password support would be nice too!
    redis.DialTLSConfig(&tlsConfig),
    redis.DialTLSSkipVerify(true),
    redis.DialUseTLS(true))

Relevant links:

• AWS Elasticache for Redis In-Transit Encryption (TLS) docs: https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/in-transit-encryption.html
• Redislabs' "Redis Enterprise and Go": https://redislabs.com/lp/redis-enterprise-go/

[martin308]

@Vlaaaaaaad thanks for bringing this up! I've added it to our backlog for our current period of work on samproxy so that we can prioritize accordingly

[leonardo7w]

hi! I created a small PR for this with a very basic configuration for TLS and a password here. I tested it running a kubernetes service with multiple instances of refinery using Docker desktop and my redis instance was in Azure, and it was working fine. Unfortunately, I wasn't able to test the AWS redis cluster since it can only be accessed from the VPC and requires a bit more setup to run. Let me know your thoughts!

[Vlaaaaaaad]

I can confirm this seems to work fine in AWS using ElastiCache too!

When the next release is cut (https://github.com/honeycombio/refinery/pull/201 was already merged) I think this issue can be closed.

[MikeGoldsmith]

Closing as #201 was merged and part of the v0.15.0 release.