Skip to content

fix(collector): use Redis username for ACL authentication instead of client name - #4360

Open
zjncs wants to merge 1 commit into
apache:masterfrom
zjncs:fix-redis-username-auth
Open

zjncs wants to merge 1 commit into
apache:masterfrom
zjncs:fix-redis-username-auth

Conversation

@zjncs

@zjncs zjncs commented Sep 4, 2026

Copy link
Copy Markdown

What's changed

The Redis collector passed the configured username to RedisURI.Builder.withClientName, which only sets the informational CLIENT SETNAME sent after connecting and plays no role in authentication (lettuce 6.8.2 resolves the username exclusively from RedisURI credentials).

As a result, with a Redis 6+ ACL user (e.g. ACL SETUSER monitor on >secretpass ~* +@read) and a monitor configured with username=monitor, password=secretpass (both exposed by app-redis.yml), lettuce authenticated as the default user and the collection failed with WRONGPASS/NOAUTH — the username setting never worked.

Fix: use withAuthentication(username, password) so the username and password are applied as credentials. An ACL user without a password authenticates with an empty one (AUTH <username> ""); password-only configuration keeps the previous withPassword behavior.

Checklist

  • I have read the Contributing Guidelines.
  • I have added unit tests for the change (RedisCommonCollectImplTest — the module previously had no Redis collector tests).
  • All tests pass locally (mvn -pl hertzbeat-collector/hertzbeat-collector-basic -am test: 273 tests, 0 failures).

Verification

  • fail-before: redisUriShouldUseUsernameForAuthentication fails on master — expected: <monitor> but was: <null> (RedisURI.getUsername() is null because the username went to CLIENT NAME)
  • pass-after: both new tests pass; full module suite passes (273 tests, 0 failures, 4 pre-existing skips)
  • Verified against lettuce-core 6.8.2.RELEASE (the version resolved by the build): withClientName sets only the client name, withAuthentication(String, char[]) sets the credentials used for AUTH

AI assistance disclosure: the bug analysis, fix and tests in this PR were prepared with the help of an AI coding agent, and were verified locally by the author as described above.

…client name

The Redis collector passed the configured username to
RedisURI.Builder.withClientName, which only sets the informational
CLIENT SETNAME sent after connecting and plays no role in
authentication. Redis 6+ ACL users configured in the monitor template
therefore never authenticated as the configured user; lettuce always
authenticated as the default user and the collection failed with
WRONGPASS/NOAUTH.

Use withAuthentication so the username and password are applied as
credentials. An ACL user without a password authenticates with an empty
one. Password-only configuration keeps the previous behavior.

Signed-off-by: zjncs <18910855655@163.com>
@zjncs
zjncs marked this pull request as ready for review September 4, 2026 12:13
Copilot AI lite review requested due to automatic review settings September 4, 2026 12:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants