Skip to content

Commit 018b8eb

Browse files
mjudeikisclaude
andcommitted
fix(infra): deterministic APIResourceSchema name hash (stop etcd-filling leak)
schemaPrefix hashed the CRD's OpenAPIV3Schema with fmt %v. OpenAPIV3Schema is full of pointer fields (Default, Items, AdditionalProperties, …), and fmt %v prints nested pointer fields by *address*, not value. Those addresses change every reconcile, so the hash — and thus the immutable APIResourceSchema name (tmpl<hash>.templates.infrastructure.kedge.faros.sh) — was non-deterministic. Every reconcile minted a brand-new schema and never deleted the old one. In a long-lived environment this leaked ~4.5k APIResourceSchemas (~55MB in /registry, mirrored in /cache) into a single logical cluster. On the next control-plane restart, kcp's cold-start LIST of all schemas pushed etcd past its memory limit into a permanent OOMKill loop, taking the root shard (and the console) down. The controller path already hashed via json.Marshal but kept a %v fallback; the install path still used %v directly. - install/apiexport.go: hash json.Marshal(schema); stable, address-free fallback instead of %v. - controller/template/apiexport.go: drop the %v fallback for the same. - install/apiexport_test.go: regression test — identical content (incl. Default pointer fields) must hash equal across allocations, and a real content change must still change the name. Both callsites now frame the hash identically, so bootstrap-install and the runtime controller produce the same name for the same schema. Note: this stops new leaks. Recovering an already-affected cluster still requires bumping etcd memory, GC-ing the orphaned schemas, and an etcd defrag/compact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c774a9a commit 018b8eb

3 files changed

Lines changed: 91 additions & 2 deletions

File tree

providers/infrastructure/controller/template/apiexport.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,11 @@ func schemaPrefix(crd *apiextensionsv1.CustomResourceDefinition) string {
202202
// non-deterministic.
203203
data, err := crdsJSONMarshal(v.Schema.OpenAPIV3Schema)
204204
if err != nil {
205-
_, _ = fmt.Fprintf(h, "%v\n", v.Schema.OpenAPIV3Schema)
205+
// Stable, address-free fallback. NEVER use fmt %v here: it
206+
// prints pointer-field addresses, which makes the schema name
207+
// non-deterministic and leaks a new immutable APIResourceSchema
208+
// every reconcile (fills etcd → OOM).
209+
_, _ = fmt.Fprintf(h, "marshal-error:%s/%s\n", crd.Name, v.Name)
206210
continue
207211
}
208212
_, _ = h.Write(data)

providers/infrastructure/install/apiexport.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,23 @@ func schemaPrefix(crd *apiextensionsv1.CustomResourceDefinition) string {
305305
for _, v := range crd.Spec.Versions {
306306
fmt.Fprintln(h, v.Name)
307307
if v.Schema != nil && v.Schema.OpenAPIV3Schema != nil {
308-
fmt.Fprintf(h, "%v\n", v.Schema.OpenAPIV3Schema)
308+
// Hash a deterministic JSON serialization. NEVER use fmt %v
309+
// here: OpenAPIV3Schema is full of pointer fields (Default,
310+
// Items, AdditionalProperties, …) and %v prints their memory
311+
// addresses, which change every reconcile. That makes the
312+
// APIResourceSchema name non-deterministic, so each reconcile
313+
// mints a new immutable schema and leaks the old one — the leak
314+
// that filled etcd and OOM-killed it. json.Marshal sorts object
315+
// keys and renders pointers by value, so identical content always
316+
// yields an identical name. Mirrors controller/template.
317+
data, err := json.Marshal(v.Schema.OpenAPIV3Schema)
318+
if err != nil {
319+
// Should never happen for a JSONSchemaProps. Fall back to a
320+
// stable, address-free key rather than %v.
321+
fmt.Fprintf(h, "marshal-error:%s/%s\n", crd.Name, v.Name)
322+
continue
323+
}
324+
h.Write(data)
309325
}
310326
}
311327
return "tmpl" + hex.EncodeToString(h.Sum(nil))[:8]
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package install
2+
3+
import (
4+
"testing"
5+
6+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
7+
)
8+
9+
// schemaWithPointers builds a CRD whose OpenAPIV3Schema populates the
10+
// pointer-typed fields (Default, *bool flags) that fmt %v renders as
11+
// memory addresses. Each call allocates fresh, so two structurally
12+
// identical results live at different addresses — the precise condition
13+
// that made the old %v-based hash non-deterministic and leaked a new
14+
// immutable APIResourceSchema on every reconcile (eventually OOM-ing etcd).
15+
func schemaWithPointers() *apiextensionsv1.CustomResourceDefinition {
16+
preserve := true
17+
return &apiextensionsv1.CustomResourceDefinition{
18+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
19+
Group: "infrastructure.kedge.faros.sh",
20+
Names: apiextensionsv1.CustomResourceDefinitionNames{Kind: "Template", Plural: "templates"},
21+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{
22+
Name: "v1alpha1",
23+
Schema: &apiextensionsv1.CustomResourceValidation{
24+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
25+
Type: "object",
26+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
27+
"image": {
28+
Type: "string",
29+
Default: &apiextensionsv1.JSON{Raw: []byte(`"registry.example/img:v1"`)},
30+
},
31+
"replicas": {
32+
Type: "integer",
33+
Default: &apiextensionsv1.JSON{Raw: []byte(`1`)},
34+
},
35+
},
36+
XPreserveUnknownFields: &preserve,
37+
},
38+
},
39+
}},
40+
},
41+
}
42+
}
43+
44+
// TestSchemaPrefixDeterministic locks the fix: identical schema content
45+
// must hash to the same name regardless of allocation, even when the
46+
// schema carries pointer fields. With the old fmt %v hash this failed
47+
// because %v printed pointer addresses.
48+
func TestSchemaPrefixDeterministic(t *testing.T) {
49+
a := schemaPrefix(schemaWithPointers())
50+
b := schemaPrefix(schemaWithPointers())
51+
if a != b {
52+
t.Fatalf("schemaPrefix must be deterministic for identical content; got %q vs %q", a, b)
53+
}
54+
}
55+
56+
// TestSchemaPrefixSensitiveToContent ensures a real content change still
57+
// produces a different name (so genuine schema updates get a new schema).
58+
func TestSchemaPrefixSensitiveToContent(t *testing.T) {
59+
base := schemaWithPointers()
60+
changed := schemaWithPointers()
61+
changed.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["replicas"] =
62+
apiextensionsv1.JSONSchemaProps{
63+
Type: "integer",
64+
Default: &apiextensionsv1.JSON{Raw: []byte(`3`)}, // 1 -> 3
65+
}
66+
if schemaPrefix(base) == schemaPrefix(changed) {
67+
t.Fatal("schemaPrefix must change when schema content changes")
68+
}
69+
}

0 commit comments

Comments
 (0)