Add dynamic/static mode support to VirtualMCPServer operator#3235
Add dynamic/static mode support to VirtualMCPServer operator#3235
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3235 +/- ##
==========================================
+ Coverage 64.74% 64.85% +0.11%
==========================================
Files 370 372 +2
Lines 36076 36292 +216
==========================================
+ Hits 23356 23537 +181
- Misses 10881 10906 +25
- Partials 1839 1849 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
There was a problem hiding this comment.
Pull request overview
This PR adds support for dynamic and static operational modes to the VirtualMCPServer operator. In dynamic mode (source: discovered), the vMCP server discovers backends at runtime via Kubernetes API with RBAC permissions and a minimal ConfigMap. In static mode (source: inline), backends are pre-configured in the ConfigMap with no RBAC resources created, eliminating the need for Kubernetes API access.
Key changes:
- Added mode-based RBAC resource management (create in dynamic, skip/cleanup in static)
- Modified ConfigMap generation to produce minimal or full content based on mode
- Updated deployment to use appropriate ServiceAccount based on mode
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/thv-operator/controllers/virtualmcpserver_controller.go | Adds ensureRBACResources logic for mode-based RBAC creation/cleanup and getServiceAccountNameForVmcp helper |
| cmd/thv-operator/controllers/virtualmcpserver_deployment.go | Updates RBAC rules to include mcptoolconfigs and virtualmcpservers/status permissions, adds service account determination |
| cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go | Modifies ConfigMap generation to conditionally include backend details based on source mode |
| cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go | Adds unit tests for dynamic and static mode ConfigMap content |
| test/e2e/thv-operator/virtualmcp/virtualmcp_lifecycle_test.go | Adds E2E tests for dynamic mode RBAC creation, static mode RBAC absence, and mode switching behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go
Outdated
Show resolved
Hide resolved
cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go
Outdated
Show resolved
Hide resolved
Large PR justification has been provided. Thank you!
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* remove docs * fixes from review * simplify code and fixes from review * fixes from review * fix ci --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
* remove docs * fixes from review * simplify code and fixes from review * fixes from review * fix ci --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
* remove docs * fixes from review * simplify code and fixes from review * fixes from review * fix ci --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
* remove docs * fixes from review * simplify code and fixes from review * fixes from review * fix ci --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
* remove docs * fixes from review * simplify code and fixes from review * fixes from review * fix ci --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
* remove docs * fixes from review * simplify code and fixes from review * fixes from review * fix ci --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
Fixes #3448 A very subtle bug introduced in #3235 caused the vMCP reconciler to repeatedly update the associated deployment, because the config.Backends ordering was non-deterministic. This PR makes the ordering of backends deterministic to not cause false positives when asking "should the deployment be updated?" --------- Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
Fixes #3448 A very subtle bug introduced in #3235 caused the vMCP reconciler to repeatedly update the associated deployment, because the config.Backends ordering was non-deterministic. This PR makes the ordering of backends deterministic to not cause false positives when asking "should the deployment be updated?" --------- Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
* remove docs * fixes from review * simplify code and fixes from review * fixes from review * fix ci --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
Update the VirtualMCPServer operator to generate different ConfigMap content and RBAC resources based on the outgoingAuth.source field, enabling two distinct operational modes:
Dynamic mode (source: discovered):
Static mode (source: inline):
Closes: #3003
Large PR Justification
This is an atomic PR that covers a single functionality. It includes structure changes and comprehensive testing. Cannot be split.