docs(migration): expand guide on TLS nuances#331
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigration docs add a pre-apply TLS warning and replace the TLS section with detailed guidance on CA Secret handling, SAN/DNS coverage for adopted and replacement members (mixed-window), wildcard SAN requirements, silent failure modes, and etcdctl validation commands. ChangesTLS Migration Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the migration documentation in docs/migration.md to provide detailed guidance on TLS preparation, including CA locations, SAN coverage requirements, and the necessity of wildcard certificates to avoid silent member replacement failures. The review feedback suggests minor readability improvements, such as adding a missing comma and rephrasing a repetitive sentence, as well as a critical addition to remind users to configure the necessary TLS environment variables when running etcdctl validation commands.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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 `@docs/migration.md`:
- Around line 277-280: Update the fenced code blocks that contain the etcdctl
commands so they include the shell language tag by changing the opening
triple-backtick fences to ```sh for the blocks that contain "etcdctl endpoint
health --cluster" / "etcdctl endpoint status --cluster -w table" and the
subsequent similar block (also noted at the later block covering the lines
flagged 284-287); ensure both fenced blocks are updated to start with ```sh so
markdownlint MD040 is satisfied and the commands render as shell.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
…exts The branch requires `smoke (manifest)` and `smoke (helm)` status checks — the per-leg names a `strategy.matrix` produces when it RUNS. But a matrix job skipped via job-level `if:` collapses to a single check named just `smoke`, so the two required contexts are never reported and sit forever in "Expected", blocking docs-only PRs (e.g. #331) despite everything else passing. Split the matrix into two plain jobs whose `name:` equals each required context exactly. A skipped plain job still reports its check under its own name (as `kamaji-datastore` already does in e2e.yml), so docs-only PRs now satisfy both gates while real release-machinery changes still run both smokes. Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
a16863c to
e5fd25c
Compare
Summary by CodeRabbit
--applyTLS preparation warning clarifying that dry-run coverage is partial.etcdctlvalidation steps with the relevant x509 error signature.