diff --git a/changelogs/fragments/404-vault_write-spicy-keys.yml b/changelogs/fragments/404-vault_write-spicy-keys.yml new file mode 100644 index 000000000..1a4c5d6c6 --- /dev/null +++ b/changelogs/fragments/404-vault_write-spicy-keys.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - vault_write - the ``vault_write`` lookup and module were not able to write data containing keys named ``path`` or ``wrap_ttl`` due to a bug in the ``hvac`` library. These plugins have now been updated to take advantage of fixes in ``hvac>=1.2`` to address this (https://github.com/ansible-collections/community.hashi_vault/issues/389). diff --git a/plugins/lookup/vault_write.py b/plugins/lookup/vault_write.py index 6864c76fb..a8c056a73 100644 --- a/plugins/lookup/vault_write.py +++ b/plugins/lookup/vault_write.py @@ -43,7 +43,9 @@ type: str required: true data: - description: A dictionary to be serialized to JSON and then sent as the request body. + description: + - A dictionary to be serialized to JSON and then sent as the request body. + - If the dictionary contains keys named C(path) or C(wrap_ttl), the call will fail with C(hvac<1.2). type: dict required: false default: {} @@ -122,8 +124,8 @@ from ansible.module_utils.six import raise_from -from ansible_collections.community.hashi_vault.plugins.plugin_utils._hashi_vault_lookup_base import HashiVaultLookupBase -from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_common import HashiVaultValueError +from ..plugin_utils._hashi_vault_lookup_base import HashiVaultLookupBase +from ..module_utils._hashi_vault_common import HashiVaultValueError display = Display() @@ -164,7 +166,16 @@ def run(self, terms, variables=None, **kwargs): for term in terms: try: - response = client.write(path=term, wrap_ttl=wrap_ttl, **data) + try: + # TODO: write_data will eventually turn back into write + # see: https://github.com/hvac/hvac/issues/1034 + response = client.write_data(path=term, wrap_ttl=wrap_ttl, data=data) + except AttributeError as e: + # https://github.com/ansible-collections/community.hashi_vault/issues/389 + if "path" in data or "wrap_ttl" in data: + raise_from(AnsibleError("To use 'path' or 'wrap_ttl' as data keys, use hvac >= 1.2"), e) + else: + response = client.write(path=term, wrap_ttl=wrap_ttl, **data) except hvac.exceptions.Forbidden as e: raise_from(AnsibleError("Forbidden: Permission Denied to path '%s'." % term), e) except hvac.exceptions.InvalidPath as e: diff --git a/plugins/modules/vault_write.py b/plugins/modules/vault_write.py index 35c7fcb60..76cf4f8e9 100644 --- a/plugins/modules/vault_write.py +++ b/plugins/modules/vault_write.py @@ -46,7 +46,9 @@ type: str required: True data: - description: A dictionary to be serialized to JSON and then sent as the request body. + description: + - A dictionary to be serialized to JSON and then sent as the request body. + - If the dictionary contains keys named C(path) or C(wrap_ttl), the call will fail with C(hvac<1.2). type: dict required: false default: {} @@ -108,8 +110,8 @@ from ansible.module_utils._text import to_native from ansible.module_utils.basic import missing_required_lib -from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_module import HashiVaultModule -from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_common import HashiVaultValueError +from ..module_utils._hashi_vault_module import HashiVaultModule +from ..module_utils._hashi_vault_common import HashiVaultValueError try: import hvac @@ -157,7 +159,16 @@ def run_module(): if module.check_mode: response = {} else: - response = client.write(path=path, wrap_ttl=wrap_ttl, **data) + try: + # TODO: write_data will eventually turn back into write + # see: https://github.com/hvac/hvac/issues/1034 + response = client.write_data(path=path, wrap_ttl=wrap_ttl, data=data) + except AttributeError: + # https://github.com/ansible-collections/community.hashi_vault/issues/389 + if "path" in data or "wrap_ttl" in data: + module.fail_json("To use 'path' or 'wrap_ttl' as data keys, use hvac >= 1.2") + else: + response = client.write(path=path, wrap_ttl=wrap_ttl, **data) except hvac.exceptions.Forbidden: module.fail_json(msg="Forbidden: Permission Denied to path '%s'." % path, exception=traceback.format_exc()) except hvac.exceptions.InvalidPath: diff --git a/tests/integration/targets/lookup_vault_write/tasks/lookup_vault_write_test.yml b/tests/integration/targets/lookup_vault_write/tasks/lookup_vault_write_test.yml index 33dc245f9..e43c04708 100644 --- a/tests/integration/targets/lookup_vault_write/tasks/lookup_vault_write_test.yml +++ b/tests/integration/targets/lookup_vault_write/tasks/lookup_vault_write_test.yml @@ -11,6 +11,9 @@ test_data: a: 1 b: two + # https://github.com/ansible-collections/community.hashi_vault/issues/389 + path: path_value + wrap_ttl: wrap_ttl_value block: - name: Write data to the cubbyhole vars: diff --git a/tests/integration/targets/module_vault_write/tasks/module_vault_write_test.yml b/tests/integration/targets/module_vault_write/tasks/module_vault_write_test.yml index 244b8e29a..b9824b91f 100644 --- a/tests/integration/targets/module_vault_write/tasks/module_vault_write_test.yml +++ b/tests/integration/targets/module_vault_write/tasks/module_vault_write_test.yml @@ -20,6 +20,9 @@ data: a: 1 b: two + # https://github.com/ansible-collections/community.hashi_vault/issues/389 + path: path_value + wrap_ttl: wrap_ttl_value - assert: that: @@ -42,6 +45,9 @@ data: a: 1 b: two + # https://github.com/ansible-collections/community.hashi_vault/issues/389 + path: path_value + wrap_ttl: wrap_ttl_value - assert: that: @@ -57,7 +63,13 @@ that: - "'result' in result" - "'data' in result.result" - - "result.result.data == {'a': 1, 'b': 'two'}" + - > + result.result.data == { + 'a': 1, + 'b': 'two', + 'path': 'path_value', + 'wrap_ttl': 'wrap_ttl_value', + } - name: Write data to an endpoint that returns data and test wrapping register: result diff --git a/tests/unit/plugins/lookup/test_vault_write.py b/tests/unit/plugins/lookup/test_vault_write.py index c3c325228..eaac0ff08 100644 --- a/tests/unit/plugins/lookup/test_vault_write.py +++ b/tests/unit/plugins/lookup/test_vault_write.py @@ -68,18 +68,18 @@ def test_vault_write_auth_validation_error(self, vault_write_lookup, minimal_var def test_vault_write_return_data(self, vault_write_lookup, minimal_vars, approle_secret_id_write_response, vault_client, paths, data, wrap_ttl): client = vault_client - expected_calls = [mock.call(path=p, wrap_ttl=wrap_ttl, **data) for p in paths] + expected_calls = [mock.call(path=p, wrap_ttl=wrap_ttl, data=data) for p in paths] - def _fake_write(path, wrap_ttl, **data): + def _fake_write(path, wrap_ttl, data=None): r = approle_secret_id_write_response.copy() r.update({'path': path}) return r - client.write = mock.Mock(wraps=_fake_write) + client.write_data = mock.Mock(wraps=_fake_write) response = vault_write_lookup.run(terms=paths, variables=minimal_vars, wrap_ttl=wrap_ttl, data=data) - client.write.assert_has_calls(expected_calls) + client.write_data.assert_has_calls(expected_calls) assert len(response) == len(paths), "%i paths processed but got %i responses" % (len(paths), len(response)) @@ -96,7 +96,7 @@ def test_vault_write_empty_response(self, vault_write_lookup, minimal_vars, vaul requests_unparseable_response.status_code = 204 - client.write.return_value = requests_unparseable_response + client.write_data.return_value = requests_unparseable_response response = vault_write_lookup.run(terms=['fake'], variables=minimal_vars) @@ -108,7 +108,7 @@ def test_vault_write_unparseable_response(self, vault_write_lookup, minimal_vars requests_unparseable_response.status_code = 200 requests_unparseable_response.content = '﷽' - client.write.return_value = requests_unparseable_response + client.write_data.return_value = requests_unparseable_response with mock.patch('ansible_collections.community.hashi_vault.plugins.lookup.vault_write.display.warning') as warning: response = vault_write_lookup.run(terms=['fake'], variables=minimal_vars) @@ -127,7 +127,40 @@ def test_vault_write_unparseable_response(self, vault_write_lookup, minimal_vars def test_vault_write_exceptions(self, vault_write_lookup, minimal_vars, vault_client, exc): client = vault_client - client.write.side_effect = exc[0] + client.write_data.side_effect = exc[0] with pytest.raises(AnsibleError, match=exc[1]): vault_write_lookup.run(terms=['fake'], variables=minimal_vars) + + @pytest.mark.parametrize( + 'data', + [ + {"path": mock.sentinel.path_value}, + {"wrap_ttl": mock.sentinel.wrap_ttl_value}, + {"path": mock.sentinel.data_value, "wrap_ttl": mock.sentinel.write_ttl_value}, + ], + ) + def test_vault_write_data_fallback_bad_params(self, vault_write_lookup, minimal_vars, vault_client, data): + client = vault_client + client.mock_add_spec(['write']) + + with pytest.raises(AnsibleError, match=r"To use 'path' or 'wrap_ttl' as data keys, use hvac >= 1\.2"): + vault_write_lookup.run(terms=['fake'], variables=minimal_vars, data=data) + + client.write.assert_not_called() + + @pytest.mark.parametrize( + 'data', + [ + {"item1": mock.sentinel.item1_value}, + {"item2": mock.sentinel.item2_value}, + {"item1": mock.sentinel.item1_value, "item2": mock.sentinel.item2_value}, + ], + ) + def test_vault_write_data_fallback_write(self, vault_write_lookup, minimal_vars, vault_client, data): + client = vault_client + client.mock_add_spec(['write']) + + vault_write_lookup.run(terms=['fake'], variables=minimal_vars, data=data) + + client.write.assert_called_once_with(path='fake', wrap_ttl=None, **data) diff --git a/tests/unit/plugins/modules/test_vault_write.py b/tests/unit/plugins/modules/test_vault_write.py index afe877e8d..59d2e38a1 100644 --- a/tests/unit/plugins/modules/test_vault_write.py +++ b/tests/unit/plugins/modules/test_vault_write.py @@ -88,7 +88,7 @@ def test_vault_write_auth_validation_error(self, authenticator, exc, capfd): @pytest.mark.parametrize('patch_ansible_module', [[_combined_options(), 'data', 'wrap_ttl']], indirect=True) def test_vault_write_return_data(self, patch_ansible_module, approle_secret_id_write_response, vault_client, opt_wrap_ttl, opt_data, capfd): client = vault_client - client.write.return_value = approle_secret_id_write_response + client.write_data.return_value = approle_secret_id_write_response with pytest.raises(SystemExit) as e: vault_write.main() @@ -98,7 +98,7 @@ def test_vault_write_return_data(self, patch_ansible_module, approle_secret_id_w assert e.value.code == 0, "result: %r" % (result,) - client.write.assert_called_once_with(path=patch_ansible_module['path'], wrap_ttl=opt_wrap_ttl, **opt_data) + client.write_data.assert_called_once_with(path=patch_ansible_module['path'], wrap_ttl=opt_wrap_ttl, data=opt_data) assert result['data'] == approle_secret_id_write_response, ( "module result did not match expected result:\nmodule: %r\nexpected: %r" % (result['data'], approle_secret_id_write_response) @@ -110,7 +110,7 @@ def test_vault_write_empty_response(self, vault_client, requests_unparseable_res requests_unparseable_response.status_code = 204 - client.write.return_value = requests_unparseable_response + client.write_data.return_value = requests_unparseable_response with pytest.raises(SystemExit) as e: vault_write.main() @@ -129,7 +129,7 @@ def test_vault_write_unparseable_response(self, vault_client, requests_unparseab requests_unparseable_response.status_code = 200 requests_unparseable_response.content = '﷽' - client.write.return_value = requests_unparseable_response + client.write_data.return_value = requests_unparseable_response with pytest.raises(SystemExit) as e: vault_write.main() @@ -166,7 +166,7 @@ def test_vault_write_no_hvac(self, capfd): def test_vault_write_vault_exception(self, vault_client, exc, capfd): client = vault_client - client.write.side_effect = exc[0] + client.write_data.side_effect = exc[0] with pytest.raises(SystemExit) as e: vault_write.main() @@ -176,3 +176,51 @@ def test_vault_write_vault_exception(self, vault_client, exc, capfd): assert e.value.code != 0, "result: %r" % (result,) assert re.search(exc[1], result['msg']) is not None + + @pytest.mark.parametrize( + 'opt_data', + [ + {"path": 'path_value'}, + {"wrap_ttl": 'wrap_ttl_value'}, + {"path": 'data_value', "wrap_ttl": 'write_ttl_value'}, + ], + ) + @pytest.mark.parametrize('patch_ansible_module', [[_combined_options(), 'data']], indirect=True) + def test_vault_write_data_fallback_bad_params(self, vault_client, opt_data, capfd): + client = vault_client + client.mock_add_spec(['write']) + + with pytest.raises(SystemExit) as e: + vault_write.main() + + out, err = capfd.readouterr() + result = json.loads(out) + + assert e.value.code != 0, "result: %r" % (result,) + assert re.search(r"To use 'path' or 'wrap_ttl' as data keys, use hvac >= 1\.2", result['msg']) is not None + + client.write.assert_not_called() + + @pytest.mark.parametrize( + 'opt_data', + [ + {"item1": 'item1_value'}, + {"item2": 'item2_value'}, + {"item1": 'item1_value', "item2": 'item2_value'}, + ], + ) + @pytest.mark.parametrize('patch_ansible_module', [[_combined_options(), 'data']], indirect=True) + def test_vault_write_data_fallback_write(self, vault_client, opt_data, patch_ansible_module, approle_secret_id_write_response, capfd): + client = vault_client + client.mock_add_spec(['write']) + client.write.return_value = approle_secret_id_write_response + + with pytest.raises(SystemExit) as e: + vault_write.main() + + out, err = capfd.readouterr() + result = json.loads(out) + + assert e.value.code == 0, "result: %r" % (result,) + + client.write.assert_called_once_with(path=patch_ansible_module['path'], wrap_ttl=None, **opt_data)