Skip to content

Allow standalone Redis in auth server storage#4994

Merged
reyortiz3 merged 18 commits into
mainfrom
feat/authserver-standalone-redis
May 1, 2026
Merged

Allow standalone Redis in auth server storage#4994
reyortiz3 merged 18 commits into
mainfrom
feat/authserver-standalone-redis

Conversation

@reyortiz3
Copy link
Copy Markdown
Contributor

@reyortiz3 reyortiz3 commented Apr 21, 2026

Summary

  • The auth server storage previously required Redis Sentinel, which is not available on managed Redis services such as GCP Memorystore, AWS ElastiCache, and Azure Cache for Redis — these services expose a single endpoint and manage HA internally.
  • Adds a mutually exclusive Addr field (standalone mode) alongside the existing SentinelConfig (Sentinel mode). Exactly one must be set. Mirrors the pattern already used in pkg/transport/session/redis_config.go.
  • Propagates standalone support through all layers: RedisConfig/RedisRunConfig in storage, convertRedisRunConfig in the embedded auth server runner, RedisStorageConfig CRD type with CEL XValidation, and buildStorageRunConfig in the operator controller.
  • Makes usernameSecretRef optional in the CRD and removes the empty-username validation in validateConfig. When username is omitted, go-redis v9 sends HELLO with "default" as the username and falls back to legacy AUTH <password> for servers that predate HELLO. This is required for managed Redis tiers that do not expose ACL users (GCP Memorystore Basic/Standard HA, Azure Cache for Redis).
  • Updates docs/redis-storage.md and docs/arch/11-auth-server-storage.md to document both connection modes, the optional username, TLS CA cert guidance for managed services (e.g. gcloud redis instances get-server-ca-certs), and the known limitation that Redis Cluster mode is not supported (tracked in Add Redis Cluster mode support to auth server storage #5010).

Fixes #4990

Type of change

  • New feature

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

New tests added in the standalone addr PR:

  • TestRedisConfig_Validation extended with standalone/conflict/neither cases
  • TestNewRedisStorage_Standalone_ConnectionFailure — unreachable addr returns error
  • TestNewRedisStorage_Standalone_WithMiniredis — successful standalone connection via miniredis
  • TestConvertRedisRunConfig extended with standalone happy path, both-set, neither-set cases
  • TestBuildStorageRunConfig extended with standalone addr case and both-set error case

New tests added in the optional-username commit:

  • TestConvertRedisRunConfig_WithEnvVars: "empty UsernameEnvVar uses legacy password-only auth" — verifies empty UsernameEnvVar resolves to empty username without error
  • TestBuildStorageRunConfig: "Redis standalone with password-only auth omits UsernameEnvVar" — verifies operator omits UsernameEnvVar from RunConfig when usernameSecretRef is absent

API Compatibility

This PR adds an optional addr field to RedisStorageConfig, makes sentinelConfig optional (it was previously implicitly required), and makes usernameSecretRef optional in RedisACLUserConfig. All changes are additive and backward compatible — existing Sentinel-based EmbeddedAuthServerConfig resources with a username are unchanged.

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File Change
pkg/authserver/storage/redis.go Add Addr to RedisConfig; update validateConfig (XOR, drop empty-username check); branch NewRedisStorage on standalone vs Sentinel
pkg/authserver/storage/config.go Add Addr to RedisRunConfig
pkg/authserver/runner/embeddedauthserver.go Update convertRedisRunConfig to handle Addr XOR SentinelConfig; make convertRedisACLConfig skip username resolution when UsernameEnvVar is empty
cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go Add Addr to RedisStorageConfig; make SentinelConfig optional; add CEL XValidation; make usernameSecretRef optional
cmd/thv-operator/pkg/controllerutil/authserver.go Update buildStorageRunConfig to handle standalone path; only populate UsernameEnvVar when usernameSecretRef is present
docs/redis-storage.md Rewritten to document standalone and Sentinel modes, optional username, TLS CA cert setup, and cluster mode limitation
docs/arch/11-auth-server-storage.md Updated to reflect standalone support and optional username
docs/server/swagger.json/yaml Regenerated to include addr field
CRD YAMLs, crd-api.md Regenerated

Does this introduce a user-facing change?

Yes. Two user-facing changes:

  1. Operators can now configure auth server Redis storage with a single addr: "host:port" field instead of requiring a Sentinel deployment. Existing Sentinel configurations are unchanged.
  2. usernameSecretRef is now optional. Operators targeting managed Redis tiers without ACL support (GCP Memorystore Basic/Standard HA, Azure Cache for Redis) can omit it and rely on password-only AUTH.

Implementation plan

Approved implementation plan

See docs/superpowers/plans/2026-04-21-authserver-standalone-redis.md.

Special notes for reviewers

  • The CEL XValidation rule uses self.addr.size() > 0 rather than self.addr != '' because the gci formatter in golangci-lint converts single-quoted CEL string literals to Unicode curly quotes, which corrupts the generated CRD YAML. The size() > 0 form is semantically equivalent and linter-safe.
  • SentinelTLS is only populated when SentinelConfig is non-nil, matching the standalone path that has no separate dialer TLS config.
  • go-redis v9 Hello() substitutes "default" for an empty username (see commands.go:363), so HELLO 2 AUTH default <password> is sent for password-only connections. Servers that reject HELLO fall back to legacy AUTH <password> automatically.
  • Redis Cluster mode (GCP Memorystore Cluster, ElastiCache Serverless) is out of scope and tracked in Add Redis Cluster mode support to auth server storage #5010.

Generated with Claude Code

reyortiz3 and others added 6 commits April 21, 2026 16:33
Add Addr field to RedisConfig for standalone mode (e.g., GCP Memorystore,
AWS ElastiCache), making it mutually exclusive with SentinelConfig.
Update validateConfig to enforce the mutual exclusion and require exactly
one connection mode. Update NewRedisStorage to branch between
redis.NewClient (standalone) and redis.NewFailoverClient (sentinel).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…te converter

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add an Addr field for standalone Redis mode and make SentinelConfig
optional. A CEL XValidation rule enforces that exactly one of addr
(standalone) or sentinelConfig (Sentinel) is set. Regenerate deepcopy,
CRD manifests, and API reference docs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nConfig

Replace the Sentinel-only requirement with support for either a standalone
addr or a SentinelConfig, matching the new RedisStorageConfig.Addr field
added to the CRD in earlier tasks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix CEL XValidation rule Unicode curly quote bug in RedisStorageConfig
  (replaced U+201D right double quotation mark with ASCII single quotes)
- Add mutual exclusivity guard when both addr and sentinelConfig are set
  in buildStorageRunConfig
- Add test case covering the addr+sentinelConfig conflict error path
- Update RedisStorage doc comment to describe both standalone and Sentinel modes
- Use t.Cleanup instead of defer in withRedisStorage parallel test helper
- Use non-default ACL username in TestNewRedisStorage_Standalone_WithMiniredis
- Regenerate CRD manifests and API docs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d, tests

- Fix CEL XValidation rule: replace string literal comparison with size() > 0 to avoid gci formatter rewriting quote characters
- Add mutual-exclusivity guard in buildStorageRunConfig for both addr AND sentinelConfig set
- Add test case for "both addr and sentinelConfig set" error in buildStorageRunConfig
- Fix doc comment on RedisStorage to mention both standalone and Sentinel modes
- Switch withRedisStorage cleanup from defer to t.Cleanup for parallel test safety
- Use non-default username in standalone miniredis test
- Regenerate CRD manifests and API docs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the size/L Large PR: 600-999 lines changed label Apr 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 89.71963% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.78%. Comparing base (68f4c2f) to head (f9489a4).
⚠️ Report is 57 commits behind head on main.

Files with missing lines Patch % Lines
pkg/authserver/runner/embeddedauthserver.go 88.37% 4 Missing and 1 partial ⚠️
pkg/authserver/storage/redis.go 88.88% 2 Missing and 2 partials ⚠️
cmd/thv-operator/pkg/controllerutil/authserver.go 92.85% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4994      +/-   ##
==========================================
+ Coverage   69.76%   69.78%   +0.01%     
==========================================
  Files         560      560              
  Lines       56475    56513      +38     
==========================================
+ Hits        39399    39436      +37     
  Misses      14053    14053              
- Partials     3023     3024       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 21, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@reyortiz3 reyortiz3 requested a review from amirejaz as a code owner April 21, 2026 21:56
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 21, 2026
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 21, 2026
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 21, 2026
@github-actions github-actions Bot removed the size/L Large PR: 600-999 lines changed label Apr 22, 2026
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 22, 2026
@reyortiz3
Copy link
Copy Markdown
Contributor Author

@jhrozek comment addressed

@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 22, 2026
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 22, 2026
@jhrozek
Copy link
Copy Markdown
Contributor

jhrozek commented Apr 23, 2026

I think you forgot to push the new code @reyortiz3 ? I only see main merges.

Redis services such as GCP Memorystore Basic/Standard HA and Azure Cache
for Redis use password-only AUTH and do not expose ACL users. The previous
implementation required a non-empty username, which forced go-redis to send
AUTH <username> <password> and caused authentication failures on those tiers.

When username is omitted, go-redis v9 sends HELLO with "default" as the
username (falling back to legacy AUTH <password> for servers that predate
HELLO), which is the correct form for password-only managed Redis.

- Remove the empty-username validation in storage.validateConfig
- Skip username env var resolution in convertRedisACLConfig when
  UsernameEnvVar is empty, producing an empty ACLUserConfig.Username
- Mark UsernameSecretRef optional in RedisACLUserConfig CRD type and
  only populate UsernameEnvVar in the RunConfig when the ref is present
- Update docs to reflect standalone mode support, optional username,
  TLS CA cert guidance for managed services, and note that Redis Cluster
  mode (GCP Memorystore Cluster, ElastiCache Serverless) is not supported

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@reyortiz3 reyortiz3 requested a review from rdimitrov as a code owner April 23, 2026 18:53
@github-actions github-actions Bot removed the size/L Large PR: 600-999 lines changed label Apr 23, 2026
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 23, 2026
@reyortiz3
Copy link
Copy Markdown
Contributor Author

@jhrozek ya, sorry the changes were not pushed, should be updated now/

Runs task operator-manifests and task crdref-gen to pick up the
optional-username changes: removes usernameSecretRef from the required
list in MCPExternalAuthConfig and VirtualMCPServer CRDs, updates the
field description, and refreshes crd-api.md accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 23, 2026
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Apr 24, 2026
Copy link
Copy Markdown
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

One small item worth a follow-up: the default timeouts (3s read/write, 5s dial) are likely too short for managed-service failover windows — Memorystore and ElastiCache typically take 15–30s to complete a failover, so in-flight ops will error during that window. Worth either raising the defaults or adding a note in the user guide recommending higher values for managed deployments.

@reyortiz3 reyortiz3 merged commit dc74588 into main May 1, 2026
45 checks passed
@reyortiz3 reyortiz3 deleted the feat/authserver-standalone-redis branch May 1, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support standalone Redis in auth server storage (no Sentinel required)

2 participants