57/K1: A module declares its config with type, description and validation (issue 660) #320
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: novox/mesh-controller#320
Reference in New Issue
Block a user
Design 57 package K1 (issue 660, ADR 0348 §1, §4, §5, §6, §8, §11).
What changed
config,takes-configand${config:<key>}, withsettings,takes-settingsand${setting:<key>}read as aliases. A config value declarestype(number, string, boolean; object or list only when trusted),description, validation (enum,minimum,maximum,pattern, the pattern matching the whole value),trusted, optionalpredefined,defaultorrequired, and a new per-valueruntime: truefor a value the module's code reads itself.Manifest.UnmarshalJSONreads the config form into the fields the settings form fills (Settings, a mergeable file's takes-settings,${setting:<key>}), andMarshalJSONwrites it back in the config form. So${config:<key>}fills exactly as${setting:<key>}does, and the mesh holds the manifest as its author wrote it. A settings declaration is written byte for byte as before.internal/catalogue/config.go,ConfigProblems), each by name with what to do:predefinedis refused saying the list is not yet present. The check against the list is built and tested with a stand-in list (predefinedKeys); K2 only has to fill it;${config:}in a file, unit, owner, contribution or served fact, or named in atakes-config) norruntime: true. This is skipped while the manifest still names templates, and judged again once their text is read (WithTemplatesFrom,Resolve);reload-onorrestart-onnaming a file the module does not declare. Allowed: its own resource ids, another module's<module>.<id>, and the ids the mesh composes for it (composed-jails,filtering,certificate,certificate-authority,bus-users,secret-*,bound-*,needs-*,received-*,grant-*,access-*);takes-config;puid/pgidintakes-config.SettingProblems,TakenSettingsProblems,WrittenSettingsProblems). Config-form fields undersettings, andkind/whyunderconfig, are refused and the refusal says which form to use. A required value nothing sets composes as today's operator-own setting does: it is refused by name at composition (UnsetSettingError). Rules shared with the settings form are said in the config form's words for a module in the config form.Deliberate choices, for the reviewer
domainmust berequired, not defaulted) apply only to modules in the config form. ADR 0348 §11 says a module not yet moved keeps today's form. Repositories outside the catalogue (the media catalogue and the applications, package T6) are not checked here.takes-configkey does not have to be declared underconfig. K1's list does not include that refusal, but ADR 0348 §4 implies it. I left it for the packages that move modules (T2–T5) or for R1 to decide.Left for other packages
internal/inventory/mirrors.go): #315 was not merged when I started, so this pull request does not widen it. Once #315 merges, the bucket's value should carry the new named fields ofSettingDeclaration(type, description, enum, minimum, maximum, pattern, trusted, predefined, required, runtime). They are on the struct and in its JSON, so this needs no new reading, only the bucket's value type.predefinedKeys.How it is proven
Tests in
internal/catalogue/config_test.go. They failed before this change (they did not compile without it) and pass now:TestConfigRefusesAValueWithoutTypeOrDescription,…WithNeitherDefaultNorRequired,…ADefaultFailingItsValidation,…AnObjectOrListNotTrusted,…APredefinedKeyNotOnTheListOrOfAnotherType,…AnUntrustedValueFallingBackToATrustedKey,…AValueNeitherPlacedNorReadAtRuntime,…APlaceholderNamingAnUndeclaredValue,…AReloadOrRestartOnNamingAnUndeclaredFile,…AMergeableFileNotNamingItsKeys,…AMeshWordOrAnAccountFactInTakesConfig;TestAManifestMixingTheConfigAndSettingsFormsIsRefused,TestEachFormKeepsItsOwnDeclaration,TestAModuleInTheConfigFormRegisters,TestAConfigManifestIsWrittenBackInItsOwnForm,TestASettingsManifestIsWrittenBackAsBefore;TestAModuleRewrittenFromSettingsToConfigComposesByteIdentically. dunst is written in both forms and composed on a machine with no layer, with the node's layer, and with the mesh's layer under the node's. The composed declarations are equal byte for byte.Every catalogue manifest still registers. I built a controller from this branch and ran
mesh-controller module check modules/*/module.jsonover mesh-catalogmain(128 manifests) and over itsfeat/57-config-and-provisioningbranch (128 manifests). Both exit 0. Their output is identical, byte for byte, to that of a controller built from the base branch atdc36536. The real dunst manifest rewritten toconfigalso passesmodule check.Reviewer's checklist
internal/catalogueonly: the newconfig.go, plus small hooks inmanifest.go(UnmarshalJSON, MarshalJSON, ParseManifest),setting_defaults.go(declaration fields;Effectiveskips a value with no default) andsetting_takes.go(WrittenSettingsProblemsdefers toConfigProblemsfor the config form). It does not touch F2's functions (settings.go, inventory/catalogue.go, modules.go), F3's or V1's. It is rebased on F2 #316 and V1 #317 as merged.module checkoutput is identical over both catalogue branches.SettingDeclaration's fields are decoded with unknown fields refused, andconfig/takes-configare read before anything writes them. The node-engine is untouched, because composition output is unchanged.gofmt -l cmd internal examplesprints nothing;go vet ./...passes;sh merge-check.shexits 0 withMESH_TEST_POSTGRESset to a throwaway PostgreSQL. That run includesgo test -race ./...and every listed store test, which all passed.Delivery
novox/mesh-controller@9aa033269495— delivered since 2026-10-14T05:25:55ZDelivery plan — builds nothing: the change touches no module of the mesh's graph
Transitions
The commit's note under
refs/notes/mesh-plankeeps every transition:git log --notes=mesh-plan.Verdict: CHANGES at
ae071e2b59Independent review of K1 (issue 660), checked against ADR 0348 §1, §4, §5, §11 and How it is checked, and design 57's K1.
What I ran (worktree at
ae071e2, basedc36536):gofmt -l cmd internal examplesprinted nothing.go vet ./...passed.sh merge-check.shwith a throwaway PostgreSQL on 127.0.0.1:55432 (dynamic_shared_memory_type=mmap) exited 0. It ran with the race detector, and every listed store test passed.module check modules/*/module.jsonover mesh-catalog main (ebf7f37, 128 manifests) and feat/57 (8aa37e8, 128 manifests). All four runs exited 0, and the head's output matched the base's byte for byte.configandresourcesare semantically equal to the input, no key is lost, and the declarations are equal when read again.What I found holds:
configbesidesettings,settingswithtakes-config, and${setting:}in a config module.TypedSettingProblemsrefuses only fields that the strict parser refused before as unknown fields, so no settings-form manifest is newly refused.takes-*is stripped at composition (settings.go:116), and the declarations compose byte-identically, so nothing new reaches the node-engine's strict parse. An unknown field underconfigis still refused.Findings:
configTakesProblems) accepts atakes-configkey that is not declared underconfig. I confirmed this with a probe:takes-config: ["position","theme","nobody-declared"]registers. ADR 0348 §4 says "A module declares every value it reads". A mergeable file's taken key is a value the module reads, so §4 requires this refusal. §11 only says every mergeable file names its keys, and How it is checked does not list the refusal explicitly. There are three reasons to change it now. First, an undeclared key has no type, validation or trusted mark, so a mergeable file could take a value that decides what runs without §8's trusted rule applying. Second, §10'sconfigverb refuses undeclared keys, so such a key could never be set. Third, no module uses the config form yet, so adding the refusal now refuses nothing, while adding it after T2–T5 move modules could refuse modules that have already moved. Refuse an undeclared key intakes-configfor the config form, with a test.settings set notifier <file>". My probe TestRVRequiredUnset shows this.inConfigWordscovers only the registration problems. Say it in the config form's words whenm.DeclaresConfig(), or leave it to K3, which replaces the verb.Resolve.Manifest.Resolvedoes not re-parse. It is re-judged only because its callers (the merge gate at merge_gate.go:608-613 and the controller's take-in at build.go:801) marshal the result and parse it again. ComposingResolve's output in memory leaves${config:}unfilled, and the unconsumed-placeholder rule refuses it loudly, so this is not a bug today. Reword the comment to name the callers.A key a mergeable file took had no type, validation or trusted mark, and the config verb could never set it (ADR 0348 §4). A required config value nothing sets was refused naming ${setting:} and settings set, words the module does not use. Review of mesh-controller#320 (issue 660). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>Verdict: APPROVE at
9aa0332Re-review of the three findings of review 264.
Checked:
dc36536: output identical (131 lines each, exit 0).60d390a, F2b #321 merged): clean, no conflicts.Note, not for this pull request: merge-check.sh as shipped fails the store-tests drop check, because origin/main in the clone now carries #319 (two provider-hold tests added to testdata/store-tests) that feat/57 does not have yet. Run against the merge base instead of origin/main, it passes (exit 0). Merging main into feat/57 clears this for every package.