Submit an issue View all issues Source
MIR-1284

schemagen: emit real Validate() bodies instead of empty stubs with stale comments

Open public
phinze phinze Opened Jul 1, 2026 Updated Jul 2, 2026

The serverconfig generator (pkg/serverconfig/cmd/configgen) emits a Validate() method for every config type, but the body is an empty stub carrying a copy-pasted // Check for port conflicts in <Type> comment, regardless of whether the type has anything to do with ports. CodeRabbit flagged it on the new AppVersionConfig (no ports in sight).

Two easy wins here:

  • Drop the misleading boilerplate comment from the template.
  • Optionally emit real validation from the schema metadata we already have: duration-string fields (retention_period, gc_keep_duration) could be format-checked, int fields could get range/non-negative checks. Today a malformed retention_period parses to 0 and silently falls back to the default rather than erroring at startup, which is safe but quiet.

Small, template-level change in configgen; not specific to any one config. Split out of MIR-329, where the stub surfaced on the new app_version config.

Related cleanup: collapse the duplicated defaults

The version retention defaults (retention_count / retention_period) currently live in two places: serverconfig/schema.yml (the real source of truth, what fills a fresh server) and controllers/version DefaultGCConfig(), which repeats them. The controller copy exists only as a fallback for when the config value is unset/zero/invalid: coordinate.go does if configValue > 0 { override }, so a bad value falls back to the code default instead of pruning everything.

Once this validation lands (guaranteeing a sane, defaulted value), coordinate.go can pass the config values straight through and the controller fallback can drop the retention numbers, leaving serverconfig as the single source of truth. Note CheckInterval stays controller-only since it isn't user-configurable, so DefaultGCConfig() doesn't disappear entirely, it just stops carrying the duplicated retention numbers.