ignore unknown namespaces in ParseStrict for forward compatibility

CHANGELOG.md

@@ -8,6 +8,10 @@ Please choose versions by [Semantic Versioning](http://semver.org/).
 * MINOR version when you add functionality in a backwards-compatible manner, and
 * PATCH version when you make backwards-compatible bug fixes.
 
+## Unreleased
+
+- `maintainerconfig`: `ParseStrict` now ignores unknown top-level namespaces instead of rejecting them, so a repo adopting a newer bot's namespace no longer breaks binaries built before that namespace existed. Typos INSIDE a known namespace stay fatal, which is the property `ParseStrict` exists for. Fixes a silent prod wedge: adding `goUpdate:` to two repos made the deployed github-releaser-agent fail planning with `field goUpdate not found`, clearing the task assignee so the release never tagged and never retried. Trade-off: a misspelled namespace is now indistinguishable from a newer one, so both are ignored and logged at WARNING.
+
 ## v0.48.1
 
 - `maintainerconfig`: add `goUpdate.autoUpdate` bool to a new `GoUpdateConfig`, following the existing `ReleaseConfig`/`PrReviewerConfig` shape — the per-repo consent flag the upcoming github-update-go-watcher gates on. Defaults false (key, section, or file absent all read false). Schema-only in this repo; the watcher itself ships as a follow-up.

maintainerconfig/maintainerconfig.go

@@ -17,16 +17,24 @@
 // MaintainerConfig — every consumer imports this one type, so there is
 // never a divergent copy of the file's shape.
 //
-// Unknown fields (top-level OR nested) are REJECTED at parse time
-// (yaml.NewDecoder + KnownFields(true)). This catches typos like
-// `changelogRwrite` or `prRevierer` that would otherwise produce a
-// silent default-false config — a high-trust .maintainer.yaml is
-// load-bearing for release gating, so a typo must fail loudly. To
-// add a new bot's namespace, extend MaintainerConfig with the new
-// field FIRST (one PR), then deploy the bot (next PR); the brief
-// window between the two is the only time a forward-incompat
-// .maintainer.yaml would error, and it errors loudly rather than
-// silently downgrading.
+// Typos in a KNOWN namespace are REJECTED by ParseStrict (`changelogRwrite`
+// inside `release:`), because a high-trust .maintainer.yaml is load-bearing for
+// release gating and a typo must fail loudly rather than produce a silent
+// default-false config.
+//
+// UNKNOWN top-level namespaces are IGNORED, even by ParseStrict. This is
+// forward compatibility, and it is not optional: one schema is read by several
+// independently-deployed binaries, so a repo adopting a new bot's namespace
+// must not break the bots that have not been rebuilt yet.
+//
+// This package previously rejected unknown top-level keys too, on the
+// assumption that "add the field, then deploy the bot" left only a brief
+// incompatible window. It does not. The window lasts until every consumer is
+// rebuilt AND redeployed, and until then the failure is severe and quiet:
+// on 2026-08-16, adding `goUpdate:` to two repos made the deployed
+// github-releaser-agent fail its planning step with
+// `field goUpdate not found`, which cleared the task's assignee and wedged the
+// release. No tag, no retry, no alert — the repo simply stopped releasing.
 //
 // Parse does NO I/O — fetching the bytes is each consumer's job (the
 // watcher fetches via the GitHub API; the agent reads the cloned workDir
@@ -36,8 +44,11 @@ package maintainerconfig
 import (
 	"bytes"
 	"context"
+	"reflect"
+	"strings"
 
 	"github.com/bborbe/errors"
+	"github.com/golang/glog"
 	"gopkg.in/yaml.v3"
 )
 
@@ -131,10 +142,16 @@ func Parse(ctx context.Context, content []byte) (MaintainerConfig, error) {
 }
 
 // ParseStrict unmarshals a `.maintainer.yaml` document with `KnownFields(true)`
-// so any unrecognized top-level or nested key produces a wrapped error.
-// Use this when the caller wants typos like `changelogRwrite` to fail loudly
-// (e.g. the github-releaser planning step where a silent zero-value would
-// disable the rewrite pipeline without operator signal).
+// applied to the namespaces this binary knows, so an unrecognized key INSIDE a
+// known namespace produces a wrapped error. Use this when the caller wants
+// typos like `changelogRwrite` to fail loudly (e.g. the github-releaser
+// planning step where a silent zero-value would disable the rewrite pipeline
+// without operator signal).
+//
+// Unknown top-level namespaces are ignored rather than rejected — see the
+// package doc for why that is required, and for the failure it prevents. The
+// cost is that a misspelled namespace is indistinguishable from a newer one;
+// both are ignored, and both are logged at WARNING.
 //
 // The lib's lenient Parse remains the default for fleet readers (watcher).
 func ParseStrict(ctx context.Context, content []byte) (MaintainerConfig, error) {
@@ -154,6 +171,19 @@ func parseInternal(
 		// short-circuit keeps the contract crisp.
 		return cfg, nil
 	}
+	if strict {
+		// Drop namespaces this binary does not know about BEFORE the strict
+		// decode. KnownFields(true) cannot distinguish "typo inside release:"
+		// from "namespace added by a newer schema", and conflating those makes
+		// every additive schema change a fleet-wide outage. Filtering first
+		// keeps typos inside known namespaces fatal, which is the property
+		// ParseStrict exists for.
+		filtered, err := dropUnknownNamespaces(ctx, content)
+		if err != nil {
+			return MaintainerConfig{}, err
+		}
+		content = filtered
+	}
 	dec := yaml.NewDecoder(bytes.NewReader(content))
 	if strict {
 		dec.KnownFields(true)
@@ -163,3 +193,54 @@ func parseInternal(
 	}
 	return cfg, nil
 }
+
+// dropUnknownNamespaces removes top-level keys that MaintainerConfig does not
+// declare, so a document written against a newer schema still parses here.
+// Values are round-tripped as yaml.Node, which preserves the nested content
+// verbatim for the strict decode that follows.
+func dropUnknownNamespaces(ctx context.Context, content []byte) ([]byte, error) {
+	var raw map[string]yaml.Node
+	if err := yaml.Unmarshal(content, &raw); err != nil {
+		return nil, errors.Wrap(ctx, err, "unmarshal .maintainer.yaml")
+	}
+	known := knownNamespaces()
+	for key := range raw {
+		if _, ok := known[key]; ok {
+			continue
+		}
+		// Warning, not V(2). This is the one downside of tolerating unknown
+		// namespaces: a misspelled one (`prRevierer:`) is now indistinguishable
+		// from a genuinely newer one, so neither fails the parse. Logging it
+		// loudly is what keeps a typo discoverable. Volume is low — only the
+		// strict path filters, and that runs once per release, not per fleet
+		// scan (the watcher uses lenient Parse, which never reaches here).
+		glog.Warningf(
+			"ignoring unknown .maintainer.yaml namespace %q — either a newer schema than this binary, or a typo",
+			key,
+		)
+		delete(raw, key)
+	}
+	data, err := yaml.Marshal(raw)
+	if err != nil {
+		return nil, errors.Wrap(ctx, err, "marshal filtered .maintainer.yaml")
+	}
+	return data, nil
+}
+
+// knownNamespaces reads the yaml tags off MaintainerConfig rather than
+// hardcoding a list, so adding a namespace stays a one-field edit.
+func knownNamespaces() map[string]struct{} {
+	t := reflect.TypeOf(MaintainerConfig{})
+	out := make(map[string]struct{}, t.NumField())
+	for i := 0; i < t.NumField(); i++ {
+		tag := t.Field(i).Tag.Get("yaml")
+		if tag == "" || tag == "-" {
+			continue
+		}
+		name, _, _ := strings.Cut(tag, ",")
+		if name != "" {
+			out[name] = struct{}{}
+		}
+	}
+	return out
+}

maintainerconfig/maintainerconfig_test.go

@@ -182,13 +182,41 @@ var _ = Describe("Parse", func() {
 		Expect(cfg.PrReviewer.AutoApprove).To(BeTrue())
 	})
 
-	It("ParseStrict rejects unknown top-level field", func() {
-		_, err := maintainerconfig.ParseStrict(
+	It("ParseStrict ignores an unknown top-level namespace and still reads known ones", func() {
+		// Forward compatibility. A repo may adopt a namespace belonging to a
+		// bot this binary predates; that must not fail the parse, because the
+		// binary reading it may not be rebuilt for days. Rejecting this wedged
+		// two repos' releases on 2026-08-16 when `goUpdate:` was introduced.
+		cfg, err := maintainerconfig.ParseStrict(
 			ctx,
 			[]byte("build-fix:\n  enabled: true\nprReviewer:\n  autoApprove: true\n"),
 		)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(cfg.PrReviewer.AutoApprove).To(BeTrue())
+	})
+
+	It("ParseStrict ignores the goUpdate namespace on a binary that predates it", func() {
+		// The exact document that broke github-releaser-agent in prod.
+		cfg, err := maintainerconfig.ParseStrict(
+			ctx,
+			[]byte(
+				"release:\n  autoRelease: true\n  changelogRewrite: false\nprReviewer:\n  autoApprove: true\ngoUpdate:\n  autoUpdate: true\n",
+			),
+		)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(cfg.Release.AutoRelease).To(BeTrue())
+		Expect(cfg.PrReviewer.AutoApprove).To(BeTrue())
+	})
+
+	It("ParseStrict still rejects a typo inside a known namespace", func() {
+		// The property ParseStrict exists for must survive the change above:
+		// forward compatibility applies to unknown NAMESPACES, never to
+		// unknown keys inside a namespace this binary owns.
+		_, err := maintainerconfig.ParseStrict(
+			ctx,
+			[]byte("build-fix:\n  enabled: true\nrelease:\n  autoReleese: true\n"),
+		)
 		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(ContainSubstring("unmarshal .maintainer.yaml"))
 		Expect(err.Error()).To(ContainSubstring("not found"))
 	})
 
@@ -212,13 +240,19 @@ var _ = Describe("Parse", func() {
 		Expect(cfg.Release.AllowFork).To(BeTrue())
 	})
 
-	It("ParseStrict rejects typo in top-level prReviewer key", func() {
-		_, err := maintainerconfig.ParseStrict(
+	It("ParseStrict no longer rejects a typo'd top-level namespace", func() {
+		// The accepted cost of forward compatibility, pinned so it is a
+		// decision rather than a surprise: a misspelled NAMESPACE is
+		// indistinguishable from one belonging to a newer bot, so it is
+		// ignored instead of fatal, and the gate it meant to set stays false.
+		// It is logged at WARNING so the typo is still discoverable.
+		// Typos INSIDE a known namespace remain fatal — asserted above.
+		cfg, err := maintainerconfig.ParseStrict(
 			ctx,
 			[]byte("prRevierer:\n  autoApprove: true\n"),
 		)
-		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(ContainSubstring("unmarshal .maintainer.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		Expect(cfg.PrReviewer.AutoApprove).To(BeFalse())
 	})
 
 	It("release.changelogRewrite: non-bool string value -> wrapped error", func() {