From d6cf48544a712da040a43b2cd0553bb82334d34f Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 12 Feb 2025 22:49:16 -0800 Subject: [PATCH] Get existing tests passing. --- borgmatic/hooks/credential/parse.py | 2 +- borgmatic/hooks/credential/systemd.py | 28 +++++++++++++-------- tests/unit/hooks/credential/test_parse.py | 22 ++++++++++++---- tests/unit/hooks/credential/test_systemd.py | 8 +++--- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/borgmatic/hooks/credential/parse.py b/borgmatic/hooks/credential/parse.py index 9125d235..699f90b7 100644 --- a/borgmatic/hooks/credential/parse.py +++ b/borgmatic/hooks/credential/parse.py @@ -42,5 +42,5 @@ def resolve_credential(value): raise ValueError(f'Cannot load credential with invalid syntax "{value}"') return borgmatic.hooks.dispatch.call_hook( - 'load_credential', {}, hook_name, credential_parameters + 'load_credential', {}, hook_name, tuple(credential_parameters) ) diff --git a/borgmatic/hooks/credential/systemd.py b/borgmatic/hooks/credential/systemd.py index 6e3630c7..8289c6dc 100644 --- a/borgmatic/hooks/credential/systemd.py +++ b/borgmatic/hooks/credential/systemd.py @@ -5,29 +5,35 @@ import re logger = logging.getLogger(__name__) -SECRET_NAME_PATTERN = re.compile(r'^\w+$') -SECRETS_DIRECTORY = '/run/secrets' +CREDENTIAL_NAME_PATTERN = re.compile(r'^\w+$') def load_credential(hook_config, config, credential_parameters): ''' Given the hook configuration dict, the configuration dict, and a credential parameters tuple - containing a secret name to load, read the secret from the corresponding container secrets file - and return it. + containing a credential name to load, read the credential from the corresponding systemd + credential file and return it. - Raise ValueError if the credential parameters is not one element, the secret name is invalid, or - the secret file cannot be read. + Raise ValueError if the systemd CREDENTIALS_DIRECTORY environment variable is not set, the + credential name is invalid, or the credential file cannot be read. ''' try: - (secert_name,) = credential_parameters + (credential_name,) = credential_parameters except ValueError: - raise ValueError(f'Cannot load invalid secret name: "{' '.join(credential_parameters)}"') + raise ValueError(f'Cannot load invalid credential name: "{' '.join(credential_parameters)}"') - if not SECRET_NAME_PATTERN.match(SECRET_NAME): - raise ValueError(f'Cannot load invalid secret name: "{credential_name}"') + credentials_directory = os.environ.get('CREDENTIALS_DIRECTORY') + + if not credentials_directory: + raise ValueError( + f'Cannot load credential "{credential_name}" because the systemd CREDENTIALS_DIRECTORY environment variable is not set' + ) + + if not CREDENTIAL_NAME_PATTERN.match(credential_name): + raise ValueError(f'Cannot load invalid credential name "{credential_name}"') try: - with open(os.path.join(SECRETS_DIRECTORY, credential_name)) as credential_file: + with open(os.path.join(credentials_directory, credential_name)) as credential_file: return credential_file.read().rstrip(os.linesep) except (FileNotFoundError, OSError) as error: logger.warning(error) diff --git a/tests/unit/hooks/credential/test_parse.py b/tests/unit/hooks/credential/test_parse.py index 9c2ca8e3..5fd4301a 100644 --- a/tests/unit/hooks/credential/test_parse.py +++ b/tests/unit/hooks/credential/test_parse.py @@ -4,7 +4,7 @@ from flexmock import flexmock from borgmatic.hooks.credential import parse as module -def test_resolve_credential_passes_through_string_without_credential_tag(): +def test_resolve_credential_passes_through_string_without_credential(): module.resolve_credential.cache_clear() flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').never() @@ -19,7 +19,7 @@ def test_resolve_credential_passes_through_none(): @pytest.mark.parametrize('invalid_value', ('{credential}', '{credential }', '{credential systemd}')) -def test_resolve_credential_with_invalid_credential_tag_raises(invalid_value): +def test_resolve_credential_with_invalid_credential_raises(invalid_value): module.resolve_credential.cache_clear() flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').never() @@ -27,25 +27,37 @@ def test_resolve_credential_with_invalid_credential_tag_raises(invalid_value): module.resolve_credential(invalid_value) -def test_resolve_credential_with_valid_credential_tag_loads_credential(): +def test_resolve_credential_with_valid_credential_loads_credential(): module.resolve_credential.cache_clear() flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').with_args( 'load_credential', {}, 'systemd', - 'mycredential', + ('mycredential',), ).and_return('result').once() assert module.resolve_credential('{credential systemd mycredential}') == 'result' +def test_resolve_credential_with_valid_credential_and_quoted_parameters_loads_credential(): + module.resolve_credential.cache_clear() + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').with_args( + 'load_credential', + {}, + 'systemd', + ('my credential',), + ).and_return('result').once() + + assert module.resolve_credential('{credential systemd "my credential"}') == 'result' + + def test_resolve_credential_caches_credential_after_first_call(): module.resolve_credential.cache_clear() flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').with_args( 'load_credential', {}, 'systemd', - 'mycredential', + ('mycredential',), ).and_return('result').once() assert module.resolve_credential('{credential systemd mycredential}') == 'result' diff --git a/tests/unit/hooks/credential/test_systemd.py b/tests/unit/hooks/credential/test_systemd.py index 97b2877f..d2f70d40 100644 --- a/tests/unit/hooks/credential/test_systemd.py +++ b/tests/unit/hooks/credential/test_systemd.py @@ -13,7 +13,7 @@ def test_load_credential_without_credentials_directory_raises(): ) with pytest.raises(ValueError): - module.load_credential(hook_config={}, config={}, credential_name='mycredential') + module.load_credential(hook_config={}, config={}, credential_parameters=('mycredential',)) def test_load_credential_with_invalid_credential_name_raises(): @@ -22,7 +22,7 @@ def test_load_credential_with_invalid_credential_name_raises(): ) with pytest.raises(ValueError): - module.load_credential(hook_config={}, config={}, credential_name='../../my!@#$credential') + module.load_credential(hook_config={}, config={}, credential_parameters=('../../my!@#$credential',)) def test_load_credential_reads_named_credential_from_file(): @@ -35,7 +35,7 @@ def test_load_credential_reads_named_credential_from_file(): builtins.should_receive('open').with_args('/var/mycredential').and_return(credential_stream) assert ( - module.load_credential(hook_config={}, config={}, credential_name='mycredential') + module.load_credential(hook_config={}, config={}, credential_parameters=('mycredential',)) == 'password' ) @@ -48,4 +48,4 @@ def test_load_credential_with_file_not_found_error_raises(): builtins.should_receive('open').with_args('/var/mycredential').and_raise(FileNotFoundError) with pytest.raises(ValueError): - module.load_credential(hook_config={}, config={}, credential_name='mycredential') + module.load_credential(hook_config={}, config={}, credential_parameters=('mycredential',))