Skip to content

Commit

Permalink
fix idempotency issue when creating existing key pair with key materi…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
abikouo authored Feb 27, 2023
1 parent 0ef8359 commit b769b6a
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -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).
10 changes: 7 additions & 3 deletions plugins/modules/ec2_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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


Expand Down
23 changes: 22 additions & 1 deletion tests/integration/targets/ec2_key/tasks/main.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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)
Expand Down
46 changes: 41 additions & 5 deletions tests/unit/plugins/modules/test_ec2_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit b769b6a

Please sign in to comment.