Skip to content

Commit f701838

Browse files
mjudeikisclaude
andauthored
fix(infra): deterministic APIResourceSchema name hash (stop etcd-filling leak) (#371)
* 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> * fix(app-studio): MCP discovery honors KEDGE_HUB_INSECURE app-studio reaches the hub's MCPServer virtual-workspace endpoint at the in-cluster service DNS (kedge-...-hub.<ns>.svc.cluster.local:9443), but the hub serves its external cert (e.g. console-dev.faros.sh) with no SAN for the internal name. Every provider — and app-studio's own GraphQL client and heartbeat — skips verification for in-cluster hub calls via KEDGE_HUB_INSECURE. The MCP path, however, was gated by a separate APP_STUDIO_MCP_INSECURE_SKIP_TLS_VERIFY flag that the Helm chart never templates, so there was no way to make MCP discovery trust the hub even when the operator set hub.insecure. Result: GraphQL worked, MCP tool discovery failed with "x509: certificate is valid for console-dev.faros.sh, not kedge-...-hub.svc.cluster.local". Make the MCP TLS-skip honor KEDGE_HUB_INSECURE in addition to its own flag, matching the other hub clients. No chart/value changes needed: prod already sets KEDGE_HUB_INSECURE=true. Also pick up the license boilerplate codegen added to the new install/apiexport_test.go. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c774a9a commit f701838

4 files changed

Lines changed: 114 additions & 3 deletions

File tree

providers/app-studio/main.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,13 @@ func runServe() {
162162
msgStore,
163163
openWorkspaceStore(),
164164
os.Getenv("KEDGE_HUB_URL"),
165-
os.Getenv("APP_STUDIO_MCP_INSECURE_SKIP_TLS_VERIFY") == "true",
165+
// The MCP virtual-workspace endpoint lives on the same hub host as the
166+
// GraphQL client above, so it must honor the standard KEDGE_HUB_INSECURE
167+
// knob every provider uses for in-cluster hub TLS (the hub serves its
168+
// external cert, not one valid for the .svc.cluster.local name). Keep the
169+
// MCP-specific override too, for callers that want to scope it narrowly.
170+
os.Getenv("APP_STUDIO_MCP_INSECURE_SKIP_TLS_VERIFY") == "true" ||
171+
os.Getenv("KEDGE_HUB_INSECURE") == "true",
166172
)
167173
apiServer.SetAutoApproveAssistantActions(os.Getenv("APP_STUDIO_AUTO_APPROVE_ACTIONS") == "true")
168174
if secret := os.Getenv("APP_STUDIO_PREVIEW_TOKEN_SECRET"); secret != "" {

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: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
Copyright 2026 The Faros Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package install
18+
19+
import (
20+
"testing"
21+
22+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
23+
)
24+
25+
// schemaWithPointers builds a CRD whose OpenAPIV3Schema populates the
26+
// pointer-typed fields (Default, *bool flags) that fmt %v renders as
27+
// memory addresses. Each call allocates fresh, so two structurally
28+
// identical results live at different addresses — the precise condition
29+
// that made the old %v-based hash non-deterministic and leaked a new
30+
// immutable APIResourceSchema on every reconcile (eventually OOM-ing etcd).
31+
func schemaWithPointers() *apiextensionsv1.CustomResourceDefinition {
32+
preserve := true
33+
return &apiextensionsv1.CustomResourceDefinition{
34+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
35+
Group: "infrastructure.kedge.faros.sh",
36+
Names: apiextensionsv1.CustomResourceDefinitionNames{Kind: "Template", Plural: "templates"},
37+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{
38+
Name: "v1alpha1",
39+
Schema: &apiextensionsv1.CustomResourceValidation{
40+
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
41+
Type: "object",
42+
Properties: map[string]apiextensionsv1.JSONSchemaProps{
43+
"image": {
44+
Type: "string",
45+
Default: &apiextensionsv1.JSON{Raw: []byte(`"registry.example/img:v1"`)},
46+
},
47+
"replicas": {
48+
Type: "integer",
49+
Default: &apiextensionsv1.JSON{Raw: []byte(`1`)},
50+
},
51+
},
52+
XPreserveUnknownFields: &preserve,
53+
},
54+
},
55+
}},
56+
},
57+
}
58+
}
59+
60+
// TestSchemaPrefixDeterministic locks the fix: identical schema content
61+
// must hash to the same name regardless of allocation, even when the
62+
// schema carries pointer fields. With the old fmt %v hash this failed
63+
// because %v printed pointer addresses.
64+
func TestSchemaPrefixDeterministic(t *testing.T) {
65+
a := schemaPrefix(schemaWithPointers())
66+
b := schemaPrefix(schemaWithPointers())
67+
if a != b {
68+
t.Fatalf("schemaPrefix must be deterministic for identical content; got %q vs %q", a, b)
69+
}
70+
}
71+
72+
// TestSchemaPrefixSensitiveToContent ensures a real content change still
73+
// produces a different name (so genuine schema updates get a new schema).
74+
func TestSchemaPrefixSensitiveToContent(t *testing.T) {
75+
base := schemaWithPointers()
76+
changed := schemaWithPointers()
77+
changed.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["replicas"] =
78+
apiextensionsv1.JSONSchemaProps{
79+
Type: "integer",
80+
Default: &apiextensionsv1.JSON{Raw: []byte(`3`)}, // 1 -> 3
81+
}
82+
if schemaPrefix(base) == schemaPrefix(changed) {
83+
t.Fatal("schemaPrefix must change when schema content changes")
84+
}
85+
}

0 commit comments

Comments
 (0)