Skip to content

Add reconverge's default configuration template (#149) - #151

Merged
CoestPiper merged 4 commits into
mainfrom
CoestPiper/issue-149
Oct 5, 2026
Merged

CoestPiper merged 4 commits into
mainfrom
CoestPiper/issue-149

Conversation

@CoestPiper

@CoestPiper CoestPiper commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds reconverge’s first configuration template, default, so the Central Manager can offer it for first installs. It runs one detector every 5 minutes on the Data Store’s HTTP events using the HttpUriThreat label database and the model unsupervised-default. The engine keeps running before that model exists and starts detecting at the next scheduled run after it is created.

Registers the exact product-owner template with English and Korean catalog texts, adds tests for recursive TOML key sets and typed values plus all five required host tokens, and records the addition in the changelog.

The template body is immutable under the id default. Any later configuration change requires a new file and a new id.

Closes #149

Deviations from the issue

None

Test plan

  • Verify templates/reconverge/default.toml matches Scope 1 byte for byte, including its single trailing newline.
  • Verify CATALOG contains exactly one entry with the required component, id, include_str! body, and exact English and Korean names and descriptions.
  • Verify find("reconverge", "default") returns that entry and templates_for("reconverge") yields exactly that one entry.
  • Render with all five fixture host values and assert every required table’s exact key set, TOML types and values, and array lengths, including start as a string and batch_size as integer 500000.
  • Omit each host value individually and verify ConfigTemplateError::UnresolvedToken names the corresponding token for all five members of TemplateToken::ALL.
  • Run the existing catalog-invariant and file-correspondence tests unchanged against the populated catalog.
  • Cross-check every configuration key and type against reconverge 0.55.0’s Config, DetectorConfig, auth and control configurations, input configurations, LabelDbConfig, and ColumnConfig.
  • Verify the diff contains only the required catalog addition, template, tests, permitted lookup-test updates, and changelog entry, with no other code, documentation, public API, or dependency changes.
  • Verify the changelog entry appears under [Unreleased] → Added, describes the template, and contains no issue or PR reference.
  • Verify the PR description states that default is immutable and later configuration changes require a new file and id.
  • cargo fmt -- --check --config group_imports=StdExternalCrate
  • cargo clippy --all-targets -- -D warnings
  • cargo clippy --all-targets --features test-support -- -D warnings
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --document-private-items --features test-support
  • cargo test
  • cargo test --features test-support
  • Verify all Platform CI jobs pass, including Linux ARM tests and macOS builds.
  • Run recursive Markdownlint over **/*.md, covering all committed Markdown files.
  • Verify the agent-instructions drift check passes.

Verification

Reverified on 2026-10-05 at 85a21599e276a31a5024902d24af3f75198d598b: all 19 Test plan items and all 8 issue acceptance criteria pass. The 633 template bytes (including one trailing newline), exact catalog texts and sole-entry lookups, recursive TOML key sets/types/values/array lengths, all five unresolved-token errors, unchanged catalog invariants and file correspondence, changelog, restricted diff, and immutable-template policy were checked. Every configuration field and type was cross-checked against reconverge 0.55.0.

Formatting, both Clippy configurations, warning-free private-item documentation, and both full test suites passed locally. Each suite passed on the first attempt with 1,256 unit tests, one intentionally ignored fixture-regeneration test, all integration tests, and 54 doctests. Recursive Markdownlint passed against a clean snapshot containing all 11 committed Markdown files. All five CI jobs and every step pass for this commit, including Linux ARM tests, macOS builds, and the agent-instructions drift check. No repository files were changed; no services were required; all verification processes completed. Issue #149 has no parent issue.

Reconverge requires a catalog entry before the Central Manager can
install it. Supply the product owner's first-install configuration and
explain that detection begins once unsupervised-default exists.

Pin the rendered shape and all five host-value tokens in tests so
misspelled fields cannot silently change the engine's configuration.

Closes #149
The empty-catalog wording no longer describes first installs once the
reconverge default entry is available.

Part of #149
@CoestPiper CoestPiper changed the title Add reconverge's default configuration template Add reconverge's default configuration template (#149) Oct 5, 2026
The catalog's localized descriptions are part of the issue contract.
Check both full strings so wording regressions fail the lookup test.

Part of #149
The issue restricts changes to the template, catalog entry, new tests
and changelog. Keep the original module documentation so the completed
change satisfies that scope without an additional documentation edit.

Closes #149
@CoestPiper

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 1]

I recommend approving PR #151. It does what issue #149 asks for and stays in scope; four small non-blocking points are listed below.

How I reviewed it. The branch still carries the commit from #148, so I compared it against that commit, c77d17c, which origin/main contains. The local main is out of date. The real change is three files: CHANGELOG.md, src/config_template.rs and templates/reconverge/default.toml.

Against the issue

  • Template file (Scope 1). I wrote the issue's block out verbatim and ran cmp against templates/reconverge/default.toml. They are byte-identical: 633 bytes, ending in one newline.
  • Catalog entry (Scope 2). src/config_template.rs:47-59 has the right component and id, and the body comes from the template file via include_str!. The English and Korean name and description texts match the issue exactly.
    • reconverge_default_is_the_only_template (src/config_template.rs:718) checks every one of those texts. It also checks that templates_for("reconverge") returns exactly this entry and that CATALOG == [*entry], which covers acceptance criterion 2.
  • Render test (Scope 3). reconverge_default_renders_to_reconverge_config renders the body, parses it, and compares the whole result against a hand-written expected table. It does not assert each table's keys one by one, as the issue sketched, but the result is the same:
    • Two toml::Table values are only equal if every nested table has exactly the same keys, values and value types, and every array has the same length.
    • So an extra or misspelled key fails the test. A start value parsed as a datetime instead of a string fails, and so does batch_size stored as anything other than the integer 500000.
    • The fixture values differ from the issue's examples, which is allowed: the issue says "e.g.". The test reuses the module's existing VALUES fixture.
    • I agree with "Deviations: None" here.
  • Missing-token test (Scope 4). reconverge_default_requires_every_host_value runs the real entry body once for each token in TemplateToken::ALL, and checks that the error names the token that was left out. It follows the pattern of the existing refuses_each_absent_host_value.
  • Existing tests (Scope 5). None of Add the configuration template catalog and its renderer #148's tests were changed. None of them asserted an empty catalog, so none needed to change. Its invariant and file-correspondence tests now run against this entry.
  • Changelog (Scope 6). The entry is under [Unreleased] → ### Added, above Add the configuration template catalog and its renderer #148's entry, and carries no issue or PR reference.
  • No other changes. There are no new dependencies and no API changes.
  • I ran cargo test --lib config_template: all 25 tests pass.

Non-blocking notes

  1. The module docs are now out of date (src/config_template.rs:13-14). They still say: "The catalog is initially empty, while reconverge requires a template, so the manager refuses its first installs." After this PR the manager no longer refuses reconverge installs.
    • Commit 668774a fixed this sentence; 85a2159 put it back, to stay within the issue's "no other change" rule.
    • "Initially" keeps the first half technically true, and the strict reading of the issue is defensible. But the "so the manager refuses its first installs" part now misleads anyone reading the docs.
    • A better outcome would have been a two-line fix listed under ## Deviations from the issue instead of leaving the docs stale silently. Worth a follow-up, or a fix before merge if the human merging agrees.
  2. The expected table copies the template almost line for line. If the template ever gains a typo, a test written by copying it would carry the same typo. That is no weaker than writing out the key lists by hand, as the issue suggested. Still, a short comment pointing to the reconverge 0.55.0 structs each table comes from (as the issue lists them) would help a future reader see where the expected keys come from.
  3. PR hygiene:
    • The body has the closing line twice: Closes #149. and Closes #149. Harmless, but one should go.
    • The PR title ends in (#149). A squash merge would put an issue number in the commit title, which the commit rules forbid. I'd drop it from the title.
  4. Changelog line wrapping. The new changelog entry's first line is 83 characters, while the entries around it wrap at 80 or less. Markdownlint passes, so this is cosmetic only.

Policy

The PR description says the template can never change under the id default, as the issue required, and the module docs already state that rule. The template contains no secret, only the five host-value placeholders.

@CoestPiper

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 1: APPROVED]

@CoestPiper

Copy link
Copy Markdown
Contributor Author

Suggested squash commit

Title

Add reconverge's default configuration template

Body

Reconverge requires a catalog entry before the Central Manager can
install it. Supply the product owner's first-install configuration and
explain that detection begins once unsupervised-default exists.

Pin the rendered TOML shape, localized texts and all five host-value
tokens in tests so misspelled fields and wording regressions cannot
silently change the configuration or its operator guidance.

Closes #149

@CoestPiper
CoestPiper merged commit 86ab32b into main Oct 5, 2026
5 checks passed
@CoestPiper
CoestPiper deleted the CoestPiper/issue-149 branch October 5, 2026 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add reconverge's default configuration template

1 participant