fix idempotency issue when creating existing key pair with key materi…

…al (ansible-collections#1383)

ec2_key - fix idempotency issue when updating existing key pair

SUMMARY

when running module with state=present on existing key pair with the same key material the module raises the following issue
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: TypeError: ansible_collections.amazon.aws.plugins.module_utils.modules.AnsibleAWSModule.exit_json() argument after ** must be a mapping, not NoneType
fatal: [localhost]: FAILED! => {"changed": false, "module_stderr": "Traceback (most recent call last):\n  File \"/home/aubin/.ansible/tmp/ansible-tmp-1676994806.9488113-2761194-77181612241045/AnsiballZ_ec2_key.py\", line 100, in <module>\n    _ansiballz_main()\n  File \"/home/aubin/.ansible/tmp/ansible-tmp-1676994806.9488113-2761194-77181612241045/AnsiballZ_ec2_key.py\", line 92, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/aubin/.ansible/tmp/ansible-tmp-1676994806.9488113-2761194-77181612241045/AnsiballZ_ec2_key.py\", line 40, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.amazon.aws.plugins.modules.ec2_key', init_globals=dict(_module_fqn='ansible_collections.amazon.aws.plugins.modules.ec2_key', _modlib_path=modlib_path),\n  File \"/usr/lib64/python3.9/runpy.py\", line 225, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib64/python3.9/runpy.py\", line 97, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib64/python3.9/runpy.py\", line 87, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_amazon.aws.ec2_key_payload_tghhfma0/ansible_amazon.aws.ec2_key_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_key.py\", line 389, in <module>\n  File \"/tmp/ansible_amazon.aws.ec2_key_payload_tghhfma0/ansible_amazon.aws.ec2_key_payload.zip/ansible_collections/amazon/aws/plugins/modules/ec2_key.py\", line 385, in main\nTypeError: ansible_collections.amazon.aws.plugins.module_utils.modules.AnsibleAWSModule.exit_json() argument after ** must be a mapping, not NoneType\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

This pull request aims to fix this issue

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

ec2_key

Reviewed-by: Mark Chappell
Reviewed-by: Bikouo Aubin

changelogs/fragments/1383-ec2_key-fix-idempotency-issue-when-creating-existing-key-with-same-key-material.yaml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - ec2_key - fix issue when trying to update existing key pair with the same key material (https://github.com/ansible-collections/amazon.aws/pull/1383).

plugins/modules/ec2_key.py

@@ -270,11 +270,15 @@ def update_key_pair_by_key_material(check_mode, ec2_client, name, key, key_mater
     if check_mode:
         return {'changed': True, 'key': None, 'msg': 'key pair updated'}
     new_fingerprint = get_key_fingerprint(check_mode, ec2_client, key_material)
+    changed = False
+    msg = "key pair already exists"
     if key['KeyFingerprint'] != new_fingerprint:
         delete_key_pair(check_mode, ec2_client, name, finish_task=False)
         key = _import_key_pair(ec2_client, name, key_material, tag_spec)
-        key_data = extract_key_data(key)
-        return {'changed': True, 'key': key_data, 'msg': "key pair updated"}
+        msg = "key pair updated"
+        changed = True
+    key_data = extract_key_data(key)
+    return {"changed": changed, "key": key_data, "msg": msg}
 
 
 def update_key_pair_by_key_type(check_mode, ec2_client, name, key_type, tag_spec):
@@ -327,7 +331,7 @@ def handle_existing_key_pair_update(module, ec2_client, name, key):
         changed |= ensure_ec2_tags(ec2_client, module, key['KeyPairId'], tags=tags, purge_tags=purge_tags)
         key = find_key_pair(ec2_client, name)
         key_data = extract_key_data(key)
-        result = {'changed': changed, 'key': key_data, 'msg': 'key pair alreday exists'}
+        result = {"changed": changed, "key": key_data, "msg": "key pair already exists"}
     return result
 
 

tests/integration/targets/ec2_key/tasks/main.yml

@@ -1,6 +1,5 @@
 ---
 # TODO - name: test 'validate_certs' parameter
-# TODO - name: test creating key pair with another_key_material with force=yes
 # =============================================================
 
 - module_defaults:
@@ -345,6 +344,28 @@
           - 'result.key.name == "{{ec2_key_name}}"'
           - 'result.key.fingerprint == "{{fingerprint}}"'
 
+    # ============================================================
+    - name: test state=present with key_material (idempotency)
+      ec2_key:
+        name: '{{ ec2_key_name }}'
+        key_material: '{{ key_material }}'
+        state: present
+      register: result
+
+    - name: assert state=present with key_material
+      assert:
+        that:
+          - result is not changed
+          - '"key" in result'
+          - '"name" in result.key'
+          - '"fingerprint" in result.key'
+          - '"private_key" not in result.key'
+          - '"id" in result.key'
+          - '"tags" in result.key'
+          - 'result.key.name == "{{ec2_key_name}}"'
+          - 'result.key.fingerprint == "{{fingerprint}}"'
+          - 'result.msg == "key pair already exists"'
+
     # ============================================================
 
     - name: test force=no with another_key_material (expect changed=false)

tests/unit/plugins/modules/test_ec2_key.py

@@ -386,9 +386,45 @@ def test_update_key_pair_by_key_material_update_needed(m_get_key_fingerprint, m_
     m_extract_key_data.assert_called_with(key)
 
 
-@patch(module_name + '.extract_key_data')
-@patch(module_name + '._create_key_pair')
-@patch(module_name + '.delete_key_pair')
+@patch(module_name + ".extract_key_data")
+@patch(module_name + ".get_key_fingerprint")
+def test_update_key_pair_by_key_material_key_exists(m_get_key_fingerprint, m_extract_key_data):
+    ec2_client = MagicMock()
+
+    key_material = MagicMock()
+    key_fingerprint = MagicMock()
+    tag_spec = MagicMock()
+    key_id = MagicMock()
+    key_name = MagicMock()
+    key = {
+        "KeyName": key_name,
+        "KeyFingerprint": key_fingerprint,
+        "KeyPairId": key_id,
+        "Tags": {},
+    }
+
+    check_mode = False
+    m_get_key_fingerprint.return_value = key_fingerprint
+    m_extract_key_data.return_value = {
+        "name": key_name,
+        "fingerprint": key_fingerprint,
+        "id": key_id,
+        "tags": {},
+    }
+
+    expected_result = {"changed": False, "key": m_extract_key_data.return_value, "msg": "key pair already exists"}
+
+    assert expected_result == ec2_key.update_key_pair_by_key_material(
+        check_mode, ec2_client, key_name, key, key_material, tag_spec
+    )
+
+    m_get_key_fingerprint.assert_called_once_with(check_mode, ec2_client, key_material)
+    m_extract_key_data.assert_called_once_with(key)
+
+
+@patch(module_name + ".extract_key_data")
+@patch(module_name + "._create_key_pair")
+@patch(module_name + ".delete_key_pair")
 def test_update_key_pair_by_key_type_update_needed(m_delete_key_pair, m__create_key_pair, m_extract_key_data):
     module = MagicMock()
     ec2_client = MagicMock()
@@ -415,7 +451,7 @@ def test_update_key_pair_by_key_type_update_needed(m_delete_key_pair, m__create_
         "type": "rsa"
     }
 
-    expected_result = {'changed': True, 'key': m_extract_key_data.return_value, 'msg': "key pair updated"}
+    expected_result = {"changed": True, "key": m_extract_key_data.return_value, "msg": "key pair updated"}
 
     result = ec2_key.update_key_pair_by_key_type(module.check_mode, ec2_client, name, key_type, tag_spec)
 
@@ -539,7 +575,7 @@ def test_handle_existing_key_pair_else(m_extract_key_data):
         "type": "rsa"
     }
 
-    expected_result = {'changed': False, 'key': m_extract_key_data.return_value, 'msg': 'key pair alreday exists'}
+    expected_result = {"changed": False, "key": m_extract_key_data.return_value, "msg": "key pair already exists"}
 
     result = ec2_key.handle_existing_key_pair_update(module, ec2_client, name, key)