Skip to content

fix(operator): forward default Redis password to MCPServer and MCPRemoteProxy - #6679

Merged
reyortiz3 merged 3 commits into
mainfrom
fix/operator-default-redis-password-mcpserver-remoteproxy
Sep 18, 2026
Merged

reyortiz3 merged 3 commits into
mainfrom
fix/operator-default-redis-password-mcpserver-remoteproxy

Conversation

@reyortiz3

Copy link
Copy Markdown
Collaborator

Summary

  • When the operator's global Redis default (operator.defaultRedis / global.redis) is used because spec.sessionStorage is unset, MCPServer and MCPRemoteProxy pods received the default Redis address but never the default password, so they crash-looped against an authenticated Redis/Valkey instance with NOAUTH Authentication required.
  • VirtualMCPServer already handled this correctly by falling back to TOOLHIVE_DEFAULT_REDIS_SECRET_NAME/TOOLHIVE_DEFAULT_REDIS_SECRET_KEY in buildRedisPasswordEnvVar when spec.sessionStorage is nil.
  • This mirrors that same fallback into MCPServerReconciler.buildRedisPasswordEnvVar and buildRedisPasswordEnvVarForRemoteProxy, so address and credential always travel together for the global-default path, while an explicit spec.sessionStorage on any of the three kinds remains authoritative and never falls through to the global default.
  • The password is still only ever injected via secretKeyRef — never copied into the pod spec as plaintext.

Fixes stacklok/stacklok-enterprise-platform#4310

Test plan

  • Added TestMCPServerBuildRedisPasswordEnvVar_GlobalDefault, _NonRedisProviderNotOverriddenByGlobal, and _PasswordlessGlobalDefault (mirrors the existing VirtualMCPServer coverage).
  • Added the same three cases for MCPRemoteProxy as TestBuildRedisPasswordEnvVarForRemoteProxy_GlobalDefault, _NonRedisProviderNotOverriddenByGlobal, _PasswordlessGlobalDefault.
  • go test ./cmd/thv-operator/controllers/... passes.
  • go build ./cmd/thv-operator/... passes.

🤖 Generated with Claude Code

…oteProxy

VirtualMCPServer already fell back to TOOLHIVE_DEFAULT_REDIS_SECRET_NAME/KEY
when spec.sessionStorage is unset, but MCPServer and MCPRemoteProxy only
injected THV_SESSION_REDIS_PASSWORD when an explicit passwordRef was set.
Both still picked up the default Redis address, so pods deployed against an
authenticated global default Redis/Valkey instance connected without a
password and crash-looped with "NOAUTH Authentication required".

Mirror VirtualMCPServer's buildRedisPasswordEnvVar fallback in
MCPServerReconciler.buildRedisPasswordEnvVar and
buildRedisPasswordEnvVarForRemoteProxy so the default address and default
password always travel together, while an explicit spec.sessionStorage
remains authoritative and never falls through to the global default.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Sep 17, 2026
@codecov

codecov Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.16%. Comparing base (158e671) to head (ec2b386).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6679      +/-   ##
==========================================
- Coverage   79.20%   79.16%   -0.04%     
==========================================
  Files         789      789              
  Lines       79028    79054      +26     
==========================================
- Hits        62597    62586      -11     
- Misses      16426    16463      +37     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 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/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 17, 2026
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Sep 17, 2026

@jhrozek jhrozek 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.

Reviewed via Claude Code — small, well-scoped fix that mirrors VirtualMCPServer's existing buildRedisPasswordEnvVar fallback logic exactly for MCPServer and MCPRemoteProxy. Explicit sessionStorage correctly short-circuits and never falls through to the global default; secret is injected via SecretKeyRef, never plaintext. Test coverage mirrors the sibling VirtualMCPServer suite (global-default injection, non-redis-provider not overridden, passwordless-default no-op). No architecture doc updates needed — this brings two controllers in line with a third's already-documented behavior.

@reyortiz3
reyortiz3 merged commit 5a4854a into main Sep 18, 2026
77 of 78 checks passed
@reyortiz3
reyortiz3 deleted the fix/operator-default-redis-password-mcpserver-remoteproxy branch September 18, 2026 13:04
@github-actions github-actions Bot mentioned this pull request Sep 18, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants