fix(rpk-docs): never delete pages for plugins absent from the tree - #221
fix(rpk-docs): never delete pages for plugins absent from the tree#221JakeSCahill wants to merge 5 commits into
Conversation
rpk core ships only an install/uninstall/upgrade shim for managed plugins. When the generation environment lacks a plugin binary, the plugin's real commands are missing from --print-tree, and the generator treated its pages and nav entries as stale and deleted them. This happens every pre-GA window: the plugin publisher only promotes stable X.Y.Z releases, so 'rpk <plugin> install' resolves nothing until GA. The v26.2.1-rc2 regen deleted the five rpk k8s multicluster pages this way (redpanda-data/docs#1831). Auto-detect known plugins whose subtree is absent or shim-only and preserve their existing pages and nav entries, deduplicating against entries regenerated in the same run. Also track failed plugin installs in the source-build path for the generation log. Verified against a copy of the docs beta rpk reference with a plugin-less v26.2.1-rc2 tree: all four shim-only plugin directories preserved, 26 plugin nav entries kept, zero plugin pages deleted, while non-plugin regeneration proceeds normally.
✅ Deploy Preview for docs-extensions-and-macros ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe rpk documentation generator now accepts and auto-detects protected plugins, preserves their existing navigation entries, and excludes their generated directories from stale-file deletion. The Docker-based plugin installation flow also records plugin names whose installation fails. Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/rpk-docs/generate-rpk-docs.js`:
- Around line 2639-2667: Add regression tests covering auto-protection in the
plugin detection logic around PLUGIN_SHIM_COMMANDS, KNOWN_PLUGIN_NAMES, and
effectiveProtectedPlugins when a known plugin is absent or has only shim
commands. Verify protected plugin directories survive stale-file cleanup, and
verify regenerated navigation does not contain duplicate plugin xrefs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 56f4e31d-1293-450e-8e54-26e666d697d4
📒 Files selected for processing (2)
tools/rpk-docs/generate-rpk-docs.jstools/rpk-docs/rpk-docs-handler.js
Command help text often underlines section titles with a run of = or - characters and uses ALL-CAPS column-header rows in aligned sample output. The section parser promoted the titles to headings but left the underline in the content, where a line of 4+ = characters is an AsciiDoc example-block delimiter, producing an unterminated block (seen on rpk cluster brokers decommission-status, and previously on its rpk redpanda admin predecessor). It also promoted multi-space column-header rows such as 'PARTITION REASON' to section headings, splitting sample tables. Consume the underline when it directly follows a section header, treat lines containing multi-space runs as content rather than headers, and warn if any bare delimiter run still reaches section content. Verified by regenerating all 355 pages from the v26.2.1-rc2 tree: the decommission-status sections now render with their tables intact, and no bare delimiter runs remain outside code fences.
…(CodeRabbit) Covers auto-protection when a known plugin is absent or shim-only, explicit protectedPlugins, protected plugin directories surviving the stale-file sweep while genuinely stale pages are still deleted, and preserved nav entries not duplicating entries that are also present in the regenerated output.
…splacing nav entries Protection for managed plugins previously only exempted their pages from the stale-file sweep. Two gaps remained when the tree had a plugin's shim (install/uninstall/upgrade) but not its real commands: - Pages under the plugin subtree were still written, so the parent page (e.g. rpk-k8s.adoc) was regenerated with a shim-only Subcommands table, orphaning the preserved child pages. - Preserved nav entries were appended after all generated entries, so they rendered nested under the wrong parent at the end of the section. Protection now covers the whole pipeline. Protected plugins are detected before the write loop (explicit callers merged with shim-only/absent auto-detection) and generation skips their entire subtree, parent page included, exactly like the fully-absent case. updateNavFile no longer generates entries for protected plugins. It preserves each plugin's previous nav block (parent plus nested children) and splices it back in at its original position, anchored to the nearest preceding surviving entry. In the common shim scenario the nav and every plugin page come out byte-identical to the previous run. The preservation log line now lists only plugins that actually had nav entries preserved instead of every protected plugin.
… plugins
fetchRpkTreeFromLinuxSource collected failedPlugins but never used it.
It now returns { tree, failedPlugins } and the handler threads the list
into both generateRpkDocs calls as explicit protectedPlugins, merged
there with auto-detection, so a failed install can never delete or
rewrite that plugin's existing docs even if the shim heuristic changes.
Also drop the handler's duplicate KNOWN_PLUGINS definition and share the
single constant exported by generate-rpk-docs.js (the handler still
re-exports it for existing consumers).
|
Pushed two commits addressing the review blockers, plus the smaller findings from the same review. Blocker 1: preserved nav entries appended at the end (c2db373)
Blocker 2: shim content overwrote the plugin parent page (c2db373)Protection now covers page writes, not just the deletion sweep. Protected plugins are detected before the write loop ( Cleanups (f2a976a and c2db373)
Test evidenceEnd-to-end harness reproducing the pre-GA shim scenario (seeded full k8s pages and nav, shim-only tree, stale Before this change:
After this change:
New regression tests: preserved entries spliced in place under the plugin parent (nav byte-identical in the shim case), fully absent plugin block kept in its original position, shim run leaves the parent page content untouched and writes no shim pages under the protected subtree, and a fully installed plugin still regenerates pages and sweeps its stale files as usual. One existing test's comment was updated to match the new preserve-in-place semantics (its assertions were placement-agnostic and still pass). Full suite: 33 suites, 760 tests passing ( |
1 participant
Problem
The rpk-docs automation deleted the
rpk k8s multiclusterreference pages in redpanda-data/docs#1831. Root cause, verified in an isolated environment with the v26.2.1-rc2 binary:install/uninstall/upgrade) for managed plugins.multiclusterandversioncome from the separately publishedk8splugin.stableVersionReonly promotes pure X.Y.Z releases, so unpinnedrpk k8s installfails until GA.--print-tree, the generator treated its pages and nav entries as stale and removed them.ai,check, andconnectare equally exposed (all shim-only in a plugin-less tree).Fix
Auto-detect known plugins whose command subtree is absent or shim-only in the current run's tree and preserve their existing pages and nav entries (deduplicating nav lines against entries regenerated in the same run). Callers can also pass
protectedPluginsexplicitly. The source-build path now records failed plugin installs in the log.Second fix: help-text underlines and column headers
Command help text underlines section titles with
=/-runs and uses ALL-CAPS column-header rows in aligned sample output. The section parser left the underlines in section content, where a run of 4+=is an AsciiDoc example-block delimiter — producing an unterminated block on the generatedrpk cluster brokers decommission-statuspage (and itsrpk redpanda adminpredecessor) — and promoted multi-space column-header rows (PARTITION REASON) to section headings, splitting sample tables. The parser now consumes an underline that directly follows a section header, treats multi-space lines as content, and warns if a bare delimiter run still reaches content. Verified by regenerating all 355 pages from the v26.2.1-rc2 tree: zero bare delimiter runs outside code fences (the only matches are legitimate[tabs]block delimiters from overrides).Verification
Ran
generateRpkDocsagainst a copy of the docs repo's beta rpk reference using a clean-environment v26.2.1-rc2 tree (no plugins): all four shim-only plugin directories preserved with a log line each, 26 plugin nav entries kept (including the four multicluster entries #1831 dropped), zero plugin pages deleted, and non-plugin regeneration proceeded normally.node --checkpasses on both changed files.🤖 Generated with Claude Code