Merge a6214ce into 85ead94

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).

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:

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:

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:

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

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)

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)