diff --git a/changelogs/fragments/1383-ec2_key-fix-idempotency-issue-when-creating-existing-key-with-same-key-material.yaml b/changelogs/fragments/1383-ec2_key-fix-idempotency-issue-when-creating-existing-key-with-same-key-material.yaml new file mode 100644 index 00000000000..db0223063de --- /dev/null +++ b/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). diff --git a/plugins/modules/ec2_key.py b/plugins/modules/ec2_key.py index 6b711a7ae5e..0023ce6255f 100644 --- a/plugins/modules/ec2_key.py +++ b/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 diff --git a/tests/integration/targets/ec2_key/tasks/main.yml b/tests/integration/targets/ec2_key/tasks/main.yml index 6eab3bab967..2754289f1ee 100644 --- a/tests/integration/targets/ec2_key/tasks/main.yml +++ b/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) diff --git a/tests/unit/plugins/modules/test_ec2_key.py b/tests/unit/plugins/modules/test_ec2_key.py index 094941e7c5d..413eec00656 100644 --- a/tests/unit/plugins/modules/test_ec2_key.py +++ b/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)