From 4bd8aeb5bf2a1db6f02b2836559c657f6badbfe1 Mon Sep 17 00:00:00 2001 From: bussyjd Date: Sat, 8 Aug 2026 17:37:16 +0400 Subject: [PATCH] fix(discovery): hold published bundles while an upstream probe is unsettled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the restart window that #810 left open. The upstream-OpenAPI cache is process-local, so every controller restart begins with no entry for any offer. reconcileStaticSite rebuilds the SHARED bundle from that cache on EVERY offer's reconcile — including reconciles belonging to other offers — so an offer that has not reconciled yet was re-rendered from the route-table fallback and visibly lost its advertised routes until its own reconcile landed. Measured on a live nine-offer stack during the v0.14.0-rc3 validation: after a controller image swap, five offers dropped from 11/12/12/12/16 advertised resources to 1 at ~t+30s and were fully recovered by ~t+90s. Buyers crawling discovery inside that window see a single root entry instead of the real paid routes. #810 stopped a FAILED probe being pinned for the whole generation. It could not fix this, because get() returns nil for "never fetched" and "fetched, no document" alike, and the caller cannot tell them apart. getSettled now returns that distinction, and buildOfferBundles takes the currently-published ConfigMap data. While a probe is unsettled the already served openapi.json and x402.json are kept — the ConfigMap survives the restart even though the cache does not, so there is something correct to hold on to. Stale beats thinner. The other direction matters just as much and is tested: once a probe HAS settled with no document, the fallback is the correct final answer and must re-render, or an offer that legitimately drops its upstream OpenAPI would serve the old document forever. A first-ever reconcile with nothing published still renders the fallback rather than an empty document. Verified TestBuildOfferBundles_UnsettledPreservesPublished fails with the preserve branch disabled and passes with it. --- .../bundle_unsettled_test.go | 125 ++++++++++++++++++ internal/serviceoffercontroller/catalog.go | 14 ++ internal/serviceoffercontroller/controller.go | 2 +- .../serviceoffercontroller/hostoffer_test.go | 22 +-- .../serviceoffercontroller/offerbundle.go | 31 ++++- .../upstream_openapi.go | 23 +++- .../upstream_openapi_test.go | 6 +- 7 files changed, 204 insertions(+), 19 deletions(-) create mode 100644 internal/serviceoffercontroller/bundle_unsettled_test.go diff --git a/internal/serviceoffercontroller/bundle_unsettled_test.go b/internal/serviceoffercontroller/bundle_unsettled_test.go new file mode 100644 index 000000000..48d674a61 --- /dev/null +++ b/internal/serviceoffercontroller/bundle_unsettled_test.go @@ -0,0 +1,125 @@ +package serviceoffercontroller + +import ( + "strings" + "testing" + + "github.com/ObolNetwork/obol-stack/internal/monetizeapi" + "github.com/ObolNetwork/obol-stack/internal/schemas" +) + +// upstreamDocWithPaidRoutes is a minimal upstream OpenAPI document with two +// paid operations, so the expanded /.well-known/x402 has more than the single +// root entry the route-table fallback produces. +func upstreamDocWithPaidRoutes() map[string]any { + paidOp := func(summary string) map[string]any { + return map[string]any{ + "summary": summary, + "security": []any{map[string]any{"x402": []any{}}}, + "x-payment-info": map[string]any{"price": map[string]any{"amount": "0.001"}}, + "responses": map[string]any{"200": map[string]any{}, "402": map[string]any{}}, + } + } + return map[string]any{ + "openapi": "3.1.0", + "info": map[string]any{"title": "upstream", "version": "1.0.0"}, + "paths": map[string]any{ + "/v1/alpha": map[string]any{"get": paidOp("Alpha")}, + "/v1/beta": map[string]any{"get": paidOp("Beta")}, + }, + } +} + +func bundleContent(bundles []offerBundleFile, key string) string { + for _, b := range bundles { + if b.Key == key { + return b.Content + } + } + return "" +} + +// TestBuildOfferBundles_UnsettledPreservesPublished is the regression test for +// the restart window. +// +// The upstream-OpenAPI cache is process-local, so every controller restart +// begins with no entry for any offer. reconcileStaticSite rebuilds the SHARED +// bundle on EVERY offer's reconcile — including reconciles belonging to other +// offers — so an offer that has not reconciled yet would be re-rendered from +// the route-table fallback and visibly lose its advertised routes until its +// own reconcile lands. Measured at 30–90s on a nine-offer stack. +// +// While a probe is unsettled, the already-published document must be kept. +func TestBuildOfferBundles_UnsettledPreservesPublished(t *testing.T) { + offer := hostnameOffer() + profile := schemas.StorefrontProfile{} + + settled := func(*monetizeapi.ServiceOffer) (map[string]any, bool) { + return upstreamDocWithPaidRoutes(), true + } + unsettled := func(*monetizeapi.ServiceOffer) (map[string]any, bool) { return nil, false } + + x402Key := offerBundleKey(offer, "x402.json") + openapiKey := offerBundleKey(offer, "openapi.json") + + // What a healthy controller publishes once the probe has settled. + good := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, settled, nil) + goodX402 := bundleContent(good, x402Key) + goodOpenAPI := bundleContent(good, openapiKey) + if !strings.Contains(goodX402, "/v1/alpha") || !strings.Contains(goodX402, "/v1/beta") { + t.Fatalf("precondition: settled x402 should enumerate upstream paid routes, got %s", goodX402) + } + + published := map[string]string{x402Key: goodX402, openapiKey: goodOpenAPI} + + // Controller restarts: cache empty, this offer has not reconciled yet. + after := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, unsettled, published) + if got := bundleContent(after, x402Key); got != goodX402 { + t.Errorf("unsettled probe replaced the published x402 document.\n got: %s\nwant: %s", got, goodX402) + } + if got := bundleContent(after, openapiKey); got != goodOpenAPI { + t.Error("unsettled probe replaced the published openapi.json") + } +} + +// TestBuildOfferBundles_SettledEmptyStillFallsBack guards the other direction: +// once we HAVE probed and there is genuinely no upstream document, the +// route-table fallback is the correct final answer and must not be blocked by +// stale published content. Otherwise an offer that legitimately drops its +// upstream OpenAPI would serve the old document forever. +func TestBuildOfferBundles_SettledEmptyStillFallsBack(t *testing.T) { + offer := hostnameOffer() + profile := schemas.StorefrontProfile{} + x402Key := offerBundleKey(offer, "x402.json") + + fallback := bundleContent( + buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI, nil), x402Key) + + stale := map[string]string{x402Key: `{"x402Version":2,"resources":[{"method":"GET","resource":"https://stale/v1/gone"}]}`} + got := bundleContent( + buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI, stale), x402Key) + + if got != fallback { + t.Errorf("settled-with-no-document must re-render the fallback, not keep stale content.\n got: %s\nwant: %s", got, fallback) + } + if strings.Contains(got, "stale") { + t.Error("stale published content leaked into a settled rebuild") + } +} + +// TestBuildOfferBundles_UnsettledWithNoPublishedRendersFallback covers a first +// ever reconcile: nothing published yet, nothing to preserve, so the fallback +// is correct rather than an empty document. +func TestBuildOfferBundles_UnsettledWithNoPublishedRendersFallback(t *testing.T) { + offer := hostnameOffer() + unsettled := func(*monetizeapi.ServiceOffer) (map[string]any, bool) { return nil, false } + + bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, schemas.StorefrontProfile{}, unsettled, nil) + got := bundleContent(bundles, offerBundleKey(offer, "x402.json")) + if got == "" { + t.Fatal("first reconcile produced no x402 document at all") + } + if !strings.Contains(got, "resources") { + t.Errorf("first reconcile should render the route-table fallback, got %s", got) + } +} diff --git a/internal/serviceoffercontroller/catalog.go b/internal/serviceoffercontroller/catalog.go index b605d6098..05ba6d5a1 100644 --- a/internal/serviceoffercontroller/catalog.go +++ b/internal/serviceoffercontroller/catalog.go @@ -88,6 +88,20 @@ func (c *Controller) staticSiteContentUnchanged(ctx context.Context, content, se return staticSiteContentMatches(cm, content, servicesJSON, openAPIJSON, apiDocsHTML, wellKnownX402JSON, bundles), nil } +// publishedStaticSiteData returns the currently-served ConfigMap data, or nil +// when it does not exist yet. buildOfferBundles uses it to hold the line on an +// offer whose upstream probe has not settled since this process started — +// see the !settled branch there. A read error is not fatal: the caller simply +// renders from scratch, which is the pre-existing behaviour. +func (c *Controller) publishedStaticSiteData(ctx context.Context) map[string]string { + cm, err := c.configMaps.Namespace(staticSiteNamespace).Get(ctx, staticSiteConfigMapName, metav1.GetOptions{}) + if err != nil || cm == nil { + return nil + } + data, _, _ := unstructured.NestedStringMap(cm.Object, "data") + return data +} + func computeStaticSiteContentHash(content, servicesJSON, openAPIJSON, apiDocsHTML, wellKnownX402JSON string, bundles []offerBundleFile) string { // The embedded vendor bundle is part of the served content: fold it in // so a controller upgrade that changes it re-applies the ConfigMap and diff --git a/internal/serviceoffercontroller/controller.go b/internal/serviceoffercontroller/controller.go index 05371a5d4..288457300 100644 --- a/internal/serviceoffercontroller/controller.go +++ b/internal/serviceoffercontroller/controller.go @@ -1354,7 +1354,7 @@ func (c *Controller) reconcileStaticSite(ctx context.Context, override *monetize openAPIJSON := buildOpenAPIDocument(offers, baseURL, resolvedProfile) wellKnownX402JSON := buildAggregateWellKnownX402(offers, baseURL) apiDocsHTML := scalarHTML(resolvedProfile) - bundles := buildOfferBundles(offers, resolvedProfile, c.upstreamOpenAPICache.get) + bundles := buildOfferBundles(offers, resolvedProfile, c.upstreamOpenAPICache.getSettled, c.publishedStaticSiteData(ctx)) contentHash := computeStaticSiteContentHash(content, servicesJSON, openAPIJSON, apiDocsHTML, wellKnownX402JSON, bundles) unchanged, err := c.staticSiteContentUnchanged(ctx, content, servicesJSON, openAPIJSON, apiDocsHTML, wellKnownX402JSON, bundles) diff --git a/internal/serviceoffercontroller/hostoffer_test.go b/internal/serviceoffercontroller/hostoffer_test.go index c24056576..426d7aa21 100644 --- a/internal/serviceoffercontroller/hostoffer_test.go +++ b/internal/serviceoffercontroller/hostoffer_test.go @@ -22,7 +22,9 @@ func hostnameOffer() *monetizeapi.ServiceOffer { // noUpstreamOpenAPI is the buildOfferBundles cache-lookup stub for tests // that don't exercise the upstream-OpenAPI path. -func noUpstreamOpenAPI(*monetizeapi.ServiceOffer) map[string]any { return nil } +// noUpstreamOpenAPI is a SETTLED probe with no document: the offer was asked +// and has none, so the route-table fallback is the correct final answer. +func noUpstreamOpenAPI(*monetizeapi.ServiceOffer) (map[string]any, bool) { return nil, true } // TestBuildHostHTTPRoute pins the dedicated-origin route topology: Exact // discovery rules rewriting into the offer's bundle files on the catalog @@ -112,11 +114,11 @@ func TestBuildOfferBundles(t *testing.T) { profile := schemas.StorefrontProfile{DisplayName: "Acme", ContactEmail: "ops@acme.example"} offer := hostnameOffer() - if got := buildOfferBundles([]*monetizeapi.ServiceOffer{routeTableOffer()}, profile, noUpstreamOpenAPI); len(got) != 0 { + if got := buildOfferBundles([]*monetizeapi.ServiceOffer{routeTableOffer()}, profile, noUpstreamOpenAPI, nil); len(got) != 0 { t.Fatalf("path-only offer produced bundles: %v", got) } - bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI) + bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI, nil) if len(bundles) != 4 { t.Fatalf("len(bundles) = %d, want 4", len(bundles)) } @@ -216,7 +218,7 @@ func TestBuildOfferBundles_InferenceOfferAgreesWithOpenAPI(t *testing.T) { }, } - bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI) + bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI, nil) byPath := map[string]string{} for _, f := range bundles { byPath[f.Path] = f.Content @@ -268,7 +270,7 @@ func TestBuildOfferBundles_BrandingOverride(t *testing.T) { Description: "**Deep** audits by AuditCo.", } - bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI) + bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, noUpstreamOpenAPI, nil) byPath := map[string]string{} for _, f := range bundles { byPath[f.Path] = f.Content @@ -458,7 +460,7 @@ func TestStaticSiteServesChatWidget(t *testing.T) { // Per-offer page: agent offers gain a chat.html bundle file carrying // the landing page's theme tokens and title; non-agent offers do not. profile := schemas.StorefrontProfile{DisplayName: "Acme"} - plain := buildOfferBundles([]*monetizeapi.ServiceOffer{hostnameOffer()}, profile, noUpstreamOpenAPI) + plain := buildOfferBundles([]*monetizeapi.ServiceOffer{hostnameOffer()}, profile, noUpstreamOpenAPI, nil) for _, f := range plain { if strings.HasSuffix(f.Path, "chat.html") { t.Fatalf("non-agent offer rendered a chat page: %s", f.Path) @@ -466,7 +468,7 @@ func TestStaticSiteServesChatWidget(t *testing.T) { } agent := hostnameOffer() agent.Spec.Type = "agent" - bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{agent}, profile, noUpstreamOpenAPI) + bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{agent}, profile, noUpstreamOpenAPI, nil) var chat string for _, f := range bundles { if f.Path == "offers/sec/audit/chat.html" { @@ -525,7 +527,7 @@ func TestBuildOfferBundles_UpstreamOpenAPI(t *testing.T) { offer := hostnameOffer() offer.Spec.Registration.Name = "Hyperliquid Trading Intelligence" offer.Spec.Registration.Description = "Full first-party catalog." - upstream := func(*monetizeapi.ServiceOffer) map[string]any { + upstream := func(*monetizeapi.ServiceOffer) (map[string]any, bool) { return map[string]any{ "openapi": "3.1.0", "info": map[string]any{"title": "upstream-title", "version": "1.1.0"}, @@ -541,9 +543,9 @@ func TestBuildOfferBundles_UpstreamOpenAPI(t *testing.T) { "get": map[string]any{"summary": "Free overview", "security": []any{}, "responses": map[string]any{"200": map[string]any{}}}, }, }, - } + }, true } - bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, upstream) + bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, profile, upstream, nil) byPath := map[string]string{} for _, f := range bundles { byPath[f.Path] = f.Content diff --git a/internal/serviceoffercontroller/offerbundle.go b/internal/serviceoffercontroller/offerbundle.go index 998ffe174..2f03f5a42 100644 --- a/internal/serviceoffercontroller/offerbundle.go +++ b/internal/serviceoffercontroller/offerbundle.go @@ -52,7 +52,10 @@ func offerBundleKey(offer *monetizeapi.ServiceOffer, file string) string { // content hash and roll the shared discovery pod. The production caller // passes the Controller's upstreamOpenAPICache.get, refreshed independently // from each offer's own reconcile. -func buildOfferBundles(offers []*monetizeapi.ServiceOffer, profile schemas.StorefrontProfile, upstreamOpenAPI func(*monetizeapi.ServiceOffer) map[string]any) []offerBundleFile { +// published is the currently-served ConfigMap data (bundle key → content), or +// nil on the first ever reconcile. It is only read for offers whose upstream +// probe has not settled yet — see the comment at the !settled branch below. +func buildOfferBundles(offers []*monetizeapi.ServiceOffer, profile schemas.StorefrontProfile, upstreamOpenAPI func(*monetizeapi.ServiceOffer) (map[string]any, bool), published map[string]string) []offerBundleFile { var bundles []offerBundleFile for _, offer := range offers { if offer == nil || offer.Spec.Hostname == "" { @@ -62,16 +65,38 @@ func buildOfferBundles(offers []*monetizeapi.ServiceOffer, profile schemas.Store // branding block overrides the storefront profile field-wise // (empty fields inherit). originProfile := storefront.MergeProfile(profile, offer.Spec.Branding.ProfilePatch()) - upstreamDoc := upstreamOpenAPI(offer) + upstreamDoc, settled := upstreamOpenAPI(offer) openapiContent := buildOfferScopedOpenAPI(offer, originProfile) x402Content := buildOfferWellKnownX402(offer) - if upstreamDoc != nil { + switch { + case upstreamDoc != nil: if rewritten, ok := rewriteUpstreamOpenAPI(upstreamDoc, offer, originProfile); ok { openapiContent = rewritten } if expanded := buildOfferWellKnownX402FromOpenAPI(offer, upstreamDoc); expanded != "" { x402Content = expanded } + case !settled: + // We have not probed this offer yet. The upstream-derived document + // enumerates one resource per real paid route; the fallback above + // collapses to the offer root. Publishing the fallback now would + // visibly thin out discovery for an offer we simply have not asked + // about — which is exactly what happens on every controller + // restart, because the cache is process-local and the shared + // bundle is rebuilt from it on EVERY offer's reconcile, including + // reconciles belonging to other offers. + // + // The ConfigMap survives the restart, so prefer what is already + // being served until this offer's own reconcile settles the probe. + // Stale beats thinner. Once settled, the branches above own the + // content — including settled-with-no-document, where the fallback + // is the correct final answer. + if prev, ok := published[offerBundleKey(offer, "openapi.json")]; ok && prev != "" { + openapiContent = prev + } + if prev, ok := published[offerBundleKey(offer, "x402.json")]; ok && prev != "" { + x402Content = prev + } } bundles = append(bundles, offerBundleFile{ diff --git a/internal/serviceoffercontroller/upstream_openapi.go b/internal/serviceoffercontroller/upstream_openapi.go index 64d68b498..ca49d8a55 100644 --- a/internal/serviceoffercontroller/upstream_openapi.go +++ b/internal/serviceoffercontroller/upstream_openapi.go @@ -155,12 +155,31 @@ type upstreamOpenAPICache struct { // get returns the cached doc, or nil if no fetch has completed yet for this // offer's current generation. func (c *upstreamOpenAPICache) get(offer *monetizeapi.ServiceOffer) map[string]any { + doc, _ := c.getSettled(offer) + return doc +} + +// getSettled is get plus whether the cache has a SETTLED answer for this +// offer — i.e. a fetch has completed at least once. A nil doc means two very +// different things and callers that render discovery documents must tell them +// apart: +// +// - settled=true, doc=nil → we probed and there is no upstream document. +// The route-table fallback is the correct, final answer. +// - settled=false → we have not probed yet (the controller just +// started, or this offer has not reconciled since). Rendering the fallback +// here would publish a thinner document than the one already being served. +// +// The cache is process-local, so every controller restart begins unsettled for +// every offer. +func (c *upstreamOpenAPICache) getSettled(offer *monetizeapi.ServiceOffer) (map[string]any, bool) { if offer == nil { - return nil + return nil, false } c.mu.Lock() defer c.mu.Unlock() - return c.entries[offer.UID].doc + entry, ok := c.entries[offer.UID] + return entry.doc, ok } // refresh fetches (via fetch) and caches the result, but only when the diff --git a/internal/serviceoffercontroller/upstream_openapi_test.go b/internal/serviceoffercontroller/upstream_openapi_test.go index 08bc2507f..7db660fa5 100644 --- a/internal/serviceoffercontroller/upstream_openapi_test.go +++ b/internal/serviceoffercontroller/upstream_openapi_test.go @@ -54,8 +54,8 @@ func TestRewriteUpstreamOpenAPI_SizeCapFallsBack(t *testing.T) { // openapi.json (buildOfferScopedOpenAPI) instead of failing the whole // static site. fallback := buildOfferScopedOpenAPI(offer, schemas.StorefrontProfile{}) - upstream := func(*monetizeapi.ServiceOffer) map[string]any { return oversized } - bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, schemas.StorefrontProfile{}, upstream) + upstream := func(*monetizeapi.ServiceOffer) (map[string]any, bool) { return oversized, true } + bundles := buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, schemas.StorefrontProfile{}, upstream, nil) var openapiContent string for _, f := range bundles { if f.Path == "offers/sec/audit/openapi.json" { @@ -115,7 +115,7 @@ func TestUpstreamOpenAPICache_DeterministicAcrossFlappingFetch(t *testing.T) { // itself, however many times it's called (the static-site rebuild that // happens on every offer's reconcile). for i := 0; i < 3; i++ { - buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, schemas.StorefrontProfile{}, cache.get) + buildOfferBundles([]*monetizeapi.ServiceOffer{offer}, schemas.StorefrontProfile{}, cache.getSettled, nil) } if fetchCount != 2 { t.Fatalf("fetchCount after 3 bundle rebuilds = %d, want 2 (buildOfferBundles must not fetch)", fetchCount)