[PR #1383/b769b6a9 backport][stable-5] ec2_key - fix idempotency issue when updating existing key pair

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

@@ -275,11 +275,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):
@@ -332,7 +336,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

@@ -387,9 +387,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()
@@ -416,7 +452,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)
 
@@ -540,7 +576,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)