Skip to content
Draft
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
10 changes: 9 additions & 1 deletion cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
vmcptypes "github.com/stacklok/toolhive/pkg/vmcp"
"github.com/stacklok/toolhive/pkg/vmcp/aggregator"
vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
vmcpruntimeconfig "github.com/stacklok/toolhive/pkg/vmcp/config/runtime"
"github.com/stacklok/toolhive/pkg/vmcp/workloads"
)

Expand Down Expand Up @@ -84,7 +85,14 @@ func (r *VirtualMCPServerReconciler) ensureVmcpConfigConfigMap(
// Marshal the serializable Config to YAML for storage in ConfigMap.
// Note: gopkg.in/yaml.v3 produces deterministic output by sorting map keys alphabetically.
// This ensures stable checksums for triggering pod rollouts only when content actually changes.
vmcpConfigYAML, err := yaml.Marshal(config)
//
// Wrap in runtime.Config so future operator-resolved fields can be
// added to the ConfigMap without leaking into the public
// vmcpconfig.Config (and therefore the CRD schema). Today runtime.Config
// embeds vmcpconfig.Config inline and adds no extra keys, so the
// marshalled YAML is byte-identical.
runtimeCfg := vmcpruntimeconfig.Config{Config: *config}
vmcpConfigYAML, err := yaml.Marshal(runtimeCfg)
if err != nil {
return fmt.Errorf("failed to marshal vmcp config: %w", err)
}
Expand Down
126 changes: 126 additions & 0 deletions cmd/thv-operator/pkg/spectoconfig/runtime_config_drift_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
// SPDX-License-Identifier: Apache-2.0

package spectoconfig

import (
"reflect"
"sort"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/stacklok/toolhive/cmd/thv-operator/internal/testutil"
vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
vmcpruntimeconfig "github.com/stacklok/toolhive/pkg/vmcp/config/runtime"
)

// runtimeOnlyLeafJustifications enumerates leaf paths that exist on
// runtime.Config but NOT on vmcpconfig.Config. Each entry must include a
// justification explaining why the field is operator-resolved and therefore
// must not appear in the public CRD schema.
//
// Today runtime.Config adds nothing extra, so this map is empty. The drift
// test below fails if anyone adds a field to runtime.Config without
// allowlisting it here, which is the forcing function that turns "I'll
// just stick this on the runtime wrapper" into a deliberate review
// conversation.
//
// When future operator-resolved fields land (e.g. a backendHeaderForward
// map populated from MCPServerEntry.spec.headerForward), add the leaf path
// here with a justification that names the operator code that populates it.
var runtimeOnlyLeafJustifications = map[string]string{}

// TestRuntimeConfigSeam guards the wrapper relationship between
// vmcpconfig.Config (the public CRD-visible type) and runtime.Config (the
// operator-write surface):
//
// - runtime.Config MUST contain every vmcpconfig.Config leaf. runtime.Config
// embeds vmcpconfig.Config inline, so today this is automatic — but the
// test pins it so a future refactor that, say, replaces the embed with a
// reference would fail loudly instead of silently dropping fields from
// the ConfigMap.
//
// - Every leaf on runtime.Config that is NOT on vmcpconfig.Config must
// appear in runtimeOnlyLeafJustifications with an explanation. These
// are operator-resolved sidecars (secret-identifier maps, resolved file
// paths, etc.) that travel through the ConfigMap to the vMCP runtime
// without becoming public API surface.
//
// - Conversely, no vmcpconfig.Config leaf may be allowlisted in
// runtimeOnlyLeafJustifications — that would be a contradiction.
//
// The test catches the mistake the wrapper exists to prevent: adding an
// operator-resolved field directly onto vmcpconfig.Config (where it leaks
// into the CRD) instead of onto runtime.Config.
func TestRuntimeConfigSeam(t *testing.T) {
t.Parallel()

configLeaves := setOf(testutil.FlattenJSONLeafFields(reflect.TypeOf(vmcpconfig.Config{})))
runtimeLeaves := setOf(testutil.FlattenJSONLeafFields(reflect.TypeOf(vmcpruntimeconfig.Config{})))

t.Run("RuntimeConfig is a superset of Config", func(t *testing.T) {
t.Parallel()
var missing []string
for leaf := range configLeaves {
if _, ok := runtimeLeaves[leaf]; !ok {
missing = append(missing, leaf)
}
}
sort.Strings(missing)
assert.Empty(
t, missing,
"every vmcpconfig.Config leaf must also be a leaf on runtime.Config (runtime.Config embeds vmcpconfig.Config inline). "+
"Missing: %v", missing,
)
})

t.Run("RuntimeConfig extras are justified", func(t *testing.T) {
t.Parallel()
for leaf := range runtimeLeaves {
if _, onConfig := configLeaves[leaf]; onConfig {
continue
}
reason, ok := runtimeOnlyLeafJustifications[leaf]
if !ok {
t.Errorf(
"runtime.Config leaf %q is not present on vmcpconfig.Config and is not allowlisted "+
"in runtimeOnlyLeafJustifications.\n"+
"Action: if this field is operator-resolved (must NOT appear in the CRD), "+
"add it to runtimeOnlyLeafJustifications with a justification. "+
"Otherwise, move it onto Config so the CRD picks it up.",
leaf,
)
continue
}
require.NotEmptyf(t, reason, "runtimeOnlyLeafJustifications[%q] must include a justification", leaf)
}
})

t.Run("allowlist has no stale or self-contradicting entries", func(t *testing.T) {
t.Parallel()
for leaf := range runtimeOnlyLeafJustifications {
if _, onConfig := configLeaves[leaf]; onConfig {
t.Errorf(
"%q is allowlisted as runtime-only but also exists on Config — contradicting classification",
leaf,
)
}
if _, onRuntime := runtimeLeaves[leaf]; !onRuntime {
t.Errorf(
"%q is allowlisted as runtime-only but is not a live leaf on runtime.Config — stale entry?",
leaf,
)
}
}
})
}

func setOf(s []string) map[string]struct{} {
out := make(map[string]struct{}, len(s))
for _, v := range s {
out[v] = struct{}{}
}
return out
}
53 changes: 53 additions & 0 deletions pkg/vmcp/config/runtime/runtime_config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
// SPDX-License-Identifier: Apache-2.0

// Package runtime hosts the operator-only on-disk wrapper around
// pkg/vmcp/config.Config. Living in a subpackage keeps the wrapper out of
// the source set scanned by crdref-gen (which globs pkg/vmcp/config/*.go,
// not subpackages) and out of the type graph that controller-gen walks for
// CRD schema generation. Both keep operator-resolved sidecar fields off the
// public CRD surface.
package runtime

import (
vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
)

// Config (referenced as runtime.Config from outside this package) is the
// on-disk shape the operator writes to the vMCP ConfigMap and the vMCP
// binary parses on startup. It embeds vmcpconfig.Config (the user-facing
// API surface that VirtualMCPServerSpec exposes) and is the place to add
// operator-resolved fields that should NOT appear in the CRD schema.
//
// Why a wrapper: cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go
// types Spec.Config as `vmcpconfig.Config`. controller-gen renders the CRD
// OpenAPI schema by walking the type graph from CRD roots, so any field
// added anywhere under vmcpconfig.Config — directly or transitively —
// leaks into the public CRD. Operator-resolved fields (per-backend
// secret-identifier maps, resolved CA bundle paths, and similar) should
// travel through the ConfigMap to the vMCP runtime without becoming
// public API surface.
//
// Today runtime.Config embeds vmcpconfig.Config inline and adds nothing.
// Marshalled YAML is byte-identical to marshalling vmcpconfig.Config
// directly. Future operator-only fields are added on this struct, leaving
// vmcpconfig.Config — and therefore the CRD — untouched.
//
// Invariants (enforced by tests in this package and in
// cmd/thv-operator/pkg/spectoconfig/runtime_config_drift_test.go):
//
// - Not a CRD type. runtime.Config has no kubebuilder markers and must
// never be field-referenced from any v1beta1 type. The single way to
// leak this struct's fields into the CRD is to retype
// VirtualMCPServerSpec.Config from `vmcpconfig.Config` to
// `runtime.Config`. Don't.
//
// - No top-level field on runtime.Config may share a JSON or YAML key
// with any vmcpconfig.Config field. encoding/json (which inlines via
// anonymous-field promotion) and gopkg.in/yaml.v3 (which honors
// `,inline`) handle key collisions differently — yaml.v3 errors or has
// a different precedence than encoding/json's outer-wins rule. The
// disjoint-tag test in runtime_config_test.go pins this.
type Config struct {
vmcpconfig.Config `json:",inline" yaml:",inline"` //nolint:revive // inline is a valid kubernetes json/yaml tag option
}
190 changes: 190 additions & 0 deletions pkg/vmcp/config/runtime/runtime_config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
// SPDX-License-Identifier: Apache-2.0

package runtime

import (
"os"
"path/filepath"
"reflect"
"strings"
"testing"

"github.com/stretchr/testify/require"
"gopkg.in/yaml.v3"

vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
)

// TestRuntimeConfig_MarshalsIdenticallyToConfig pins the invariant that
// today runtime.Config adds no extra serialized keys. If a future commit
// adds an operator-resolved field on runtime.Config the YAML will diverge,
// at which point this test should be updated to re-marshal and re-load via
// the operator's actual write/read path. The point here is to catch
// accidental divergence before the operator-only fields land.
func TestRuntimeConfig_MarshalsIdenticallyToConfig(t *testing.T) {
t.Parallel()

// Use a representative-ish Config value. We don't need every field
// populated — the YAML library walks reachable fields, and any
// divergence between Config and runtime.Config (extra top-level key)
// would surface even on a near-empty value.
cfg := vmcpconfig.Config{
Name: "demo",
Group: "demo-group",
Backends: []vmcpconfig.StaticBackendConfig{
{Name: "b1", URL: "https://example.test", Transport: "streamable-http"},
},
}

configYAML, err := yaml.Marshal(cfg)
require.NoError(t, err)

runtimeYAML, err := yaml.Marshal(Config{Config: cfg})
require.NoError(t, err)

require.Equal(
t, string(configYAML), string(runtimeYAML),
"runtime.Config must marshal identically to Config until operator-only fields are added. "+
"If you intentionally added a sidecar field, update this test to assert the new shape.",
)
}

// TestRuntimeConfig_Load_RoundTrip checks that the loader parses YAML the
// operator writes back into the same shape, with strict-unknown-field
// validation intact. Field access through the embed is transparent — rc.Name
// reaches Config.Name without an explicit unwrap.
func TestRuntimeConfig_Load_RoundTrip(t *testing.T) {
t.Parallel()

cfg := vmcpconfig.Config{
Name: "demo",
Group: "demo-group",
IncomingAuth: &vmcpconfig.IncomingAuthConfig{
Type: "anonymous",
},
}
yamlBytes, err := yaml.Marshal(Config{Config: cfg})
require.NoError(t, err)

tmp := filepath.Join(t.TempDir(), "config.yaml")
require.NoError(t, os.WriteFile(tmp, yamlBytes, 0o600))

loader := vmcpconfig.NewYAMLLoader(tmp, &fakeEnv{})
rc, err := loader.Load()
require.NoError(t, err)
require.NotNil(t, rc)
require.Equal(t, "demo", rc.Name)
require.Equal(t, "demo-group", rc.Group)
require.NotNil(t, rc.IncomingAuth)
require.Equal(t, "anonymous", rc.IncomingAuth.Type)
}

// TestRuntimeConfig_DisjointTopLevelTags pins the invariant that no
// top-level field on runtime.Config may share a JSON or YAML key with any
// Config field. encoding/json (anonymous-field promotion, outer wins) and
// yaml.v3 (`,inline`, may error or differ in precedence) handle collisions
// differently — a collision would silently produce divergent serialization
// and ambiguous round-trip behaviour. Today runtime.Config has no extra
// fields so this test is trivially green; the value is forward-looking.
func TestRuntimeConfig_DisjointTopLevelTags(t *testing.T) {
t.Parallel()

configKeys := topLevelTagKeys(reflect.TypeOf(vmcpconfig.Config{}))
runtimeOuterKeys := outerOnlyTagKeys(reflect.TypeOf(Config{}))

for _, key := range runtimeOuterKeys {
_, jsonClash := configKeys.json[key.json]
_, yamlClash := configKeys.yaml[key.yaml]
require.Falsef(
t, jsonClash,
"runtime.Config field %q has JSON key %q that collides with a Config field — "+
"yaml.v3 inline and encoding/json anonymous-promotion handle collisions differently",
key.fieldName, key.json,
)
require.Falsef(
t, yamlClash,
"runtime.Config field %q has YAML key %q that collides with a Config field",
key.fieldName, key.yaml,
)
}
}

type tagKeySets struct {
json map[string]struct{}
yaml map[string]struct{}
}

type fieldTagKey struct {
fieldName string
json string
yaml string
}

// topLevelTagKeys returns the set of JSON and YAML names exposed at the top
// level of t. For an embedded anonymous Config field, that's Config's own
// top-level fields (since both encoders flatten it).
func topLevelTagKeys(t reflect.Type) tagKeySets {
out := tagKeySets{json: map[string]struct{}{}, yaml: map[string]struct{}{}}
for i := 0; i < t.NumField(); i++ {
f := t.Field(i)
if f.Anonymous {
// Embedded fields with no explicit name flatten — recurse.
inner := topLevelTagKeys(f.Type)
for k := range inner.json {
out.json[k] = struct{}{}
}
for k := range inner.yaml {
out.yaml[k] = struct{}{}
}
continue
}
if jn := tagName(f, "json", f.Name); jn != "" && jn != "-" {
out.json[jn] = struct{}{}
}
if yn := tagName(f, "yaml", f.Name); yn != "" && yn != "-" {
out.yaml[yn] = struct{}{}
}
}
return out
}

// outerOnlyTagKeys returns ONLY the non-embedded fields of t, with their
// JSON and YAML names. Used to identify "extra" runtime.Config fields that
// might collide with the embedded Config's fields.
func outerOnlyTagKeys(t reflect.Type) []fieldTagKey {
var out []fieldTagKey
for i := 0; i < t.NumField(); i++ {
f := t.Field(i)
if f.Anonymous {
continue
}
jn := tagName(f, "json", f.Name)
yn := tagName(f, "yaml", f.Name)
if (jn == "" || jn == "-") && (yn == "" || yn == "-") {
continue
}
out = append(out, fieldTagKey{fieldName: f.Name, json: jn, yaml: yn})
}
return out
}

func tagName(f reflect.StructField, key, fallback string) string {
tag, ok := f.Tag.Lookup(key)
if !ok {
return fallback
}
name, _, _ := strings.Cut(tag, ",")
if name == "" {
return fallback
}
return name
}

// fakeEnv satisfies the env.Reader interface for the loader's secret
// resolution path. Round-trip tests don't exercise secret env vars, so
// returning empty for everything is fine.
type fakeEnv struct{}

func (*fakeEnv) Getenv(string) string { return "" }
func (*fakeEnv) LookupEnv(string) (string, bool) { return "", false }
Loading
Loading