Skip to content

Conversation

@jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Dec 3, 2025

The vMCP implementation currently has duplicated logic between the operator and vMCP server. The operator discovers backends, resolves ExternalAuthConfigs, and embeds backend configuration into a ConfigMap. Meanwhile, the vMCP server has its own discovery code but only loads config statically from the ConfigMap at startup.

This duplication creates inconsistency risk, requires pod restarts for backend changes to take effect, and has grown the operator complex with auth resolution logic scattered across components.

This proposal flips responsibilities: the operator becomes a "dumb" infrastructure manager handling only Deployment/Service/RBAC, while vMCP becomes "smart" by running controller-runtime with informers to watch MCPServer/ExternalAuthConfig/Secret resources directly.

Relates-to: #2855

@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Dec 3, 2025
@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.41%. Comparing base (892043f) to head (485eeb9).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2884      +/-   ##
==========================================
- Coverage   56.59%   56.41%   -0.19%     
==========================================
  Files         322      322              
  Lines       31439    31643     +204     
==========================================
+ Hits        17794    17851      +57     
- Misses      12110    12254     +144     
- Partials     1535     1538       +3     

☔ 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/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Dec 4, 2025
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Dec 4, 2025
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Dec 4, 2025
The vMCP implementation currently has duplicated logic between the operator
and vMCP server. The operator discovers backends, resolves
ExternalAuthConfigs, and embeds backend configuration into a ConfigMap.
Meanwhile, the vMCP server has its own discovery code but only loads
config statically from the ConfigMap at startup.

This duplication creates inconsistency risk, requires pod restarts for
backend changes to take effect, and has grown the operator complex with
auth resolution logic scattered across components.

This proposal flips responsibilities: the operator becomes a "dumb"
infrastructure manager handling only Deployment/Service/RBAC, while
vMCP becomes "smart" by running controller-runtime with informers to
watch MCPServer/ExternalAuthConfig/Secret resources directly.

Relates-to: #2855
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Dec 4, 2025
changes, the informer notifies vMCP which re-resolves auth for affected
backends. This provides a fast request path while handling secret rotation
automatically. The existing code in `pkg/vmcp/workloads/k8s.go` already
follows this pattern.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel shaky about this because the vMCP server is actually exposed to the internet and receives/serves customer traffic. The operator having extra powers is not too bad given that the operator is beholden to the k8s API and does not serve traffic nor is exposed. But this flip can be tricky. I rather defer this and keep the vMCP server just being aware of secrets via environment variables or files. Files might even be ideal since the process can leverage fsnotify or a similar strategy to react to changes.

CLI mode should remain static with one-time discovery at startup. Local
development sessions are typically short-lived, and adding dynamic discovery
to CLI would add complexity for limited benefit. This can be revisited as a
separate work item if there's demand.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this section necessary? If the refactor of vMCP leads to a dynamic CLI mode, that would be a win. Let's not add constraints where they're not needed and work on a best-effort approach instead.

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

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants