diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f6df5d91..e1297c429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +* **plugin:bitwarden_item, module:bitwarden_item**: A run against a vault that contains no items at all aborts instead of creating the first one, because `bw serve` briefly reports an empty vault after every sync ([bitwarden/clients#23283](https://github.com/bitwarden/clients/issues/23283)). * **role:repo_postgresql**: The PostgreSQL version repositories take precedence over the distribution's packages of the same name, so on RHEL 10 an install or update no longer switches a PostgreSQL server from the PGDG build to the AppStream build, which uses a different file layout. * **role:apache_httpd**: Responses of type `text/markdown` are compressed like HTML, so the Markdown versions of pages that CMSs such as Grav hand to AI agents no longer go out uncompressed. * **role:grav**: The README lists setting `session.secure` as a manual step behind a reverse proxy that terminates TLS, where Grav sends its session cookies without the `Secure` flag. @@ -55,6 +56,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +* **module:bitwarden_item**: The module works with the Mitogen strategy, where it aborted with `MODULE FAILURE` on every run, for example when the `grafana` role stores its service account tokens. +* **plugin:bitwarden_item, module:bitwarden_item**: Running against several hosts in parallel no longer creates duplicates of a Bitwarden item, whether the item is new or has existed for a long time, so the next run no longer aborts with "Found multiple Bitwarden items". * **role:wordpress**: The installation no longer aborts at `wp core download` when Ansible connects as `root` without privilege escalation. * **role:wordpress**: A second run no longer reports the `wp-config.php` constants, `wordpress__plugins` and `wordpress__theme` as changed. * **role:wordpress**: The WXR file in `wordpress__wxr_export` is imported when the role installs WordPress; the import never ran before. diff --git a/plugins/lookup/bitwarden_item.py b/plugins/lookup/bitwarden_item.py index df1994d8b..d0c985c67 100644 --- a/plugins/lookup/bitwarden_item.py +++ b/plugins/lookup/bitwarden_item.py @@ -24,6 +24,8 @@ - When I(name) is omitted, a title is generated automatically as C(hostname - purpose) (e.g. C(dbserver - MariaDB)) or just C(hostname) when no purpose is given. - Generated passwords use Python's C(secrets) module (cryptographically strong RNG), not the Bitwarden generator. This lifts the 128-character limit and allows arbitrary character sets, including hex. - Items are read from a local on-disk cache backed by C(bw serve). A cached C(bw sync) is performed at most every 60 seconds, so consecutive lookups in the same play do not hammer the API. + - Lookups on the same controller run one at a time, so hosts that are processed in parallel and need the same missing item create it only once. + - Right after a sync, C(bw serve) can report an empty vault for a few seconds (U(https://github.com/bitwarden/clients/issues/23283)). The plugin then asks again for about ten seconds and fails rather than treat every item as missing. A vault that really is empty needs one item created by hand first. notes: - Lookups are evaluated by the templating engine on the controller and have no notion of check mode, so a run with C(--check) creates a missing item for real. Set I(create) to C(false) to turn that into a failure. @@ -315,6 +317,12 @@ def run(self, terms, variables=None, **kwargs): bw = Bitwarden() + # one lookup at a time on this controller, so parallel workers do not all + # create the same missing item + with bw.mutex(): + return self._run_under_mutex(bw, terms) + + def _run_under_mutex(self, bw, terms): status = bw.status if status != 'unlocked': raise AnsibleError(bw.get_not_unlocked_message(status)) diff --git a/plugins/module_utils/bitwarden.py b/plugins/module_utils/bitwarden.py index 5c461ab4a..e6ceda419 100644 --- a/plugins/module_utils/bitwarden.py +++ b/plugins/module_utils/bitwarden.py @@ -10,6 +10,7 @@ __metaclass__ = type +import contextlib import copy import email.encoders import email.mime.application @@ -17,6 +18,7 @@ import email.mime.nonmultipart import email.parser import email.policy +import fcntl import json import mimetypes import os @@ -34,9 +36,13 @@ from ansible.utils.display import Display display = Display() -except ImportError: +except Exception: # When used from a module (not a lookup plugin), this code runs inside an AnsiballZ - # process on the remote host where ansible.utils.display is not available. + # process on the remote host where ansible.utils.display is not available, so the + # import raises ImportError. Under Mitogen the import itself succeeds, because + # Mitogen serves it from the controller, but loading ansible.constants then raises + # AnsibleError ("Missing base YAML definition file"), since the data files are not + # served along. Verified with Mitogen 0.3.53 and ansible-core 2.16 on Rocky Linux 9. class _NoopDisplay: def vvv(self, msg, **kwargs): pass @@ -152,6 +158,16 @@ def prepare_multipart_no_base64(fields): CACHE_FILE = os.path.join(CACHE_DIR, 'lfops_bitwarden_cache.json') CACHE_VERSION = 2026032701 +# how long a process waits for another one to finish its sync, search and create +MUTEX_TIMEOUT = 300 + +# `bw serve` briefly reports an empty vault right after a sync, see +# https://github.com/bitwarden/clients/issues/23283 +# Verified against bw 2026.8.0 and 2026.9.0 on Rocky Linux 9: the list recovers +# within a few seconds, so ask again a few times before giving up. +EMPTY_LIST_RETRIES = 5 +EMPTY_LIST_RETRY_DELAY = 2 + class BitwardenException(Exception): pass @@ -163,8 +179,66 @@ class Bitwarden: def __init__(self, hostname='127.0.0.1', port=8087): self._base_url = f'http://{hostname}:{port}' self._cache = None + self._mutex_fd = None + self._load_cache() + + def _acquire_mutex(self, timeout=MUTEX_TIMEOUT): + """Take the mutex, waiting at most `timeout` seconds. Use mutex() instead.""" + if self._mutex_fd is not None: + return + mutex_file = f'{CACHE_FILE}.mutex' + try: + fd = os.open(mutex_file, os.O_RDWR | os.O_CREAT, 0o600) + except OSError as e: + raise BitwardenException( + f'Unable to open the mutex file {mutex_file}: {to_native(e)}' + ) from e + deadline = time.time() + timeout + while True: + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + break + except BlockingIOError: + if time.time() >= deadline: + os.close(fd) + raise BitwardenException( + f'Another process has held {mutex_file} for more than {timeout}s. ' + 'Check for a hanging Ansible run using the Bitwarden lookup or module' + ) from None + time.sleep(0.2) + self._mutex_fd = fd + display.vvv(f'lfbw - acquired mutex {mutex_file}') self._load_cache() + def _release_mutex(self): + if self._mutex_fd is None: + return + fcntl.flock(self._mutex_fd, fcntl.LOCK_UN) + os.close(self._mutex_fd) + self._mutex_fd = None + display.vvv('lfbw - released mutex') + + @contextlib.contextmanager + def mutex(self, timeout=MUTEX_TIMEOUT): + """Let only one process at a time sync, search and create. + + Ansible evaluates a lookup in one worker process per host. Without the mutex, + workers that all miss an item create it once each, and each one overwrites the + cache file with its own view of the vault. The cache is re-read once the mutex + is held, so a process sees what the previous holder synced or created. + + Hold it only for the duration of one lookup or module run. flock() belongs to + the open file, not to the process, so a second Bitwarden() in the same process, + for example the next evaluation of the same lookup, would otherwise wait for + the first one. This is also why the release must not be left to the end of the + process: Mitogen can run modules in an interpreter that stays alive. + """ + self._acquire_mutex(timeout) + try: + yield self + finally: + self._release_mutex() + def _api_call(self, url_path, method='GET', body=None, body_format='json'): url = f'{self._base_url}/{url_path}' @@ -306,12 +380,36 @@ def sync(self, force=False, interval=60): return display.vvv(f'lfbw - syncing vault (force={force})') self._api_call('sync', method='POST') - result = self._api_call('list/object/items') - self._cache['items'] = result['data']['data'] + self._cache['items'] = self._list_items() self._cache['sync_timestamp'] = time.time() display.vvv(f'lfbw - sync complete, cached {len(self._cache["items"])} items') self._save_cache() + def _list_items(self, retries=EMPTY_LIST_RETRIES, delay=EMPTY_LIST_RETRY_DELAY): + """Return all items of the vault. An empty list is not trusted. + + Right after a sync, `bw serve` answers `list/object/items` with `success: true` + and no items for a few seconds. Taken at face value, every item would look + missing and be created again, so ask again, and give up rather than accept an + empty vault. + """ + for attempt in range(retries + 1): + items = self._api_call('list/object/items')['data']['data'] + if items: + return items + if attempt < retries: + display.vvv( + f'lfbw - bw serve returned an empty item list, retrying in {delay}s' + ) + time.sleep(delay) + raise BitwardenException( + f'`bw serve` reported an empty vault {retries + 1} times in a row. It does ' + 'that for a few seconds after a sync ' + '(https://github.com/bitwarden/clients/issues/23283), so the list is not ' + 'trusted, since every item would look missing and be created again. If the ' + 'vault really is empty, create any item in it first' + ) + def get_items( self, name, @@ -384,7 +482,9 @@ def get_items( display.vvv(f'lfbw - found {len(matching_items)} matching item(s)') return matching_items - def get_item_by_id(self, item_id): + def get_item_by_id( + self, item_id, retries=EMPTY_LIST_RETRIES, delay=EMPTY_LIST_RETRY_DELAY + ): """Get an item by ID from Bitwarden. Looks in the cache first, then falls back to the API (the item may have been created externally). Returns the item; raises BitwardenException if the API does not know the ID. @@ -396,8 +496,16 @@ def get_item_by_id(self, item_id): return item # fallback to API if not found in cache (item could have been created externally) display.vvv('lfbw - item not in cache, falling back to API') - result = self._api_call(f'object/item/{item_id}') - return result['data'] + # in the same window after a sync in which `list/object/items` comes back empty, + # `bw serve` answers 400 for an existing item, see _list_items() + for attempt in range(retries + 1): + try: + return self._api_call(f'object/item/{item_id}')['data'] + except BitwardenException: + if attempt == retries: + raise + display.vvv(f'lfbw - item lookup failed, retrying in {delay}s') + time.sleep(delay) def generate( self, diff --git a/plugins/modules/bitwarden_item.py b/plugins/modules/bitwarden_item.py index 6345d9211..d9d40b0a6 100644 --- a/plugins/modules/bitwarden_item.py +++ b/plugins/modules/bitwarden_item.py @@ -23,6 +23,8 @@ - When I(name) is omitted, a title is generated automatically as C(hostname - purpose) (e.g. C(appsrv01 - MariaDB)) or just C(hostname) when no purpose is given. - On success, the module returns the full Bitwarden item object. C(username) and C(password) are additionally lifted to the top level so they can be addressed without going through the C(login) sub-dictionary. - Items are read from a local on-disk cache backed by C(bw serve). A cached C(bw sync) is performed at most every 60 seconds, so consecutive calls in the same play do not hammer the API. + - Module runs on the same host run one at a time, so hosts that are processed in parallel and need the same missing item create it only once. + - Right after a sync, C(bw serve) can report an empty vault for a few seconds (U(https://github.com/bitwarden/clients/issues/23283)). The module then asks again for about ten seconds and fails rather than treat every item as missing. A vault that really is empty needs one item created by hand first. notes: - Only login items (Bitwarden type 1) are managed. Cards, secure notes and identities are out of scope. @@ -344,83 +346,86 @@ def run_module(): username = module.params['username'] bw = Bitwarden() + # one module run at a time on this host, so parallel runs do not all create the + # same missing item + with bw.mutex(): + status = bw.status + if status != 'unlocked': + module.fail_json(msg=bw.get_not_unlocked_message(status)) - status = bw.status - if status != 'unlocked': - module.fail_json(msg=bw.get_not_unlocked_message(status)) + # to be sure we are up to date + bw.sync() - # to be sure we are up to date - bw.sync() - - changed = False - if item_id: - current_item = bw.get_item_by_id(item_id) - else: - name = Bitwarden.get_pretty_name(name, hostname, purpose) - current_items = bw.get_items( - name, username, folder_id, collection_id, organization_id - ) - - if len(current_items) > 1: - module.fail_json( - msg='Found multiple Bitwarden items with the same name/title and username, cannot decide which one to use. Aborting.' + changed = False + if item_id: + current_item = bw.get_item_by_id(item_id) + else: + name = Bitwarden.get_pretty_name(name, hostname, purpose) + current_items = bw.get_items( + name, username, folder_id, collection_id, organization_id ) - current_item = current_items[0] if current_items else None - - login_uris = bw.get_template_item_login_uri(uris) - login = bw.get_template_item_login(username, password, login_uris) - target_item = bw.get_template_item( - name, - login, - notes, - organization_id, - collection_id, - folder_id, - ) + if len(current_items) > 1: + module.fail_json( + msg='Found multiple Bitwarden items with the same name/title and username, cannot decide which one to use. Aborting.' + ) - # A None password means "do not manage the password": preserve the existing - # item's password instead of overwriting it with null. - if password is None and current_item and current_item.get('login'): - target_item['login']['password'] = current_item['login'].get('password') - - if current_item: - # check if changed, adjust if necessary - changed, updated_item = diff_and_update(current_item, target_item) - if changed and not module.check_mode: - result = bw.edit_item(updated_item, updated_item['id']) - elif changed: - result = updated_item - else: - result = current_item + current_item = current_items[0] if current_items else None + + login_uris = bw.get_template_item_login_uri(uris) + login = bw.get_template_item_login(username, password, login_uris) + target_item = bw.get_template_item( + name, + login, + notes, + organization_id, + collection_id, + folder_id, + ) - else: - # generate a new one - changed = True - result = target_item if module.check_mode else bw.create_item(target_item) + # A None password means "do not manage the password": preserve the existing + # item's password instead of overwriting it with null. + if password is None and current_item and current_item.get('login'): + target_item['login']['password'] = current_item['login'].get('password') + + if current_item: + # check if changed, adjust if necessary + changed, updated_item = diff_and_update(current_item, target_item) + if changed and not module.check_mode: + result = bw.edit_item(updated_item, updated_item['id']) + elif changed: + result = updated_item + else: + result = current_item - if attachments: - current_attachments = { - current_attachment['fileName'] - for current_attachment in result.get('attachments', []) - } - attachments_changed = False - for attachment in attachments: - if os.path.basename(attachment) not in current_attachments: - attachments_changed = True + else: + # generate a new one + changed = True + result = target_item if module.check_mode else bw.create_item(target_item) + + if attachments: + current_attachments = { + current_attachment['fileName'] + for current_attachment in result.get('attachments', []) + } + attachments_changed = False + for attachment in attachments: + if os.path.basename(attachment) not in current_attachments: + attachments_changed = True + if not module.check_mode: + bw.add_attachment(result['id'], attachment) + + if attachments_changed: + changed = True + # we need to fetch the item again, so that it also contains the newly added attachments if not module.check_mode: - bw.add_attachment(result['id'], attachment) + result = bw.get_item_by_id(result['id']) + + result['changed'] = changed + # move username and password higher for easier access + result['username'] = result['login']['username'] + result['password'] = result['login']['password'] - if attachments_changed: - changed = True - # we need to fetch the item again, so that it also contains the newly added attachments - if not module.check_mode: - result = bw.get_item_by_id(result['id']) - - result['changed'] = changed - # move username and password higher for easier access - result['username'] = result['login']['username'] - result['password'] = result['login']['password'] module.exit_json(**result) diff --git a/tests/unit/plugins/lookup/test_bitwarden_item.py b/tests/unit/plugins/lookup/test_bitwarden_item.py index 413c32772..d8b1ccb27 100644 --- a/tests/unit/plugins/lookup/test_bitwarden_item.py +++ b/tests/unit/plugins/lookup/test_bitwarden_item.py @@ -22,6 +22,7 @@ __metaclass__ = type +import contextlib import os import unittest from typing import ClassVar @@ -48,9 +49,19 @@ class _FakeBitwarden: created_items: ClassVar[list] = [] vault_status = 'unlocked' + mutex_held = False + def __init__(self, *args, **kwargs): pass + @contextlib.contextmanager + def mutex(self, *args, **kwargs): + type(self).mutex_held = True + try: + yield self + finally: + type(self).mutex_held = False + @property def status(self): return type(self).vault_status @@ -89,7 +100,9 @@ def get_template_item( return {'login': login, 'name': name, 'notes': notes} def create_item(self, item): - type(self).created_items.append(item) + type(self).created_items.append( + {**item, 'created_under_mutex': self.mutex_held} + ) return item @staticmethod @@ -104,6 +117,7 @@ def setUp(self): _FakeBitwarden.items_by_search = [] _FakeBitwarden.item_by_id = None _FakeBitwarden.created_items = [] + _FakeBitwarden.mutex_held = False _FakeBitwarden.vault_status = 'unlocked' # a value leaking in from the caller's environment would flip the # default of the `create` option under the tests' feet @@ -160,6 +174,11 @@ def test_missing_item_is_created_by_default(self): self.assertEqual(result[0]['username'], 'dba') self.assertEqual(result[0]['password'], 'linuxfabrik') + def test_item_is_created_under_the_mutex_and_mutex_is_released(self): + self.lookup.run([{'name': 'host - db', 'username': 'dba'}]) + self.assertTrue(_FakeBitwarden.created_items[0]['created_under_mutex']) + self.assertFalse(_FakeBitwarden.mutex_held) + def test_missing_item_raises_when_creation_disabled(self): os.environ[CREATE_ENV_VAR] = 'false' lookup = _load_lookup() diff --git a/tests/unit/plugins/module_utils/test_bitwarden.py b/tests/unit/plugins/module_utils/test_bitwarden.py index 6a4be9c82..ec18e68b0 100644 --- a/tests/unit/plugins/module_utils/test_bitwarden.py +++ b/tests/unit/plugins/module_utils/test_bitwarden.py @@ -19,11 +19,18 @@ __metaclass__ = type +import contextlib +import functools import importlib.util import io import json +import multiprocessing import os +import queue +import tempfile +import time import unittest +import unittest.mock from urllib.error import HTTPError _MODULE_PATH = os.path.join( @@ -59,6 +66,38 @@ def _make_bitwarden(tmp_path='/nonexistent/lfops_bw_test_cache.json'): return bitwarden.Bitwarden() +class TestDisplayFallback(unittest.TestCase): + """The module falls back to a no-op display where Ansible's cannot be loaded.""" + + def _load_with_failing_display_import(self, exception): + real_import = __import__ + + def _import(name, *args, **kwargs): + if name == 'ansible.utils.display': + raise exception + return real_import(name, *args, **kwargs) + + spec = importlib.util.spec_from_file_location( + 'bitwarden_fallback', _MODULE_PATH + ) + module = importlib.util.module_from_spec(spec) + with unittest.mock.patch('builtins.__import__', _import): + spec.loader.exec_module(module) + return module + + def test_import_error_falls_back(self): + # AnsiballZ on a managed node without the controller's ansible package + module = self._load_with_failing_display_import(ImportError('no display')) + self.assertEqual(type(module.display).__name__, '_NoopDisplay') + + def test_other_error_falls_back(self): + # Mitogen: the import is served, but ansible.constants cannot find base.yml + module = self._load_with_failing_display_import( + RuntimeError('Missing base YAML definition file (bad install?)') + ) + self.assertEqual(type(module.display).__name__, '_NoopDisplay') + + class TestGenerate(unittest.TestCase): def test_length_and_charset(self): bw = _make_bitwarden() @@ -222,5 +261,179 @@ def test_no_match_returns_empty(self): self.assertEqual(self.bw.get_items('does-not-exist', username='dba'), []) +_LOGIN_ITEM = { + 'id': 'abc', + 'type': 1, + 'name': 'host - db', + 'login': {'username': 'dba', 'password': 'linuxfabrik'}, + 'folderId': None, + 'collectionIds': [], + 'organizationId': None, +} + + +def _serve_in_order(*payloads): + """Fake open_url that answers each call with the next payload. + + A payload that is an exception instance is raised instead. + """ + remaining = list(payloads) + calls = [] + + def _open_url(url, *args, **kwargs): + calls.append(url) + payload = remaining.pop(0) + if isinstance(payload, Exception): + raise payload + return _FakeResponse(payload) + + return _open_url, calls + + +def _items(*items): + return {'success': True, 'data': {'object': 'list', 'data': list(items)}} + + +_SYNCED = {'success': True, 'data': {'object': 'message', 'title': 'Syncing complete.'}} + + +class TestEmptyListIsNotTrusted(unittest.TestCase): + """bw serve briefly reports an empty vault right after a sync (#23283).""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.bw = _make_bitwarden(os.path.join(self._tmp.name, 'cache.json')) + self._orig_open_url = bitwarden.open_url + + def tearDown(self): + bitwarden.open_url = self._orig_open_url + self._tmp.cleanup() + + def test_empty_list_is_asked_again(self): + bitwarden.open_url, calls = _serve_in_order( + _items(), _items(), _items(_LOGIN_ITEM) + ) + self.assertEqual(self.bw._list_items(delay=0), [_LOGIN_ITEM]) + self.assertEqual(len(calls), 3) + + def test_non_empty_list_is_not_asked_again(self): + bitwarden.open_url, calls = _serve_in_order(_items(_LOGIN_ITEM)) + self.assertEqual(self.bw._list_items(delay=0), [_LOGIN_ITEM]) + self.assertEqual(len(calls), 1) + + def test_persistently_empty_list_raises(self): + bitwarden.open_url, calls = _serve_in_order(*[_items()] * 3) + with self.assertRaises(bitwarden.BitwardenException) as ctx: + self.bw._list_items(retries=2, delay=0) + self.assertEqual(len(calls), 3) + self.assertIn('23283', str(ctx.exception)) + # AnsibleError appends ". ", see TestStatus + self.assertFalse(str(ctx.exception).endswith('.')) + + def test_sync_does_not_cache_an_empty_list(self): + bitwarden.open_url, _calls = _serve_in_order( + _SYNCED, _items(), _items(_LOGIN_ITEM) + ) + # no real delay between the attempts + self.bw._list_items = functools.partial( + bitwarden.Bitwarden._list_items, self.bw, delay=0 + ) + self.bw.sync(force=True) + self.assertEqual(self.bw.get_items('host - db', username='dba'), [_LOGIN_ITEM]) + + def test_item_by_id_is_asked_again(self): + self.bw._cache['items'] = [] + bitwarden.open_url, calls = _serve_in_order( + HTTPError('http://127.0.0.1:8087/object/item/abc', 400, 'Bad', {}, None), + {'success': True, 'data': _LOGIN_ITEM}, + ) + self.assertEqual(self.bw.get_item_by_id('abc', delay=0), _LOGIN_ITEM) + self.assertEqual(len(calls), 2) + + def test_unknown_item_by_id_still_raises(self): + self.bw._cache['items'] = [] + bitwarden.open_url, calls = _serve_in_order( + *[{'success': False, 'data': 'Not found.'}] * 3 + ) + with self.assertRaises(bitwarden.BitwardenException): + self.bw.get_item_by_id('abc', retries=2, delay=0) + self.assertEqual(len(calls), 3) + + +def _create_if_missing(cache_file, created): + """Worker for TestMutex: the lookup's search-then-create, run in its own process.""" + bitwarden.CACHE_FILE = cache_file + bw = bitwarden.Bitwarden() + with bw.mutex(): + if bw._cache['items'] is None: + bw._cache['items'] = [] + if not bw.get_items('host - db', username='dba'): + time.sleep(0.2) # the time a create takes, the window of the race + bw._cache['items'].append(_LOGIN_ITEM) + bw._save_cache() + created.put(os.getpid()) + + +class TestMutex(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.cache_file = os.path.join(self._tmp.name, 'cache.json') + + def tearDown(self): + self._tmp.cleanup() + + def test_parallel_processes_create_a_missing_item_once(self): + # fork, so the children inherit the module loaded by file path above + ctx = multiprocessing.get_context('fork') + created = ctx.Queue() + workers = [ + ctx.Process(target=_create_if_missing, args=(self.cache_file, created)) + for _ in range(8) + ] + for worker in workers: + worker.start() + for worker in workers: + worker.join(timeout=60) + self.assertEqual(worker.exitcode, 0) + creators = [] + with contextlib.suppress(queue.Empty): + while True: + creators.append(created.get(timeout=1)) + self.assertEqual(len(creators), 1) + + def test_mutex_times_out_while_another_holder_has_it(self): + holder = _make_bitwarden(self.cache_file) + waiter = bitwarden.Bitwarden() + + def _wait_for_the_mutex(): + with waiter.mutex(timeout=0.5): + pass + + with holder.mutex(): + self.assertRaises(bitwarden.BitwardenException, _wait_for_the_mutex) + # released, so the next one gets it + _wait_for_the_mutex() + + def test_mutex_is_released_when_the_body_raises(self): + # fail_json() in the module leaves the with block through SystemExit + first = _make_bitwarden(self.cache_file) + second = bitwarden.Bitwarden() + with self.assertRaises(SystemExit), first.mutex(): + raise SystemExit(1) + with second.mutex(timeout=0.5): + pass + + def test_mutex_reloads_the_cache(self): + first = _make_bitwarden(self.cache_file) + second = bitwarden.Bitwarden() + with first.mutex(): + first._cache['items'] = [_LOGIN_ITEM] + first._save_cache() + # `second` loaded the cache before `first` wrote it + self.assertIsNone(second._cache['items']) + with second.mutex(): + self.assertEqual(second._cache['items'], [_LOGIN_ITEM]) + + if __name__ == '__main__': unittest.main() diff --git a/tests/unit/plugins/modules/test_bitwarden_item.py b/tests/unit/plugins/modules/test_bitwarden_item.py index 62e444ac0..87af8dd98 100644 --- a/tests/unit/plugins/modules/test_bitwarden_item.py +++ b/tests/unit/plugins/modules/test_bitwarden_item.py @@ -19,6 +19,7 @@ __metaclass__ = type +import contextlib import copy import unittest import unittest.mock @@ -93,10 +94,19 @@ class _FakeBitwarden: edited: ClassVar[list] = [] created: ClassVar[list] = [] vault_status = 'unlocked' + mutex_held = False def __init__(self, *args, **kwargs): pass + @contextlib.contextmanager + def mutex(self, *args, **kwargs): + type(self).mutex_held = True + try: + yield self + finally: + type(self).mutex_held = False + @property def status(self): return type(self).vault_status @@ -153,7 +163,9 @@ def edit_item(self, item, item_id): return result def create_item(self, item): - type(self).created.append(copy.deepcopy(item)) + type(self).created.append( + {**copy.deepcopy(item), 'under_mutex': self.mutex_held} + ) result = copy.deepcopy(item) result['id'] = 'new-id' return result @@ -164,6 +176,7 @@ def setUp(self): _FakeBitwarden.items = [] _FakeBitwarden.edited = [] _FakeBitwarden.created = [] + _FakeBitwarden.mutex_held = False _FakeBitwarden.vault_status = 'unlocked' self._patchers = [ unittest.mock.patch.object(mod, 'Bitwarden', _FakeBitwarden), @@ -194,6 +207,14 @@ def test_vault_not_unlocked_fails(self): self.assertIn(status, ctx.exception.args[0]['msg']) self.assertEqual(_FakeBitwarden.created, []) + def test_item_is_created_under_the_mutex_and_mutex_is_released(self): + result = self._run( + {'name': 'host - db', 'username': 'dba', 'password': 'linuxfabrik'} + ) + self.assertTrue(result['changed']) + self.assertTrue(_FakeBitwarden.created[0]['under_mutex']) + self.assertFalse(_FakeBitwarden.mutex_held) + def test_check_mode_create_does_not_write(self): _FakeBitwarden.items = [] # nothing exists -> would create result = self._run(