feat(decdn_node): opt-in host-side keystore generation + fix provisioning docs - #21
Conversation
…isioning docs Add `decdn_node_generate_keystore` (default false): when true the role mints a fresh eth wallet on the host via `decdn key-gen`, but ONLY when the keystore is absent — it never overwrites an existing (possibly funded) wallet, and it refuses to mint a password beside a pre-existing keystore it couldn't decrypt (the gate fails loud instead so the operator supplies the matching one). The default keeps the operator-provisioned posture; funding + on-chain staking (ADR 019 Phase 2) stay manual. Harden the pre-start gate: it now requires `node.secret` too (not just keystore + password) and locks it to 0600, so a partial key-gen (keystore written, identity not) can't be masked by the `creates:` sentinel on rerun. Fix the keystore-provisioning guidance so the documented steps work on a fresh host: the assert `fail_msg` and both READMEs now create the password file FIRST (`key-gen` reads it, never creates it) and pass `--output-dir` explicitly (a bare `key-gen` writes to ~/.decdn, which the node won't read), with an `ls -l` verify. Test: shared molecule stub learns a `key-gen` subcommand; a new `generate-keystore` scenario (no staged keystore, generate=true) proves the role mints keystore + password + node.secret at 0600 from nothing and is idempotent (changed=0). CI + Makefile run `molecule test --all`; default scenario stages node.secret to match the widened gate. Verified: ansible-lint (production profile) + yamllint + markdownlint clean; `molecule --all` green with both scenarios idempotent; fail_msg render checked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Ansible role now supports opt-in host-side keystore generation, preserves existing wallet material, requires and hardens node secrets, adds a generation-focused Molecule scenario, extends the stub CLI, updates operator documentation, and runs all Molecule scenarios in CI. ChangesKeystore generation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Molecule
participant decdn_node_role
participant decdn_node_stub
participant Verify
Molecule->>decdn_node_role: Converge with generation enabled
decdn_node_role->>decdn_node_stub: Invoke key-gen
decdn_node_stub->>decdn_node_role: Create keystore.json and node.secret
Molecule->>decdn_node_role: Run idempotence converge
Molecule->>Verify: Validate secrets, service, ownership, and permissions
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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 an opt-in turnkey wallet generation feature (decdn_node_generate_keystore) to the decdn_node Ansible role, allowing automatic generation of the keystore, password, and node identity (node.secret) when they are absent. It also adds a new Molecule test scenario (generate-keystore) to verify this path. The review feedback suggests two key improvements: using Ansible's built-in password lookup instead of an external openssl command for generating the password, and expanding the Molecule verification assertions to ensure the newly introduced node.secret file is also verified for correct ownership.
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.
Pull request overview
Adds an opt-in path for host-side eth wallet/identity generation in the decdn_node Ansible role, hardens the pre-start gate to also require node.secret, and updates docs + Molecule coverage to match the revised provisioning flow.
Changes:
- Add
decdn_node_generate_keystore(defaultfalse) to optionally mint keystore + password +node.secreton first converge (never overwriting existing material). - Harden the pre-start gate to require
node.secretand lock all wallet artifacts to0600. - Add a new Molecule scenario for the generation path and run Molecule with
--allin CI/Makefile.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ansible/roles/decdn_node/tasks/main.yml | Implements the opt-in generation flow and expands the pre-start gate to include node.secret. |
| ansible/roles/decdn_node/README.md | Updates operator provisioning steps and documents the new decdn_node_generate_keystore toggle. |
| ansible/roles/decdn_node/defaults/main.yml | Defines decdn_node_generate_keystore: false with detailed guidance comments. |
| ansible/README.md | Updates top-level deployment prerequisites to reference the corrected keystore provisioning flow / toggle. |
| ansible/molecule/generate-keystore/verify.yml | Verifies generation scenario created keystore/password/node.secret with correct modes and service is running. |
| ansible/molecule/generate-keystore/molecule.yml | Adds a Molecule scenario specifically for decdn_node_generate_keystore: true without a prepare step. |
| ansible/molecule/generate-keystore/converge.yml | Converges the role with decdn_node_generate_keystore: true against the stub binary. |
| ansible/molecule/default/verify.yml | Extends default scenario verification to include node.secret permission lockdown. |
| ansible/molecule/default/prepare.yml | Stages a placeholder node.secret so the widened gate can pass in the default scenario. |
| ansible/molecule/default/files/decdn-node-stub | Extends the stub to implement key-gen to support the new scenario. |
| ansible/Makefile | Runs Molecule with molecule test --all to cover both scenarios locally/CI. |
| ansible/galaxy/CHANGELOG.md | Notes the new decdn_node_generate_keystore option in the collection changelog. |
| .github/workflows/molecule.yml | Updates CI to execute molecule test --all (both scenarios). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ansible/molecule/generate-keystore/verify.yml (1)
47-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd ownership check for
node.secret.The ownership assertion checks
results[0](keystore.json) andresults[1](keystore.password) but omitsresults[2](node.secret). The role's lock-down task setsowner: "{{ decdn_user }}"for all three files in a loop, so node.secret ownership should be verified too.♻️ Proposed fix
- name: Assert the keystore + password are owned by the decdn user ansible.builtin.assert: that: - decdn_wallet.results[0].stat.pw_name == 'decdn' - decdn_wallet.results[1].stat.pw_name == 'decdn' + - decdn_wallet.results[2].stat.pw_name == 'decdn' - fail_msg: "generated wallet material is not owned by the decdn user" + fail_msg: "generated wallet material (keystore.json, keystore.password, or node.secret) is not owned by the decdn user"🤖 Prompt for 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. In `@ansible/molecule/generate-keystore/verify.yml` around lines 47 - 52, Extend the ownership assertion in the keystore verification task to also validate decdn_wallet.results[2].stat.pw_name equals 'decdn', covering node.secret alongside keystore.json and keystore.password.
🤖 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.
Nitpick comments:
In `@ansible/molecule/generate-keystore/verify.yml`:
- Around line 47-52: Extend the ownership assertion in the keystore verification
task to also validate decdn_wallet.results[2].stat.pw_name equals 'decdn',
covering node.secret alongside keystore.json and keystore.password.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 177dedce-c4c9-4eeb-9436-abc100aea45d
📒 Files selected for processing (13)
.github/workflows/molecule.ymlansible/Makefileansible/README.mdansible/galaxy/CHANGELOG.mdansible/molecule/default/files/decdn-node-stubansible/molecule/default/prepare.ymlansible/molecule/default/verify.ymlansible/molecule/generate-keystore/converge.ymlansible/molecule/generate-keystore/molecule.ymlansible/molecule/generate-keystore/verify.ymlansible/roles/decdn_node/README.mdansible/roles/decdn_node/defaults/main.ymlansible/roles/decdn_node/tasks/main.yml
…nership, docs
- Skip host-side wallet generation (openssl/copy/key-gen) under `--check` via
`not ansible_check_mode`, so `make check` is non-mutating and deterministic and
no longer produces a misleading empty-content password diff (Copilot).
- Assert node.secret ownership (decdn) in the generate-keystore verify, alongside
keystore + password (Gemini).
- README: the pre-start gate paragraph now names node.secret as a hard requirement
(Copilot).
Not adopted: openssl → lookup('ansible.builtin.password') (Gemini) — the lookup
runs on the control node, but AGENTS.md rule #1 requires secrets generated on the
target host; on-host `openssl rand` is the intended posture.
Verified: ansible-lint (production) + yamllint + markdownlint clean; molecule --all
green with both scenarios idempotent (changed=0).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Adds an opt-in, never-overwrite
decdn_node_generate_keystoretoggle to thedecdn_noderole and fixes the keystore-provisioning guidance so the documented manual steps actually work on a fresh host.Feature (
decdn_node_generate_keystore, defaultfalse)true, mints a fresh eth wallet on the host viadecdn key-gen— only if the keystore is absent. Never overwrites an existing (possibly funded) wallet.0600password file first, but only when the keystore is also absent. A password beside a missing keystore is treated as operator-provided and reused; a missing password beside a present keystore is not auto-minted (it couldn't decrypt the wallet) — the gate fails loud so the operator supplies the matching one.Gate hardening
node.secrettoo (not just keystore + password) and locks it to0600. This closes a partial-key-gen mask: if key-gen writeskeystore.jsonbut dies beforenode.secret, thecreates:sentinel would skip the rerun — but the gate now fails loud on the missing identity.Docs fix (works on a fresh host)
fail_msgand both READMEs now: create the password file FIRST (key-genreads it, never creates it), pass--output-direxplicitly (a baredecdn key-genwrites to~/.decdn, which the node won't read), and include anls -lverify step. Path/behavior facts verified againstdecdn/decdnsource.Tests
key-gensubcommand.generate-keystorescenario (no staged keystore,generate=true) proves the role mints keystore + password +node.secretat0600from nothing and is idempotent.molecule test --all(runs both scenarios); default scenario stagesnode.secretto match the widened gate. Note: this roughly doubles molecule CI wall-time (two containers).Verification
make lint-ansible(production profile) +make lint(yamllint / markdownlint / shellcheck) — clean.make molecule(--all) — both scenarios green, idempotencechanged=0.fail_msgrender checked via a failing assert (vars substituted, command block copy-pasteable).Follow-up (separate repo, not in this PR)
Cleanest general fix is a
decdn/decdnCLI change: havekey-gen --password-file <PATH>generate+write the password when the file is absent, collapsing provisioning to one self-contained command. Flagged only.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation