docs(config): reconcile Slack and Save key documentation with code (#621)#631
Conversation
The Slack middleware (deprecated, scheduled for removal in v1.0.0) only accepts slack-webhook and slack-only-on-error. docs/CONFIGURATION.md and docs/QUICK_REFERENCE.md previously listed slack-url (typo for slack-webhook), slack-channel, slack-mentions, slack-icon-emoji, and slack-username -- none of those keys exist on middlewares.SlackConfig, so operators copying the snippets hit 'Unknown configuration key' warnings. Same UX failure mode as #604; the right disposition for a deprecated middleware is to remove the misleading docs surface and point operators at the supported path: a [webhook "name"] section with preset = slack provides channel routing, mentions, custom username/avatar, retries, and multi-service support. A new test (TestBuildFromString_SlackUndocumentedKeysWarn) locks in the rejection contract so re-adding the keys to the docs without a code-side plan re-fires this assertion. Refs: #621 Signed-off-by: Sebastian Mendel <[email protected]>
…rmat/-retention middlewares.SaveConfig accepts restore-history and restore-history-max-age (see SaveConfig.RestoreHistory / RestoreHistoryMaxAge) but the docs never mentioned them, so operators tuning history replay had to read the source. Conversely, docs/CONFIGURATION.md documented save-format = json|text and save-retention = 30d -- neither key has a struct field, so operators following the docs hit 'Unknown configuration key' warnings. Hybrid fix per the issue: - Document restore-history / restore-history-max-age with their actual semantics (sourced from the SaveConfig field doc comments). - Drop save-format / save-retention from the docs. Output format and retention policy are operator-side concerns today; if there's appetite for implementing them, that's a follow-up with its own design. Two regression tests lock in both halves of the contract: - TestBuildFromString_SaveUndocumentedKeysWarn fires if save-format / save-retention come back to the docs without struct fields landing. - TestBuildFromString_SaveRestoreHistoryKeysAccepted fires if the restore-history* keys regress to 'Unknown configuration key'. Refs: #621 Signed-off-by: Sebastian Mendel <[email protected]>
…e keys PR #618 fixed webhook-* docs/struct drift. Issue #621 found two more instances of the same class (Slack and Save). This test catches the struct-side variant mechanically: it walks every squash-embedded middleware struct in Config.Global, reads each field's mapstructure tag, and asserts the key is mentioned somewhere in the operator-facing docs (CONFIGURATION.md, webhooks.md, QUICK_REFERENCE.md, TROUBLESHOOTING.md, README.md). The check is intentionally lenient -- single substring match in any of those docs counts. It is not enforcing per-field prose; it is catching gross drift where a struct field ships without a single docs reference, which was the failure mode for restore-history / restore-history-max-age prior to this issue. Refs: #621 Signed-off-by: Sebastian Mendel <[email protected]>
github-actions
Bot
added
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #631 +/- ##
==========================================
- Coverage 87.38% 87.36% -0.02%
==========================================
Files 88 88
Lines 10722 10722
==========================================
- Hits 9369 9367 -2
- Misses 1112 1113 +1
- Partials 241 242 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR reconciles operator-facing configuration documentation for the deprecated Slack middleware and the Save middleware with the actual mapstructure keys accepted by the codebase, and adds regression tests to prevent future docs/code drift in Config.Global.
Changes:
- Remove Slack keys that were documented but never supported (keeping only
slack-webhook/slack-only-on-error) and annotate deprecation guidance. - Document existing Save global keys (
restore-history,restore-history-max-age) and remove unimplemented Save keys (save-format,save-retention) from docs. - Add CLI tests that lock in (a) warning behavior for rejected keys and (b) a reflection-based drift guard ensuring global middleware keys are mentioned in at least one docs file.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/QUICK_REFERENCE.md | Removes unsupported Slack key from quick reference and adds deprecation guidance. |
| docs/CONFIGURATION.md | Updates Slack global/job examples to only include supported keys; updates Save docs to reflect real config surface. |
| cli/slack_drift_test.go | Adds regression test asserting warnings for previously documented-but-unsupported Slack keys. |
| cli/save_drift_test.go | Adds regression tests for Save: warnings for unimplemented keys; acceptance for restore-history keys. |
| cli/config_drift_test.go | Adds reflection-based test to detect undocumented Config.Global embedded middleware keys. |
| CHANGELOG.md | Records documentation and test additions under Unreleased. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces automated drift detection to ensure configuration keys in the code are documented. It adds a reflection-based test, TestConfigGlobalKeysAreDocumented, and specific regression tests for Slack and Save middleware configurations. The documentation has been updated to remove unsupported keys and include previously undocumented ones. Feedback suggests using FieldByName instead of a hardcoded index in the reflection logic to make the test more robust against future struct changes.
- Drop stale "slack-channel" from validator's optional-fields allowlist (and its mirrors in validator_boundary_test.go and validator_mutation_test.go). Code review noted no struct field maps to this key anymore - dead allowlist entry. - Fix slack-url -> slack-webhook typo in docs/packages/cli.md (perpetuated the same drift this PR set out to kill - the drift test does not scan docs/packages/, so it didn't catch this). - Make config_drift_test.go robust against Config struct restructuring by looking up the Global field by name instead of by index 0. Add TODO(#635) noting the walk should extend to direct Global fields once the remaining undocumented keys get documented. Signed-off-by: Sebastian Mendel <[email protected]>
|
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
|
Summary
Surfaced during the #618 review as a sibling-bug class to #604: two more middlewares ship docs that don't match their struct fields, so operators copying the snippets hit
Unknown configuration keywarnings (Slack) or have to read the source to discover real keys (Save).Fixes #621.
Disposition per finding
Slack (deprecated middleware, scheduled for removal in v1.0.0 — supported path is the generic webhook system with
preset = slack):slack-url— removed from docs (typo forslack-webhook)slack-channel— removed from docs (use[webhook "name"]for routing)slack-mentions— removed from docs (webhook system handles this)slack-icon-emoji— removed from docsslack-username— removed from docsSave:
restore-history— documented (existed onSaveConfig, undocumented before)restore-history-max-age— documented (existed onSaveConfig, undocumented before)save-format = json|text— removed from docs (unimplemented; left as follow-up if there's appetite)save-retention = 30d— removed from docs (unimplemented; left as follow-up)Mechanical drift guard
New
TestConfigGlobalKeysAreDocumentedincli/config_drift_test.gowalks the embedded middleware structs inConfig.Globalvia reflection and asserts everymapstructurekey is mentioned in at least one operator-facing docs file. Lenient by design: substring match in any ofdocs/CONFIGURATION.md,docs/webhooks.md,docs/QUICK_REFERENCE.md,docs/TROUBLESHOOTING.md,README.mdcounts. Catches the next #604/#621 mechanically.Atomic commits
79eef13docs(slack): drop documented-but-rejected Slack middleware keys0841a8fdocs(save): document restore-history keys, drop unimplemented save-format/-retention49eeab6test(cli): add reflection contract test for Config.Global mapstructure keysTest plan
TestBuildFromString_SlackUndocumentedKeysWarn— locks in rejection contract for the five removed Slack keys.TestBuildFromString_SaveUndocumentedKeysWarn— locks in rejection contract forsave-format/save-retention.TestBuildFromString_SaveRestoreHistoryKeysAccepted— verifiesrestore-history/restore-history-max-ageare accepted and values land onConfig.Global.SaveConfig.TestConfigGlobalKeysAreDocumented— passes (no remaining drift inConfig.Global).go test -race ./cli/ ./middlewares/— green.golangci-lint run --timeout=3m— 0 issues.[Unreleased]→### Documentation(Slack + Save) and### Tests(contract test).