Skip to content

Commit e719f47

Browse files
yroblataskbot
andauthored
Remove session v1 and SessionManagementV2 feature flag (#4128)
* Remove session v1 and SessionManagementV2 feature flag Session v2 is now the sole implementation. The SessionManagementV2 config option has been deleted along with all v1 code paths: VMCPSession, sessionIDAdapter, the global-router-based discovery fallback, and related tests. SessionFactory is now a required field on server.Config. All "V2" naming has been dropped — factories, managers, tests, and e2e fixtures now use plain "session" terminology. Dead v1 capability-injection helpers (injectCapabilities, injectOptimizerCapabilities, setSessionResourcesDirect) have been removed, as has the operator's conditional HMAC-secret guard. Affected: pkg/vmcp/server, pkg/vmcp/session, pkg/vmcp/config, pkg/vmcp/discovery, cmd/vmcp, cmd/thv-operator, test/e2e Related-to: #3872 * use mocks --------- Co-authored-by: taskbot <taskbot@users.noreply.github.com>
1 parent ccfef60 commit e719f47

43 files changed

Lines changed: 2011 additions & 2553 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cmd/thv-operator/controllers/virtualmcpserver_controller.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -681,13 +681,6 @@ func (r *VirtualMCPServerReconciler) ensureHMACSecret(
681681
) error {
682682
ctxLogger := log.FromContext(ctx)
683683

684-
// Skip HMAC secret creation only when SessionManagementV2 is explicitly set to false.
685-
// nil means "unset" which defaults to true (v2 enabled).
686-
op := vmcp.Spec.Config.Operational
687-
if op != nil && op.SessionManagementV2 != nil && !*op.SessionManagementV2 {
688-
return nil
689-
}
690-
691684
secretName := fmt.Sprintf("%s-hmac-secret", vmcp.Name)
692685
secret := &corev1.Secret{}
693686
err := r.Get(ctx, types.NamespacedName{Name: secretName, Namespace: vmcp.Namespace}, secret)

cmd/thv-operator/controllers/virtualmcpserver_deployment.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,8 @@ func (r *VirtualMCPServerReconciler) buildEnvVarsForVmcp(
277277
// Mount outgoing auth secrets
278278
env = append(env, r.buildOutgoingAuthEnvVars(ctx, vmcp, typedWorkloads)...)
279279

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

287283
// Note: Other secrets (Redis passwords, service account credentials) may be added here in the future
288284
// following the same pattern of mounting from Kubernetes Secrets as environment variables.

cmd/vmcp/app/commands.go

Lines changed: 12 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,12 @@ func createSessionFactory(
306306
minRecommendedSecretLen = 32
307307
)
308308

309+
// Build base options, conditionally including aggregator only when provided.
310+
opts := []vmcpsession.MultiSessionFactoryOption{}
311+
if agg != nil {
312+
opts = append(opts, vmcpsession.WithAggregator(agg))
313+
}
314+
309315
hmacSecret := os.Getenv(envKey)
310316

311317
if hmacSecret != "" {
@@ -319,11 +325,8 @@ func createSessionFactory(
319325
)
320326
}
321327
slog.Info("using HMAC secret from VMCP_SESSION_HMAC_SECRET environment variable for session token binding")
322-
return vmcpsession.NewSessionFactory(
323-
outgoingRegistry,
324-
vmcpsession.WithHMACSecret([]byte(hmacSecret)),
325-
vmcpsession.WithAggregator(agg),
326-
), nil
328+
opts = append(opts, vmcpsession.WithHMACSecret([]byte(hmacSecret)))
329+
return vmcpsession.NewSessionFactory(outgoingRegistry, opts...), nil
327330
}
328331

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

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

342345
// runServe implements the serve command logic
@@ -524,15 +527,9 @@ func runServe(cmd *cobra.Command, _ []string) error {
524527
return fmt.Errorf("failed to validate optimizer config: %w", err)
525528
}
526529

527-
// Create session factory only when SessionManagementV2 is enabled.
528-
// nil means "unset" which defaults to true; explicit *false opts out.
529-
sessionManagementV2 := cfg.Operational.SessionManagementV2 == nil || *cfg.Operational.SessionManagementV2
530-
var sessionFactory vmcpsession.MultiSessionFactory
531-
if sessionManagementV2 {
532-
sessionFactory, err = createSessionFactory(outgoingRegistry, agg)
533-
if err != nil {
534-
return err
535-
}
530+
sessionFactory, err := createSessionFactory(outgoingRegistry, agg)
531+
if err != nil {
532+
return err
536533
}
537534

538535
serverCfg := &vmcpserver.Config{
@@ -551,7 +548,6 @@ func runServe(cmd *cobra.Command, _ []string) error {
551548
Watcher: backendWatcher,
552549
StatusReporter: statusReporter,
553550
OptimizerConfig: optCfg,
554-
SessionManagementV2: sessionManagementV2,
555551
SessionFactory: sessionFactory,
556552
}
557553

@@ -574,54 +570,3 @@ func runServe(cmd *cobra.Command, _ []string) error {
574570
slog.Info(fmt.Sprintf("Starting Virtual MCP Server at %s", srv.Address()))
575571
return srv.Start(ctx)
576572
}
577-
578-
// aggregateCapabilities aggregates capabilities from backends or creates empty capabilities.
579-
//
580-
// NOTE: This function is currently unused due to lazy discovery implementation (issue #2501).
581-
// It may be removed in a future cleanup or used for startup-time capability caching.
582-
//
583-
//nolint:unused // Unused until we implement startup aggregation or caching
584-
func aggregateCapabilities(
585-
ctx context.Context,
586-
agg aggregator.Aggregator,
587-
backends []vmcp.Backend,
588-
) (*aggregator.AggregatedCapabilities, error) {
589-
slog.Info("aggregating capabilities from backends")
590-
591-
if len(backends) > 0 {
592-
aggCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
593-
defer cancel()
594-
595-
capabilities, err := agg.AggregateCapabilities(aggCtx, backends)
596-
if err != nil {
597-
return nil, fmt.Errorf("failed to aggregate capabilities: %w", err)
598-
}
599-
600-
slog.Info(fmt.Sprintf("Aggregated %d tools, %d resources, %d prompts from %d backends",
601-
capabilities.Metadata.ToolCount,
602-
capabilities.Metadata.ResourceCount,
603-
capabilities.Metadata.PromptCount,
604-
capabilities.Metadata.BackendCount))
605-
606-
return capabilities, nil
607-
}
608-
609-
// No backends available - create empty capabilities
610-
slog.Warn("no backends available - starting with empty capabilities")
611-
return &aggregator.AggregatedCapabilities{
612-
Tools: []vmcp.Tool{},
613-
Resources: []vmcp.Resource{},
614-
Prompts: []vmcp.Prompt{},
615-
RoutingTable: &vmcp.RoutingTable{
616-
Tools: make(map[string]*vmcp.BackendTarget),
617-
Resources: make(map[string]*vmcp.BackendTarget),
618-
Prompts: make(map[string]*vmcp.BackendTarget),
619-
},
620-
Metadata: &aggregator.AggregationMetadata{
621-
BackendCount: 0,
622-
ToolCount: 0,
623-
ResourceCount: 0,
624-
PromptCount: 0,
625-
},
626-
}, nil
627-
}

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -691,14 +691,6 @@ spec:
691691
enum:
692692
- debug
693693
type: string
694-
sessionManagementV2:
695-
default: true
696-
description: |-
697-
SessionManagementV2 enables session-scoped backend client lifecycle.
698-
When true, vMCP creates real backend connections per session via MultiSessionFactory
699-
and routes tool calls directly through the session rather than the global router.
700-
Defaults to true. Set explicitly to false to opt out.
701-
type: boolean
702694
timeouts:
703695
description: Timeouts configures timeout settings.
704696
properties:

deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -694,14 +694,6 @@ spec:
694694
enum:
695695
- debug
696696
type: string
697-
sessionManagementV2:
698-
default: true
699-
description: |-
700-
SessionManagementV2 enables session-scoped backend client lifecycle.
701-
When true, vMCP creates real backend connections per session via MultiSessionFactory
702-
and routes tool calls directly through the session rather than the global router.
703-
Defaults to true. Set explicitly to false to opt out.
704-
type: boolean
705697
timeouts:
706698
description: Timeouts configures timeout settings.
707699
properties:

docs/operator/crd-api.md

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/vmcp/aggregator/aggregator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ type Aggregator interface {
6464

6565
// ProcessPreQueriedCapabilities applies the same aggregation pipeline (overrides,
6666
// conflict resolution, advertising filter) to tools that have already been fetched
67-
// from live backends. Used by the v2 session management path to reuse aggregator
67+
// from live backends. Used by the session management path to reuse aggregator
6868
// logic without re-querying backends over HTTP.
6969
//
7070
// toolsByBackend maps backend WorkloadID → raw tools as returned by the backend.

pkg/vmcp/aggregator/default_aggregator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ func (a *defaultAggregator) AggregateCapabilities(
497497

498498
// ProcessPreQueriedCapabilities implements Aggregator.ProcessPreQueriedCapabilities.
499499
// It reuses processBackendTools, ResolveConflicts, and shouldAdvertiseTool so that
500-
// the v2 session path applies identical transforms to the v1 aggregation path.
500+
// the session path applies identical transforms to the aggregation path.
501501
func (a *defaultAggregator) ProcessPreQueriedCapabilities(
502502
ctx context.Context,
503503
toolsByBackend map[string][]vmcp.Tool,

pkg/vmcp/config/config.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -440,14 +440,6 @@ type OperationalConfig struct {
440440
// FailureHandling configures failure handling behavior.
441441
// +optional
442442
FailureHandling *FailureHandlingConfig `json:"failureHandling,omitempty" yaml:"failureHandling,omitempty"`
443-
444-
// SessionManagementV2 enables session-scoped backend client lifecycle.
445-
// When true, vMCP creates real backend connections per session via MultiSessionFactory
446-
// and routes tool calls directly through the session rather than the global router.
447-
// Defaults to true. Set explicitly to false to opt out.
448-
// +kubebuilder:default=true
449-
// +optional
450-
SessionManagementV2 *bool `json:"sessionManagementV2,omitempty" yaml:"sessionManagementV2,omitempty"`
451443
}
452444

453445
// TimeoutConfig configures timeout settings.

pkg/vmcp/config/defaults.go

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,11 @@ const (
4242
defaultCircuitBreakerEnabled = false
4343
)
4444

45-
// boolPtr returns a pointer to the given bool value.
46-
func boolPtr(b bool) *bool {
47-
return &b
48-
}
49-
5045
// DefaultOperationalConfig returns a fully populated OperationalConfig with default values.
5146
// This is the SINGLE SOURCE OF TRUTH for all operational defaults.
5247
// This should be used when no operational config is provided.
5348
func DefaultOperationalConfig() *OperationalConfig {
5449
return &OperationalConfig{
55-
SessionManagementV2: boolPtr(true),
5650
Timeouts: &TimeoutConfig{
5751
Default: Duration(defaultTimeoutDefault),
5852
PerWorkload: nil,
@@ -76,10 +70,6 @@ func DefaultOperationalConfig() *OperationalConfig {
7670
// If Operational is nil, it sets it to DefaultOperationalConfig().
7771
// If Operational exists but has nil or zero-value nested fields, those fields
7872
// are filled with defaults while preserving any user-provided values.
79-
//
80-
// Note: SessionManagementV2 is a *bool. nil means "unset" → default true.
81-
// mergo dereferences pointers and treats *false as a zero-value bool, so an
82-
// explicit *false opt-out is saved before the merge and restored afterwards.
8373
func (c *Config) EnsureOperationalDefaults() {
8474
if c == nil {
8575
return
@@ -90,24 +80,7 @@ func (c *Config) EnsureOperationalDefaults() {
9080
return
9181
}
9282

93-
// mergo treats the bool zero-value (false) as "unset" even through a pointer,
94-
// and overwrites the value at the pointer address. Save the original pointer
95-
// and the value it holds so they can be restored after the merge, preserving
96-
// pointer identity for any caller that retains a reference.
97-
origPtr := c.Operational.SessionManagementV2
98-
var origVal bool
99-
if origPtr != nil {
100-
origVal = *origPtr
101-
}
102-
10383
// Merge defaults into target, only filling zero/nil values.
104-
// User-provided values are preserved (except for the *bool caveat above).
84+
// User-provided values are preserved.
10585
_ = mergo.Merge(c.Operational, DefaultOperationalConfig())
106-
107-
// Restore the user-provided value through the original pointer so that
108-
// pointer identity is preserved and the explicit opt-out is not lost.
109-
if origPtr != nil {
110-
*origPtr = origVal
111-
c.Operational.SessionManagementV2 = origPtr
112-
}
11386
}

0 commit comments

Comments
 (0)