Add support for secure redis in redis.clusters

[alexef]

Summary

Hi, it's me again.

I'm trying to figure out how and if we can configure sentry on premises to connect to AWS Elasticcache Redis using encryption (in transit, at rest).

Although the BROKER_URL setting seems to support now rediss:// the redis.clusters setting seems to only respect hosts:

  cluster_options = {
        key: options.pop(key)
        for key in set(options.keys()).intersection(cluster_constructor_option_names)
    }

https://github.com/getsentry/sentry/blob/10f78efef6bab470c6b8c7690a794991f8cfca3b/src/sentry/utils/redis.py#L185

Am I missing something obvious, or this is not yet supported?

The way I would do it is by adding ssl as an acceptable option in https://github.com/getsentry/sentry/blob/master/docker/sentry.conf.py#L98

Motivation

To be able to use secure redis.

Additional Context

It seems like sentry uses rb which supports ssl: https://github.com/getsentry/rb/blob/7df22c9605ba6331f60c6dde249ce4f7aef48694/rb/cluster.py#L136

[BYK]

Ping @getsentry/infrastructure @wedamija for any pointers.

[BYK]

Also ping @joshuarli and @markstory

[joshuarli]

Yeah, we could add it into options and pass it down.

Fun fact, we also use hiredis-py (hiredis wrapper), which upcoming 2.0.0 bumps their hiredis to 1.0, which supports ssl: https://github.com/redis/hiredis/blob/master/CHANGELOG.md#100---2020-08-03

So if you want hiredis-py, secure redis is gonna be blocked on that. We could either move hiredis-py out of OSS sentry, or I could look into upgrading hiredis-py. I was already eyeing 1.0.1 since they build Python 3.8 wheels for it, but it's not enough for that ssl support you want.

[joshuarli]

@BYK If I could get your +1 to move hiredis-py out of here to firstly unblock this, that would be cool. It'd probably lead to regressing on redis performance overall though.

[BYK]

It'd probably lead to regressing on redis performance overall though.

Let's discuss this first and then we can make an informed decision. Thanks a lot for sharing all this information @joshuarli!

[jan-auer]

Please note that Relay also requires access to Redis and does not support SSL connections. It would require linking libssl-dev, which is not a build dependency. After looking into it briefly, It would be possible to link against our vendored version of openssl.