fix(config): normalize Packages fields when preset=lean (#134)#135
Merged
Merged
Conversation
Pre-fix, host.yaml dumped the raw default base_groups (with @standard) and required (without LEAN_EXTRA_PACKAGES) even when packages.preset=lean, while the generated ks.cfg correctly used the lean-stripped effective set. host.yaml lied about what was on the box — bad for the audit story since host.yaml is the source of truth consumed by ks-gen verify and audited by operators. Fix: model_validator(mode="before") on Packages normalizes base_groups + required into the stored fields when preset=LEAN. The in-memory model now matches the install state; model_dump() (used by writer.py to produce host.yaml) becomes accurate. The effective_base_groups and effective_required properties keep their old contract for backward compat — after normalization they just echo the fields. Why mode="before" not "after": StrictModel is frozen project-wide. Pydantic v2 forbids mode="after" validators from returning a different instance on a frozen model (via model_copy or otherwise) — that's the "A custom validator is returning a value other than `self`" warning. Mutating the input dict before construction is the supported path. Also: extracted the default base_groups + required lists to module-level constants (_PACKAGES_DEFAULT_BASE_GROUPS, _PACKAGES_DEFAULT_REQUIRED) so the validator can reference them when the input dict omits the field (default_factory values aren't applied at mode="before" time). The field default_factory lambdas reuse the constants — single source of truth. Idempotent: re-validating an already-normalized cfg is a no-op. Pinned by test_lean_preset_normalization_is_idempotent_on_roundtrip. 7 new tests cover: lean normalizes base_groups into field; lean normalizes required into field; reflected in model_dump (the headline acceptance criterion from #134); roundtrip idempotency; user-supplied required order preserved + lean extras deduplicated; user-supplied custom @group preserved while @standard is dropped; STANDARD preset unchanged (no spurious normalization). No golden snapshot changes: the existing lean snapshots (test_lean_preset, test_container_host_lean) include ks.cfg + tailoring + exceptions but NOT host.yaml, so the bug was invisible to the test suite. ubuntu_minimal does snapshot host.yaml but uses preset=standard. The actual fix verification is end-to-end: regenerating build/unifi/ (lean preset) post-fix produces a host.yaml with no @standard in base_groups and all 5 LEAN_EXTRA_PACKAGES in required. Fixes #134.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #134. Pre-fix,
host.yamldumped the raw defaultbase_groups(with@standard) andrequired(withoutLEAN_EXTRA_PACKAGES) even whenpackages.preset: lean, while the generatedks.cfg's%packagesblock correctly used the lean-stripped effective set.host.yamlwas lying about what was on the box — bad for the audit story sincehost.yamlis the source of truth consumed byks-gen verifyand audited by operators.Fix
model_validator(mode="before")onPackagesnormalizesbase_groups+requiredinto the stored fields whenpreset == LEAN. The in-memory model now matches the install state;model_dump()(used bywriter.pyto producehost.yaml) becomes accurate. Theeffective_base_groups/effective_requiredproperties keep their old contract for backward compat — after normalization they just echo the fields.Why
mode="before"notmode="after"StrictModelis frozen project-wide. Pydantic v2 forbidsmode="after"validators from returning a different instance on a frozen model — that's the "A custom validator is returning a value other thanself" warning that surfaced when I first triedmodel_copy(update=...). Mutating the input dict before construction is the supported path.Default-list extraction
Extracted the default
base_groups+requiredlists to module-level constants (_PACKAGES_DEFAULT_BASE_GROUPS,_PACKAGES_DEFAULT_REQUIRED) so the validator can reference them when the input dict omits the field (Field(default_factory=...)values aren't applied atmode="before"time). The fielddefault_factorylambdas reuse the constants — single source of truth.Idempotency
Re-validating an already-normalized cfg is a no-op. Pinned by
test_lean_preset_normalization_is_idempotent_on_roundtrip—Packages(**Packages(preset="lean").model_dump())produces an equal cfg.End-to-end verification
The lean golden snapshots (
test_lean_preset,test_container_host_lean) includeks.cfg + tailoring + exceptionsbut NOThost.yaml, so the bug was invisible to the existing test suite — that's why it took an operator inspectingbuild/unifi/host.yamlto surface it. The fix verification is end-to-end: regeneratingbuild/unifi/(which usespreset: lean) post-fix produces:Test plan
tests/test_config_schema.pycover:leannormalizesbase_groupsinto the fieldleannormalizesrequiredinto the fieldmodel_dump()for a lean cfg reflects the normalization (the headline acceptance criterion from fix(config): packages.preset=lean — host.yaml dump still shows @standard in base_groups (display lies vs actual install) #134)Packages(**lean_cfg.model_dump())→ same cfg)requiredorder preserved + lean extras deduplicated@grouppreserved while@standardis droppedeffective_*tests (8) still pass — the properties' contract is unchanged (after normalization they echo the field values; on STANDARD they compute as before).host.yaml. Worth a future-followup test that snapshotshost.yamlfor a lean cfg to catch this class of bug.ruff check && ruff format --check && mypy && pytest -q— all four green (963 passed = 956 from end of v0.29.0 + 7 new).BE707B220C995478.Related: #134 (the issue this closes).