From b769b6a9b6b564ed4b65636a70aaf802a90c0b9b Mon Sep 17 00:00:00 2001 From: Bikouo Aubin <79859644+abikouo@users.noreply.github.com> Date: Mon, 27 Feb 2023 13:16:14 +0100 Subject: [PATCH] fix idempotency issue when creating existing key pair with key material (#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 \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 \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 --- ...g-existing-key-with-same-key-material.yaml | 3 ++ plugins/modules/ec2_key.py | 10 ++-- .../targets/ec2_key/tasks/main.yml | 23 +++++++++- tests/unit/plugins/modules/test_ec2_key.py | 46 +++++++++++++++++-- 4 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/1383-ec2_key-fix-idempotency-issue-when-creating-existing-key-with-same-key-material.yaml 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)