fix(bitwarden_item): honor check mode and preserve password on None#271
Merged
Merged
Conversation
…ssword on None
Two behavior fixes:
- check_mode: the module declared supports_check_mode but wrote to the
vault regardless (edit/create/add_attachment). Guard every write behind
'not module.check_mode' and return the predicted item in check mode.
- None password: diff_and_update saw target password None vs an existing
real password as a change and overwrote it with null. A None password
now preserves the existing item's password, matching the documented
behavior ('overwritten by every non-None value').
Clarify the DOCUMENTATION accordingly and add main()-level tests (fake
Bitwarden client + the ansible module harness) for both paths.
The get_item_by_id returns-or-raises contract is left for a separate PR.
ebuerki-lf
pushed a commit
that referenced
this pull request
May 26, 2026
…ssword on None (#271) Two behavior fixes: - check_mode: the module declared supports_check_mode but wrote to the vault regardless (edit/create/add_attachment). Guard every write behind 'not module.check_mode' and return the predicted item in check mode. - None password: diff_and_update saw target password None vs an existing real password as a change and overwrote it with null. A None password now preserves the existing item's password, matching the documented behavior ('overwritten by every non-None value'). Clarify the DOCUMENTATION accordingly and add main()-level tests (fake Bitwarden client + the ansible module harness) for both paths. The get_item_by_id returns-or-raises contract is left for a separate PR.
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
1 participant
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.
Two deferred behavior-changing fixes for the bitwarden_item module.
check_mode
The module declared
supports_check_mode=Truebut callededit_item/create_item/add_attachmentregardless, so a--checkrun mutated the vault. Every write is now guarded bynot module.check_mode; in check mode the module returns the would-be item and the correctchanged.None password
diff_and_updatetreated a target password ofNone(the default, whenpasswordis not given) as differing from an existing real password and wrotenullover it, wiping the password. ANonepassword now preserves the existing item's password.This matches the documented behavior, which already stated that existing passwords "are overwritten by every non-
Nonevalue" (i.e.Nonemust not overwrite). The DOCUMENTATION is clarified to spell out the existing-item case.Tests
main()-level tests using a fake Bitwarden client and the shared ansible module harness:create_item/edit_itemnot called).passwordreportschanged=false, does not edit, and keeps the existing password.Deferred
The
get_item_by_idreturns-or-raises contract (the lookup wants a friendly "not found"; the module wants the id to exist) needs its own design and stays a separate PR.Validation
Full tox matrix green (100 tests across 13 envs);
ansible-docrenders; pre-commit green.