redis: auth support

[msukalski]

Signed-off-by: Mitch Sukalski mitch.sukalski@workday.com

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description:

Authentication support (a la Redis AUTH commands) has been added for both downstream clients and upstream Redis servers on a per-cluster basis.

A new redis_proxy filter option, downstream_auth_password, can be configured, and no redis commands, including the locally processed PING, will be executed unless a successful authentication command has been issued by the downstream client. Various errors are returned when the authentication command is invalid, or has been issued when the password has not been set; these errors are the same as the errors returned by a Redis server in the same circumstances.

Separately from downstream authentication, the envoy admin can configure an upstream server authentication password on a per-cluster basis. Upstream server connections are transparently authenticated upon connection, if the password has been configured.

Risk Level: low

Testing: unit tests, integration tests, and manual testing against a Redis cluster

Docs Changes:

Changes to version_history.rst under version "1.11.0 (Pending)", and the addition of AUTH support and explanatory text to redis.rst under "Features of Envoy Redis", Supported commands, and Failure modes

Release Notes:

• redis: added auth support :ref:downstream_auth_password <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.downstream_auth_password> for downstream client authentication, and :ref:auth_password <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProtocolOptions.auth_password> to configure authentication passwords for upstream server clusters.

[Optional Fixes #Issue]
[Optional Deprecated:]

[msukalski]

@mattklein123 @HenryYYang @FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg @maximebedard I decided to create a PR so folks can review and provide feedback, as they desire.

[HenryYYang]

I'm not sure if we should support the auth command, if we do, do we need to update the password for all existing connections? A related question, how do we intend to dealt with password rotation?

[msukalski]

/test/extensions/filters/network/kafka:serialization_test failed, but passes locally with standard debug flags...

[msukalski]

@HenryYYang and @FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg -- I've added auth support to the discovery session for the new RedisCluster. There's also an integration test for this support as well.

[FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg]

LGTM. As discussed key rotation is not supported by Redis anyway, and Redis 6 "changes things", so this works fine.

[FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg]

👍

[FAYiEKcbD0XFqF2QK2E4viAHg8rMm2VbjYKdjTg]

@mattklein123 any thoughts?

[msukalski]

@mattklein123 Can I get someone to approve this PR so it can be merged? The current PR has addressed all review feedback from folks...

[mattklein123]

Will take a look.

[mattklein123]

Very cool. I skimmed and LGTM assuming it passes @HenryYYang muster. A few small things that caught my eye. Thank you!

/wait

[mattklein123]

Thanks. Few more comments.

/wait

[msukalski]

It looks like the coverage failure is due to "Code coverage 97.4 is lower than limit of 97.5". None of my code modifications do not have code coverage, so it looks like I'll have to artificially cover non-functional interface methods for RedisActiveHealthCheckSession, RedisDiscoveryClient, and RedisDiscoverySession to get line coverage back up to the limit.

[mattklein123]

You can add coverage of more redis code, or look https://s3.amazonaws.com/lyft-envoy/coverage/report-master/coverage.html for other coverage to add. There is quite a bit of legit coverage missing elsewhere.

/wait

[HenryYYang]

LGTM

[msukalski]

ci/circleci:docs check failed due to "checking consistency... /source/generated/rst/configuration/best_practices/edge.rst: WARNING: document isn't included in any toctree" -- I didn't add this file, so I don't know how it got merged into master passing this check. Sigh.

[mattklein123]

@msukalski it was an accidental force merge. It will get fixed in the morning. cc @htuch

/wait

[mattklein123]

Great work!