fix(baseline): make the check-mode key-install conditional boolean - #37
Merged
Merged
Conversation
`make check` fails on any host where a sudo operator account already exists: [ERROR]: Task failed: A 'when' expression failed: Conditional result (True) was derived from value of type 'list' ... Conditionals must have a boolean result. `getent` with fail_key:false stores a hit as the passwd field LIST (["x","1000","1000","Ant Somers","/home/user","/bin/bash"]), not a boolean, and ansible-core 2.19+ rejects a non-boolean conditional outright rather than coercing it. It only bites under --check. A real run short-circuits on the left operand of the `or` and yields a clean True, so the right operand is never evaluated; in check mode `not ansible_check_mode` is False, the list is returned as the result, and the task dies. That asymmetry is also why no molecule scenario catches it — none of them run in check mode. Compare the getent hit to None explicitly, which is what the comment on the line already said the test was for. Verified against a real provisioned host (Ubuntu 26.04, two existing operator accounts): `make check` went from failed=1 to ok=206 changed=7 failed=0. `make lint` and `make lint-ansible` (production profile) both pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019WB9m9pHjptvDCThR4z6Lj
There was a problem hiding this comment.
Pull request overview
Fixes an Ansible check-mode failure in the baseline role’s sudo operator provisioning by ensuring the authorized_key task’s when clause always evaluates to a boolean under ansible-core 2.19+.
Changes:
- Makes the check-mode “skip key install if user is missing” conditional explicitly boolean by comparing the
getentlookup result againstnone. - Expands inline comments to document why the check-mode path differs and why ansible-core 2.19+ rejects the prior expression.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
make checkfails on any host where a sudo operator account already exists:getentwithfail_key: falsestores a hit as the passwd field list —["x","1000","1000","Ant Somers","/home/user","/bin/bash"]— not a boolean, andansible-core 2.19+ rejects a non-boolean conditional outright rather than coercing it.
Why it only shows up under
--checkA real run short-circuits on the left operand and yields a clean
True, so the rightoperand is never evaluated. In check mode
not ansible_check_modeisFalse, the listis returned as the conditional's result, and the task dies.
So
make deployis unaffected — but the documented pre-deploy dry run is broken. Thatasymmetry is also why CI never caught it: no molecule scenario runs in check mode
(
grep -rn 'check_mode\|--check' ansible/molecule/is empty), even though this taskexists specifically to handle check mode.
Introduced by 589b9b1 (#30).
Fix
Compare the getent hit to
Noneexplicitly — which is what the comment on the linealready said the test was for.
Verification
Against a real provisioned host (Ubuntu 26.04, two existing operator accounts):
make checkok=17 failed=1ok=206 changed=7 failed=0make lintandmake lint-ansible(production profile, 0 failures/warnings) both pass.Follow-up, not in this PR
ansible/galaxy/CHANGELOG.mddocuments everydecdn_nodebreaking rename from therecent syncs but has no entry for the
ssh_admin_*→baseline_sudo_usersremovalin 589b9b1. Those variables are now silently ignored — no shim, no assert — so an
inventory written against the old names still "works" while quietly provisioning the
auto-detected runner instead of the pinned account. Worth a changelog entry.
🤖 Generated with Claude Code
https://claude.ai/code/session_019WB9m9pHjptvDCThR4z6Lj