Skip to content

Conversation

@JAORMX
Copy link
Contributor

@JAORMX JAORMX commented Jan 14, 2026

Summary

This RFC proposes architectural improvements to ToolHive's Kubernetes CRDs for the v1beta1 release.

Key Changes

New Configuration CRDs:

  • MCPOIDCConfig - Shared OIDC authentication config
  • MCPTelemetryConfig - Shared OpenTelemetry/Prometheus config
  • MCPAuthzConfig - Shared Cedar authorization policies
  • MCPAggregationToolConfig - Per-workload tool filtering for VirtualMCPServer

Structural Improvements:

  • Remove VirtualMCPServer's embedded Config field (CRD-first approach)
  • Replace type discriminators with CEL-validated unions
  • Remove deprecated fields (port, targetPort, toolsFilter)
  • Consolidate MCPRegistry status to single phase + conditions

Quality of Life:

  • Add printer columns for better kubectl get output
  • Introduce common LocalObjectReference type

Implementation Phases

  1. Phase 1: Define all v1beta1 API types with CEL validation
  2. Phase 2: Implement controllers (parallelizable)
  3. Phase 3: Testing, migration tooling (skill + agent), release

Related RFCs

  • THV-0001 (OpenTelemetry) - MCPTelemetryConfig builds on this
  • THV-0014 (vMCP K8s-Aware) - VirtualMCPServer changes align with this

Breaking Change

This is a breaking change from v1alpha1. Migration tooling will be provided.


🤖 Generated with Claude Code

Copy link
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.

Added a bunch of comments related to the shared CRDs. I'm not sure if this proposal is not pulling in this opposite direction as @jerm-dro has been working in the vMCP config space.

IncomingAuth *IncomingAuthConfig `json:"incomingAuth"`

// NOTE: THIS IS NOT ENTIRELY USED AND IS PARTIALLY DUPLICATED
Config config.Config `json:"config,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

@jerm-dro might have the details paged in, but IIRC this was to make sure that the vMCP binary can manage the config itself? e.g. this is the desired field we're moving towards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging this. I've pulled out the VirtualMCPServer Config embedding sections from this RFC - those changes will come in a follow-up PR to make sure we're aligned with THV-0014 and Jeremy's work. This RFC now focuses on the shared configuration CRDs (MCPOIDCConfig, MCPTelemetryConfig, MCPAuthzConfig) that all workload types can reference.

## Goals

- **Extract shared configuration into reusable CRDs**: Create MCPOIDCConfig, MCPTelemetryConfig, MCPAuthzConfig, and MCPAggregationToolConfig following the MCPExternalAuthConfig pattern
- **Remove VirtualMCPServer Config embedding**: Adopt CRD-first approach with runtime conversion to platform-agnostic types
Copy link
Contributor

Choose a reason for hiding this comment

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

@jerm-dro 👀

Choose a reason for hiding this comment

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

Yup, thanks for raising. I mentioned to Ozz I have thoughts about this, and I will be opening a PR with an alternative proposed.

Writing writing the alternative is not on the top of my list yet, I can summarize my thinking: I don't agree. I think as much as possible we should use the same config in CRDs and the servers. If there are unique-CRD spec fields that supersede or modify the config, that should be well-documented. I'll elaborate on how I've come to that conclusion in the PR.

Choose a reason for hiding this comment

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

@JAORMX @jhrozek I've opened a stacked PR here: #27

Let's move discussion there.

JAORMX and others added 3 commits January 16, 2026 10:30
This RFC proposes architectural improvements to ToolHive's Kubernetes CRDs:

- Extract shared config into reusable CRDs (MCPOIDCConfig, MCPTelemetryConfig,
  MCPAuthzConfig, MCPAggregationToolConfig)
- Remove VirtualMCPServer's embedded Config field
- Replace type discriminators with CEL-validated unions
- Remove deprecated fields
- Consolidate MCPRegistry status

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove VirtualMCPServer Config embedding sections (deferred to follow-up PR)
- Split OIDC config into shareable (issuer, jwksUri, claims) vs per-server (audience, scopes)
- Split Telemetry config into shareable (endpoint, settings) vs per-server (serviceName)
- Add namespace to all config references
- Add status size discussion to Open Questions
- Clean up Go implementation details, keep architecture-focused content
- Update reference types to show per-server override patterns

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@JAORMX JAORMX force-pushed the rfc/crd-v1beta1-optimization branch from e3f65ad to 227f01e Compare January 16, 2026 08:59
@jhrozek
Copy link
Contributor

jhrozek commented Jan 19, 2026

LGTM now. I won't ack in case Jeremy adds his comments later so that the proposal doesn't get accidentally merged, but feel free to ping me once you need me to hit the approve button.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants