Repository navigation
Add configuration template catalog and renderer - #150
Conversation
Manager and host agent need one template contract for first-install configuration, including missing host values and immutable template ids. Keep the initial catalog empty until product-owned content is supplied, while requiring a template for reconverge installs. Closes #148
Cover supplied empty strings and errors after substitution so callers can rely on verbatim host values without exposing them in diagnostics. Part of #148
|
[Reviewer Round 1] The PR delivers the issue in full and the renderer is correct, but one test should be fixed before merge: it pins the catalog as empty, so the next entry breaks it even though the module docs say nothing else needs to change. Verdict: request changes. This is a small change: remove three assertions. What the PR delivers
Findings1. A test pins the catalog as empty, contradicting the documented add-an-entry procedure (should fix)
assert_eq!(CATALOG, []);
assert!(requires_template("reconverge"));
assert_eq!(templates_for("reconverge").count(), 0);
assert!(find("reconverge", "default").is_none());The module docs written in this PR ( Once #149 adds reconverge's first entry, The acceptance criterion "the catalog is empty …" describes the state this PR ships. It is not an invariant for a test to lock in; the test plan only asks that, on the real catalog, Fix: delete the three reconverge-specific emptiness assertions. Keep 2. PR hygiene (nits)
3. Minor observations (no change needed)
Once the three assertions in finding 1 are removed (and ideally the PR body and title tidied), I would approve. |
|
[Review Verdict Round 1: NOT_APPROVED] |
The catalog starts empty but is intended to grow without changing its lookup tests. Keep the required-component checks while leaving catalog contents to the invariant and file-correspondence tests. Part of #148
|
[Author Round 1] Pushed commit
All required local checks and the new CI run passed. No PR comments or checkpoint markers were posted. |
|
[Reviewer Round 2] [Reviewer Round 2] Verdict: approve. Round 1 items
New findingsNone. The only change since round 1 is the test edit above. The rest of the diff (
The commit message follows the repository rules: imperative title under 50 characters, no prefix, no issue number in the title, and |
|
[Review Verdict Round 2: APPROVED] |
Suggested squash commitTitle Body |
Summary
Adds
config_templateas the shared catalog and renderer for named first-install configurations. It exposes English and Korean metadata, the set of components requiring a template, and exact lookups by component and id. The catalog starts empty, withreconvergerequiring a template; its first entry follows in #149.The renderer recursively substitutes five host-value tokens in TOML string values, rejects placeholders in keys, unknown or partial placeholders, and missing host values, and returns deterministic TOML serialization. Supplied values round-trip verbatim without being re-scanned.
Adds renderer and lookup tests, catalog invariant checks, and file-correspondence checks. Documents the interim catalog, secret-free templates, immutable content per id, and the deliberate exception to the crate’s product-neutral scope. Updates the README, crate documentation, AGENTS.md, and changelog.
Closes #148
Deviations from the issue
None
Test plan
Display, exact case-sensitive lookups, fixture catalog order, and that onlyreconvergerequires a template. Catalog invariant and file-correspondence tests apply to future entries without requiring test edits.cargo fmt -- --check --config group_imports=StdExternalCratecargo clippy --all-targets -- -D warningscargo clippy --all-targets --features test-support -- -D warningsRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --document-private-items --features test-supportcargo testcargo test --features test-supportcargo test --features test-supportcargo build --all-targets --features test-support**/*.mdusing.markdownlint-cli2.yaml