feat(baseline)!: unify sudo accounts into one primitive; single operator list - #30
Conversation
…tor list Collapse the baseline role's two near-duplicate key-only NOPASSWD sudo-account paths (the admin account in main.yml and named users in sudo_users.yml) into one primitive, and remove the ssh_admin_* config family in favour of a single operator list. - New tasks/sudo_account.yml renders one account (create -> sanitized /etc/sudoers.d drop-in -> password lock -> getent probe -> keys); the admin head and every named operator both route through it, so the sanitize/lock/check-mode logic lives in one place. - tasks/sudo_users.yml validates the whole set (well-formed, keyed, unique sudoers.d filenames, non-blank key members) and loops the primitive; still standalone-includable for molecule (tasks_from). - main.yml auto-detects the runner ($USER + ~/.ssh) as the lockout- critical head of baseline_sudo_users; a same-name explicit entry overrides it. The unconditional lockout guard now requires >=1 non-root operator with a usable (non-blank) key before hardening. BREAKING CHANGE: removes ssh_admin_user, ssh_admin_pubkey, ssh_admin_pubkey_autodetect, ssh_admin_extra_pubkeys and ssh_admin_passwordless_sudo. Configure admins via baseline_sudo_users plus baseline_sudo_autodetect_runner / baseline_sudo_passwordless. Verified: ansible-lint (production profile), molecule default (converge + idempotence + verify), and a runner-head/lockout logic matrix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 47 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 PR replaces legacy single-admin SSH variables with a unified ChangesSudo operator model
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Runner as Control-machine runner
participant Baseline as baseline tasks/main.yml
participant Operators as sudo_users.yml
participant Account as sudo_account.yml
participant Host as Target host
Runner->>Baseline: Provide runner identity and SSH key
Baseline->>Baseline: Merge runner head with baseline_sudo_users
Baseline->>Baseline: Validate keyed non-root operator exists
Baseline->>Operators: Provision baseline_operators
Operators->>Account: Include one task per operator
Account->>Host: Create account and configure sudo
Account->>Host: Install authorized keys
Possibly related issues
Possibly related PRs
🚥 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 refactors the baseline Ansible role to consolidate admin and named sudo users into a single operator list, utilizing a shared primitive task for account provisioning. However, a critical issue was identified in both tasks/main.yml and tasks/sudo_users.yml where Jinja2 filters attempt to access the 'keys' attribute of dictionaries. In Jinja2, this resolves to the dictionary's built-in '.keys()' method rather than the key's value, which can cause playbook crashes or bypass the lockout guard. The reviewer provides robust Jinja2 loop suggestions to safely extract and validate these keys.
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
This PR refactors the baseline Ansible role to unify previously split “admin user” and “named sudo users” provisioning into a single operator mechanism, with the runner identity auto-detected as the default lockout-critical head. It also migrates documentation/examples away from the ssh_admin_* variable family to the new baseline_sudo_* configuration.
Changes:
- Introduces a shared
sudo_account.ymltask primitive and routes all sudo-operator provisioning through it. - Replaces the
ssh_admin_*configuration family with a singlebaseline_sudo_userslist plusbaseline_sudo_autodetect_runner/baseline_sudo_passwordless. - Updates READMEs, inventory examples, and molecule coverage text to reflect the new operator model.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Quickstart text updated to reference baseline_sudo_users runner autodetection. |
| ansible/roles/baseline/tasks/sudo_users.yml | Validates the merged operator set and provisions operators via the shared primitive. |
| ansible/roles/baseline/tasks/sudo_account.yml | New reusable task primitive for creating key-only sudo operators (sudoers drop-in, password lock, key install, check-mode safety). |
| ansible/roles/baseline/tasks/main.yml | Resolves runner head + operator set and enforces an unconditional lockout guard before hardening. |
| ansible/roles/baseline/README.md | Role documentation migrated to the unified operator list and new knobs. |
| ansible/roles/baseline/defaults/main.yml | Defaults migrated from ssh_admin_* to baseline_sudo_*. |
| ansible/README.md | Deployment docs migrated to the unified operator model and lockout-guard description. |
| ansible/molecule/default/converge.yml | Updates molecule comments to match the new multi-key loop wording. |
| ansible/inventory/group_vars/all.yml | Inventory documentation migrated to baseline_sudo_* and single operator list. |
| ansible/galaxy/README.md | Collection usage example updated to baseline_sudo_users + disabling runner autodetect for explicit admin provisioning. |
💡 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.
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 `@ansible/roles/baseline/tasks/main.yml`:
- Around line 25-39: The autodetection candidates in the baseline_runner_pubkey
task use literal `~` paths that `first_found` cannot resolve. Update the
`candidates` values used by `found` to expand the controller’s home directory
before calling `query('ansible.builtin.first_found', ...)`, while preserving the
existing autodetection condition and key lookup behavior.
🪄 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: 67f5ab18-fcd9-437b-8102-46b46f1e3ebd
📒 Files selected for processing (10)
README.mdansible/README.mdansible/galaxy/README.mdansible/inventory/group_vars/all.ymlansible/molecule/default/converge.ymlansible/roles/baseline/README.mdansible/roles/baseline/defaults/main.ymlansible/roles/baseline/tasks/main.ymlansible/roles/baseline/tasks/sudo_account.ymlansible/roles/baseline/tasks/sudo_users.yml
…itly Address PR #30 review: - hosts.yml.example + .ansible-lint still referenced the removed ssh_admin_* variables (missed by an earlier *.yml-scoped grep). Point them at baseline_sudo_users / baseline_sudo_passwordless. - Expand ~ with the expanduser filter before first_found in the runner autodetect, so the lockout-critical key lookup does not depend on first_found's version-varying tilde handling (verified working on the pinned ansible-core; hardened for robustness). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Implements #27 (unify the two duplicate sudo-account paths into one primitive) and, per follow-up, collapses the
ssh_admin_*config family into a single operator list.What changed
roles/baseline/tasks/sudo_account.ymlrenders a single key-only NOPASSWD account (create → sanitized/etc/sudoers.ddrop-in → password lock → getent probe → key install). Both the admin head and every named operator route through it.ssh_admin_user,ssh_admin_pubkey,ssh_admin_pubkey_autodetect,ssh_admin_extra_pubkeys,ssh_admin_passwordless_sudo. Admins now come from a singlebaseline_sudo_userslist, with two knobs:baseline_sudo_autodetect_runner(defaulttrue) andbaseline_sudo_passwordless(defaulttrue, per-entry override).$USER+~/.sshkey) is auto-detected and prepended as the lockout-critical head, so the operator runningmake deployis provisioned without being committed. A same-name explicit entry overrides the auto-head.ssh_hardeningruns; the pre-flight rejects malformed entries (missing name, stringy/empty/blank keys, colliding sudoers.d filenames).Breaking change
Inventories using
ssh_admin_*must migrate tobaseline_sudo_users(+ the two knobs). All in-repo references (group_vars, both READMEs, galaxy example, root quickstart) are updated.Review
A multi-agent PR review caught a silent-lockout bug: a blank/whitespace key member (
keys: [""]) slipped all validation into a password-locked, keyless account. Fixed via a pre-flightblank_key_entriesrejection and by strengthening the lockout guard to require usable (non-blank) key material. Stale-doc and comment findings also addressed.Verification
make lint-ansible— clean (production profile).make moleculedefault scenario — converge + idempotence (changed=0) + verify green.Follow-up (not in this PR)
A committed regression test for the
main.ymlresolution/guard (molecule only coverssudo_users.yml) — recommend extracting the resolution into atasks_from-able file with a dedicated scenario.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation