Skip to content

Commit ffcfd8e

Browse files
ChrisJBurnsclaude
andcommitted
Add CRD-runtime drift detection test framework
Silent drift between CRD types and their runtime config counterparts has caused user-facing bugs (e.g. PR #3118, issue #3125) where new fields appeared to work in tests but were not wired through the CRD conversion path. The only existing safety nets — code review and end-to-end tests — do not scale. Add a reusable reflection-based field walker and a bidirectional drift test pattern. The pattern requires every leaf JSON field on either side of a CRD/runtime boundary to be either explicitly mapped to the other side or explicitly declared as intentionally unmapped, with a justification string. Adding a field to either type without classifying it fails the test with a clear action-required message. Apply the pattern to MCPTelemetryConfig <-> telemetry.Config as the first real-world exercise. The drift test passes against the current codebase and surfaced one previously-undocumented leaf (openTelemetry.caBundleRef.configMapRef.optional) which is now explicitly declared. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 729e56a commit ffcfd8e

3 files changed

Lines changed: 711 additions & 0 deletions

File tree

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
// Package testutil provides reflection-based helpers used by drift-detection
5+
// tests in the operator. It is intended for test code only.
6+
package testutil
7+
8+
import (
9+
"encoding/json"
10+
"reflect"
11+
"sort"
12+
"strings"
13+
"time"
14+
15+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16+
17+
thvjson "github.com/stacklok/toolhive/pkg/json"
18+
vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
19+
)
20+
21+
// FlattenJSONLeafFields returns every leaf JSON field path under t as a sorted
22+
// slice of dot-delimited paths (e.g. "openTelemetry.tracing.enabled").
23+
//
24+
// Walking semantics:
25+
// - For struct fields, the JSON tag's name (before any comma) is used as the
26+
// path segment. If the tag is "-", the field is skipped. If the tag is
27+
// missing, the Go field name is used.
28+
// - Unexported fields are skipped.
29+
// - Fields with `,inline` flatten into the parent path with no prefix.
30+
// - Pointer types are dereferenced and walked.
31+
// - Struct types are recursed into.
32+
// - Slice and array types whose ELEMENT type is a struct (or pointer to a
33+
// struct) recurse into that element type, joining with the parent path.
34+
// A path like "tools.workload" therefore means "every element of the
35+
// `tools` slice has a `workload` field".
36+
// - Map types whose VALUE type is a struct (or pointer to a struct) recurse
37+
// into that value type the same way.
38+
// - All other types (primitives, []string, map[string]string,
39+
// time.Duration, metav1.Duration, json.RawMessage, and any type listed
40+
// in the leaf allowlist below) terminate the walk and contribute one
41+
// leaf path entry.
42+
// - To prevent infinite recursion on self-referential types, track visited
43+
// types per walk and stop if a cycle is detected.
44+
//
45+
// If t is nil or, after dereferencing, is not a struct, an empty slice is
46+
// returned rather than panicking.
47+
func FlattenJSONLeafFields(t reflect.Type) []string {
48+
if t == nil {
49+
return []string{}
50+
}
51+
for t.Kind() == reflect.Ptr {
52+
t = t.Elem()
53+
}
54+
if t.Kind() != reflect.Struct {
55+
return []string{}
56+
}
57+
58+
leafSet := make(map[string]struct{})
59+
visited := map[reflect.Type]int{}
60+
recurseStruct(t, "", leafSet, visited)
61+
62+
out := make([]string, 0, len(leafSet))
63+
for p := range leafSet {
64+
out = append(out, p)
65+
}
66+
sort.Strings(out)
67+
return out
68+
}
69+
70+
// leafTypes lists types that terminate the walk even though they are structs.
71+
// They contribute a single leaf entry at their parent path.
72+
var leafTypes = map[reflect.Type]struct{}{
73+
reflect.TypeOf(time.Duration(0)): {},
74+
reflect.TypeOf(metav1.Duration{}): {},
75+
reflect.TypeOf(json.RawMessage{}): {},
76+
reflect.TypeOf(thvjson.Map{}): {},
77+
reflect.TypeOf(thvjson.Any{}): {},
78+
reflect.TypeOf(vmcpconfig.Duration(0)): {},
79+
}
80+
81+
// skipFieldTypes lists embedded struct types that must be skipped entirely.
82+
var skipFieldTypes = map[reflect.Type]struct{}{
83+
reflect.TypeOf(metav1.TypeMeta{}): {},
84+
reflect.TypeOf(metav1.ObjectMeta{}): {},
85+
reflect.TypeOf(metav1.ListMeta{}): {},
86+
}
87+
88+
// walkStruct recurses into a struct type and adds leaf paths to leafSet.
89+
// prefix is the dot-delimited path accumulated so far (without trailing dot).
90+
// visited is the set of struct types currently on the recursion stack; it is
91+
// used to break cycles on self-referential types. Callers add t to visited
92+
// before invoking this function and remove it afterwards.
93+
func walkStruct(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]int) {
94+
for i := 0; i < t.NumField(); i++ {
95+
field := t.Field(i)
96+
// Skip unexported fields. PkgPath is non-empty for unexported fields.
97+
// Anonymous (embedded) fields have an empty PkgPath only when their
98+
// underlying type is exported, so the same check works for them.
99+
if field.PkgPath != "" && !field.Anonymous {
100+
continue
101+
}
102+
103+
// Skip explicit skip-list types (TypeMeta/ObjectMeta/ListMeta) when
104+
// embedded.
105+
if _, skip := skipFieldTypes[field.Type]; skip {
106+
continue
107+
}
108+
109+
name, inline, omit := parseJSONTag(field)
110+
if omit {
111+
continue
112+
}
113+
114+
// Determine the path segment for this field.
115+
segment := func() string {
116+
// Anonymous fields are inlined by encoding/json when they have no
117+
// json name (or an explicit `,inline` tag). Treat them as inline.
118+
if field.Anonymous && (name == "" || inline) {
119+
return ""
120+
}
121+
if inline {
122+
return ""
123+
}
124+
if name == "" {
125+
return field.Name
126+
}
127+
return name
128+
}()
129+
130+
childPrefix := joinPath(prefix, segment)
131+
walkType(field.Type, childPrefix, leafSet, visited)
132+
}
133+
}
134+
135+
// walkType walks a single type with the given accumulated prefix. It either
136+
// recurses into nested structs/slices/maps or records a leaf at prefix.
137+
func walkType(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]int) {
138+
// Deref pointers.
139+
for t.Kind() == reflect.Ptr {
140+
t = t.Elem()
141+
}
142+
143+
// Allow-listed leaf types short-circuit any further walking.
144+
if _, ok := leafTypes[t]; ok {
145+
recordLeaf(prefix, leafSet)
146+
return
147+
}
148+
149+
switch t.Kind() {
150+
case reflect.Struct:
151+
recurseStruct(t, prefix, leafSet, visited)
152+
case reflect.Slice, reflect.Array:
153+
elem := t.Elem()
154+
for elem.Kind() == reflect.Ptr {
155+
elem = elem.Elem()
156+
}
157+
// Recurse only when the element is a struct that is NOT a leaf type.
158+
if _, isLeaf := leafTypes[elem]; !isLeaf && elem.Kind() == reflect.Struct {
159+
recurseStruct(elem, prefix, leafSet, visited)
160+
return
161+
}
162+
recordLeaf(prefix, leafSet)
163+
case reflect.Map:
164+
val := t.Elem()
165+
for val.Kind() == reflect.Ptr {
166+
val = val.Elem()
167+
}
168+
if _, isLeaf := leafTypes[val]; !isLeaf && val.Kind() == reflect.Struct {
169+
recurseStruct(val, prefix, leafSet, visited)
170+
return
171+
}
172+
recordLeaf(prefix, leafSet)
173+
default:
174+
recordLeaf(prefix, leafSet)
175+
}
176+
}
177+
178+
// maxStructRevisits caps how many times a single struct type may appear on the
179+
// recursion stack. A value of 2 means the type may be entered at most twice
180+
// on the same path: the original entry plus one self-referential visit. This
181+
// is enough for one level of "next.<field>" expansion in cyclic types like
182+
// linked lists, while still guaranteeing the walk terminates.
183+
const maxStructRevisits = 2
184+
185+
// recurseStruct descends into a nested struct type with cycle protection.
186+
// visited holds the count of how many times each struct type currently appears
187+
// on the recursion stack. When entering t would exceed maxStructRevisits, the
188+
// walk stops silently — no leaf is recorded, on the assumption that the
189+
// useful leaves under t have already been captured by the earlier visit on
190+
// the same path.
191+
func recurseStruct(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]int) {
192+
if visited[t] >= maxStructRevisits {
193+
return
194+
}
195+
visited[t]++
196+
defer func() { visited[t]-- }()
197+
walkStruct(t, prefix, leafSet, visited)
198+
}
199+
200+
// parseJSONTag extracts (name, inline, omit) from the json struct tag.
201+
// - name is the explicit JSON name (empty when not specified).
202+
// - inline is true when the tag includes ",inline".
203+
// - omit is true when the tag is "-" (field must be skipped entirely).
204+
func parseJSONTag(field reflect.StructField) (string, bool, bool) {
205+
tag, ok := field.Tag.Lookup("json")
206+
if !ok {
207+
return "", false, false
208+
}
209+
if tag == "-" {
210+
return "", false, true
211+
}
212+
parts := strings.Split(tag, ",")
213+
name := parts[0]
214+
inline := false
215+
for _, p := range parts[1:] {
216+
if p == "inline" {
217+
inline = true
218+
}
219+
}
220+
return name, inline, false
221+
}
222+
223+
// joinPath joins prefix and segment with a dot, handling empty segments
224+
// (e.g. inline fields) by returning the prefix unchanged.
225+
func joinPath(prefix, segment string) string {
226+
if segment == "" {
227+
return prefix
228+
}
229+
if prefix == "" {
230+
return segment
231+
}
232+
return prefix + "." + segment
233+
}
234+
235+
// recordLeaf records prefix as a leaf path. An empty prefix is ignored
236+
// because it would not represent a real field.
237+
func recordLeaf(prefix string, leafSet map[string]struct{}) {
238+
if prefix == "" {
239+
return
240+
}
241+
leafSet[prefix] = struct{}{}
242+
}

0 commit comments

Comments
 (0)