envoyproxy/envoy

Redis Health Checks do not support ACL auth_username

Open

#11,736 opened on Jun 24, 2020

View on GitHub
 (0 comments) (0 reactions) (0 assignees)C++ (5,373 forks)batch import
area/redishelp wanted

Repository metrics

Stars
 (27,997 stars)
PR merge metrics
 (Avg merge 8d) (378 merged PRs in 30d)

Description

Title: Redis Health Checks do not support the Redis 6 ACL auth_username parameter

Description:

The only thing I was not able to handle in the previous PR #11408 is supporting auth_username for health checks. This is ok for our initial use case as we can grant the default user +ping permissions. That said, others might like this feature and I am happy to work on it.

https://github.com/envoyproxy/envoy/blob/master/source/extensions/health_checkers/redis/redis.cc#L4 is still using API V2 configuration for health checks and so I was not able to add auth_username to the proto or the impl.

I propose adding a V3 API for redis health checks that supports auth_username and make the associated changes, but I wanted to check with you on this approach before starting the work.

Contributor guide

Research direction
Study the existing Redis health checker implementation and the previous PR #11408. Understand how auth username is handled in other parts of Envoy and how to add it to the health check proto. Explore the V3 API migration pattern used elsewhere in the codebase.
Tech stack
cpp
Domain
backend
Issue type
Feature
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-2 days
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
C++Redisprotobuf
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
45