Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions cmd/thv-operator/controllers/virtualmcpserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,13 +681,6 @@ func (r *VirtualMCPServerReconciler) ensureHMACSecret(
) error {
ctxLogger := log.FromContext(ctx)

// Skip HMAC secret creation only when SessionManagementV2 is explicitly set to false.
// nil means "unset" which defaults to true (v2 enabled).
op := vmcp.Spec.Config.Operational
if op != nil && op.SessionManagementV2 != nil && !*op.SessionManagementV2 {
return nil
}

secretName := fmt.Sprintf("%s-hmac-secret", vmcp.Name)
secret := &corev1.Secret{}
err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: vmcp.Namespace}, secret)
Expand Down
8 changes: 2 additions & 6 deletions cmd/thv-operator/controllers/virtualmcpserver_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,8 @@ func (r *VirtualMCPServerReconciler) buildEnvVarsForVmcp(
// Mount outgoing auth secrets
env = append(env, r.buildOutgoingAuthEnvVars(ctx, vmcp, typedWorkloads)...)

// Mount HMAC secret when SessionManagementV2 is enabled (nil = unset = default true).
// Skip only when explicitly set to false.
op := vmcp.Spec.Config.Operational
if op == nil || op.SessionManagementV2 == nil || *op.SessionManagementV2 {
env = append(env, r.buildHMACSecretEnvVar(vmcp))
}
// Always mount HMAC secret for session token binding.
env = append(env, r.buildHMACSecretEnvVar(vmcp))

// Note: Other secrets (Redis passwords, service account credentials) may be added here in the future
// following the same pattern of mounting from Kubernetes Secrets as environment variables.
Expand Down
79 changes: 12 additions & 67 deletions cmd/vmcp/app/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,12 @@ func createSessionFactory(
minRecommendedSecretLen = 32
)

// Build base options, conditionally including aggregator only when provided.
opts := []vmcpsession.MultiSessionFactoryOption{}
if agg != nil {
opts = append(opts, vmcpsession.WithAggregator(agg))
}

hmacSecret := os.Getenv(envKey)

if hmacSecret != "" {
Expand All @@ -319,11 +325,8 @@ func createSessionFactory(
)
}
slog.Info("using HMAC secret from VMCP_SESSION_HMAC_SECRET environment variable for session token binding")
return vmcpsession.NewSessionFactory(
outgoingRegistry,
vmcpsession.WithHMACSecret([]byte(hmacSecret)),
vmcpsession.WithAggregator(agg),
), nil
opts = append(opts, vmcpsession.WithHMACSecret([]byte(hmacSecret)))
return vmcpsession.NewSessionFactory(outgoingRegistry, opts...), nil
}

// No secret provided - check if we're in Kubernetes (production environment)
Expand All @@ -336,7 +339,7 @@ func createSessionFactory(

// Development mode: use default insecure secret with warning
slog.Warn("VMCP_SESSION_HMAC_SECRET not set - using default insecure secret (NOT recommended for production)")
return vmcpsession.NewSessionFactory(outgoingRegistry, vmcpsession.WithAggregator(agg)), nil
return vmcpsession.NewSessionFactory(outgoingRegistry, opts...), nil
}

// runServe implements the serve command logic
Expand Down Expand Up @@ -526,15 +529,9 @@ func runServe(cmd *cobra.Command, _ []string) error {
return fmt.Errorf("failed to validate optimizer config: %w", err)
}

// Create session factory only when SessionManagementV2 is enabled.
// nil means "unset" which defaults to true; explicit *false opts out.
sessionManagementV2 := cfg.Operational.SessionManagementV2 == nil || *cfg.Operational.SessionManagementV2
var sessionFactory vmcpsession.MultiSessionFactory
if sessionManagementV2 {
sessionFactory, err = createSessionFactory(outgoingRegistry, agg)
if err != nil {
return err
}
sessionFactory, err := createSessionFactory(outgoingRegistry, agg)
if err != nil {
return err
}

serverCfg := &vmcpserver.Config{
Expand All @@ -553,7 +550,6 @@ func runServe(cmd *cobra.Command, _ []string) error {
Watcher: backendWatcher,
StatusReporter: statusReporter,
OptimizerConfig: optCfg,
SessionManagementV2: sessionManagementV2,
SessionFactory: sessionFactory,
}

Expand All @@ -576,54 +572,3 @@ func runServe(cmd *cobra.Command, _ []string) error {
slog.Info(fmt.Sprintf("Starting Virtual MCP Server at %s", srv.Address()))
return srv.Start(ctx)
}

// aggregateCapabilities aggregates capabilities from backends or creates empty capabilities.
//
// NOTE: This function is currently unused due to lazy discovery implementation (issue #2501).
// It may be removed in a future cleanup or used for startup-time capability caching.
//
//nolint:unused // Unused until we implement startup aggregation or caching
func aggregateCapabilities(
ctx context.Context,
agg aggregator.Aggregator,
backends []vmcp.Backend,
) (*aggregator.AggregatedCapabilities, error) {
slog.Info("aggregating capabilities from backends")

if len(backends) > 0 {
aggCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()

capabilities, err := agg.AggregateCapabilities(aggCtx, backends)
if err != nil {
return nil, fmt.Errorf("failed to aggregate capabilities: %w", err)
}

slog.Info(fmt.Sprintf("Aggregated %d tools, %d resources, %d prompts from %d backends",
capabilities.Metadata.ToolCount,
capabilities.Metadata.ResourceCount,
capabilities.Metadata.PromptCount,
capabilities.Metadata.BackendCount))

return capabilities, nil
}

// No backends available - create empty capabilities
slog.Warn("no backends available - starting with empty capabilities")
return &aggregator.AggregatedCapabilities{
Tools: []vmcp.Tool{},
Resources: []vmcp.Resource{},
Prompts: []vmcp.Prompt{},
RoutingTable: &vmcp.RoutingTable{
Tools: make(map[string]*vmcp.BackendTarget),
Resources: make(map[string]*vmcp.BackendTarget),
Prompts: make(map[string]*vmcp.BackendTarget),
},
Metadata: &aggregator.AggregationMetadata{
BackendCount: 0,
ToolCount: 0,
ResourceCount: 0,
PromptCount: 0,
},
}, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -691,14 +691,6 @@ spec:
enum:
- debug
type: string
sessionManagementV2:
default: true
description: |-
SessionManagementV2 enables session-scoped backend client lifecycle.
When true, vMCP creates real backend connections per session via MultiSessionFactory
and routes tool calls directly through the session rather than the global router.
Defaults to true. Set explicitly to false to opt out.
type: boolean
timeouts:
description: Timeouts configures timeout settings.
properties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -694,14 +694,6 @@ spec:
enum:
- debug
type: string
sessionManagementV2:
default: true
description: |-
SessionManagementV2 enables session-scoped backend client lifecycle.
When true, vMCP creates real backend connections per session via MultiSessionFactory
and routes tool calls directly through the session rather than the global router.
Defaults to true. Set explicitly to false to opt out.
type: boolean
timeouts:
description: Timeouts configures timeout settings.
properties:
Expand Down
1 change: 0 additions & 1 deletion docs/operator/crd-api.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/vmcp/aggregator/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type Aggregator interface {

// ProcessPreQueriedCapabilities applies the same aggregation pipeline (overrides,
// conflict resolution, advertising filter) to tools that have already been fetched
// from live backends. Used by the v2 session management path to reuse aggregator
// from live backends. Used by the session management path to reuse aggregator
// logic without re-querying backends over HTTP.
//
// toolsByBackend maps backend WorkloadID → raw tools as returned by the backend.
Expand Down
2 changes: 1 addition & 1 deletion pkg/vmcp/aggregator/default_aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ func (a *defaultAggregator) AggregateCapabilities(

// ProcessPreQueriedCapabilities implements Aggregator.ProcessPreQueriedCapabilities.
// It reuses processBackendTools, ResolveConflicts, and shouldAdvertiseTool so that
// the v2 session path applies identical transforms to the v1 aggregation path.
// the session path applies identical transforms to the aggregation path.
func (a *defaultAggregator) ProcessPreQueriedCapabilities(
ctx context.Context,
toolsByBackend map[string][]vmcp.Tool,
Expand Down
8 changes: 0 additions & 8 deletions pkg/vmcp/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,14 +440,6 @@ type OperationalConfig struct {
// FailureHandling configures failure handling behavior.
// +optional
FailureHandling *FailureHandlingConfig `json:"failureHandling,omitempty" yaml:"failureHandling,omitempty"`

// SessionManagementV2 enables session-scoped backend client lifecycle.
// When true, vMCP creates real backend connections per session via MultiSessionFactory
// and routes tool calls directly through the session rather than the global router.
// Defaults to true. Set explicitly to false to opt out.
// +kubebuilder:default=true
// +optional
SessionManagementV2 *bool `json:"sessionManagementV2,omitempty" yaml:"sessionManagementV2,omitempty"`
}

// TimeoutConfig configures timeout settings.
Expand Down
29 changes: 1 addition & 28 deletions pkg/vmcp/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,11 @@ const (
defaultCircuitBreakerEnabled = false
)

// boolPtr returns a pointer to the given bool value.
func boolPtr(b bool) *bool {
return &b
}

// DefaultOperationalConfig returns a fully populated OperationalConfig with default values.
// This is the SINGLE SOURCE OF TRUTH for all operational defaults.
// This should be used when no operational config is provided.
func DefaultOperationalConfig() *OperationalConfig {
return &OperationalConfig{
SessionManagementV2: boolPtr(true),
Timeouts: &TimeoutConfig{
Default: Duration(defaultTimeoutDefault),
PerWorkload: nil,
Expand All @@ -76,10 +70,6 @@ func DefaultOperationalConfig() *OperationalConfig {
// If Operational is nil, it sets it to DefaultOperationalConfig().
// If Operational exists but has nil or zero-value nested fields, those fields
// are filled with defaults while preserving any user-provided values.
//
// Note: SessionManagementV2 is a *bool. nil means "unset" → default true.
// mergo dereferences pointers and treats *false as a zero-value bool, so an
// explicit *false opt-out is saved before the merge and restored afterwards.
func (c *Config) EnsureOperationalDefaults() {
if c == nil {
return
Expand All @@ -90,24 +80,7 @@ func (c *Config) EnsureOperationalDefaults() {
return
}

// mergo treats the bool zero-value (false) as "unset" even through a pointer,
// and overwrites the value at the pointer address. Save the original pointer
// and the value it holds so they can be restored after the merge, preserving
// pointer identity for any caller that retains a reference.
origPtr := c.Operational.SessionManagementV2
var origVal bool
if origPtr != nil {
origVal = *origPtr
}

// Merge defaults into target, only filling zero/nil values.
// User-provided values are preserved (except for the *bool caveat above).
// User-provided values are preserved.
_ = mergo.Merge(c.Operational, DefaultOperationalConfig())

// Restore the user-provided value through the original pointer so that
// pointer identity is preserved and the explicit opt-out is not lost.
if origPtr != nil {
*origPtr = origVal
c.Operational.SessionManagementV2 = origPtr
}
}
28 changes: 0 additions & 28 deletions pkg/vmcp/config/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ func TestDefaultOperationalConfig(t *testing.T) {
require.NotNil(t, cfg.FailureHandling)
require.NotNil(t, cfg.FailureHandling.CircuitBreaker)

// SessionManagementV2 defaults to *true
require.NotNil(t, cfg.SessionManagementV2, "SessionManagementV2 should be non-nil by default")
assert.True(t, *cfg.SessionManagementV2, "SessionManagementV2 should default to true")

// Verify all defaults match constants
assert.Equal(t, Duration(defaultTimeoutDefault), cfg.Timeouts.Default)
assert.Nil(t, cfg.Timeouts.PerWorkload)
Expand Down Expand Up @@ -245,27 +241,3 @@ func TestEnsureOperationalDefaults_Idempotent(t *testing.T) {
assert.Equal(t, Duration(defaultHealthCheckInterval), cfg.Operational.FailureHandling.HealthCheckInterval)
assert.Equal(t, defaultUnhealthyThreshold, cfg.Operational.FailureHandling.UnhealthyThreshold)
}

func TestEnsureOperationalDefaults_SessionManagementV2OptOut(t *testing.T) {
t.Parallel()

// Explicit *false must survive EnsureOperationalDefaults (v1 opt-out).
falseBool := false
cfg := &Config{
Name: "test-vmcp",
Group: "test-group",
Operational: &OperationalConfig{
SessionManagementV2: &falseBool,
},
}

cfg.EnsureOperationalDefaults()

require.NotNil(t, cfg.Operational.SessionManagementV2, "SessionManagementV2 should not be nil")
assert.False(t, *cfg.Operational.SessionManagementV2,
"explicit false opt-out must be preserved after EnsureOperationalDefaults")
// Pointer identity must be preserved: callers that retain the original *bool
// must see the updated value through their existing pointer, not a new allocation.
assert.Same(t, &falseBool, cfg.Operational.SessionManagementV2,
"EnsureOperationalDefaults must not change the pointer identity of SessionManagementV2")
}
5 changes: 0 additions & 5 deletions pkg/vmcp/config/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading