stacklok / toolhive / 19818082525

59%

Build Type

push

github

Committed by web-flow

Commit Message

Fix vMCP error on partially set operational config (#2813)

* Add integration test for partial operational config

Add a test case that loads YAML with partial operational config
(timeouts set, failure_handling omitted). This would have crashed
before the fix with "invalid health_check_interval: time: invalid
duration \"\"" because the old code unconditionally parsed empty
duration strings.

* Add operational config defaults to prevent vMCP crash

Problem:
Users specifying partial operational config in VirtualMCPServer CRD or
CLI YAML files caused vMCP to crash. For example:

```yaml
operational:
    timeouts:
	default: 45s
    # failure_handling not specified
```

This crashed with: "invalid health_check_interval: time: invalid duration \"\""

Root cause:
- The old transformOperational() unconditionally parsed duration fields
without checking for empty strings
- Kubernetes kubebuilder defaults only apply to fields present in YAML,
not nested objects that are omitted entirely
- Both CLI (YAML loader) and K8s (converter) paths were affected

Design decision - Centralized defaults vs runtime defaults:
We chose to centralize operational defaults in the config package rather
than handle zero values at runtime (like composite tools do with workflow
timeouts). The rationale:

Alternative considered:

A minimal fix would have been just adding empty-string guards:

```go
      if raw.FailureHandling.HealthCheckInterval != "" {
          healthCheckInterval, err = time.ParseDuration(...)
      }
```

This would fix the crash but leave zero values in config, requiring
downstream code to handle them. The centralized defaults approach is
more robust for partial configs.

* Simplify YAML loader by removing rawOperational

Remove the intermediate rawOperational struct and transformOperational
function. The code now unmarshals directly into OperationalConfig,
leveraging Duration.UnmarshalYAML for duration parsing.

This simplification is possible because:
- Duration... (continued)

Run Details

35 of 35 new or added lines in 3 files covered. (100.0%)

11 existing lines in 2 files now uncovered.

25668 of 51310 relevant lines covered (50.03%)

60.93 hits per line