From 28ceb376fc66dbb35eb46145c658d72a8d91ae17 Mon Sep 17 00:00:00 2001 From: Markus Katharina Brechtel Date: Tue, 28 Dec 2021 21:04:01 +0100 Subject: [PATCH 01/10] fix boot ssh key setting --- plugins/modules/boot.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/plugins/modules/boot.py b/plugins/modules/boot.py index 18a81a2..6909f6b 100644 --- a/plugins/modules/boot.py +++ b/plugins/modules/boot.py @@ -384,7 +384,11 @@ def main(): option = module.params[option_name][option_key] if option is None or option == []: continue - data[data_key] = option + + if isinstance(option, list): + data[data_key+'[]'] = option + else: + data[data_key] = option if existing.get('active'): # Idempotence check needs_change = False @@ -392,7 +396,10 @@ def main(): should = module.params[option_name][option_key] if should is None: continue - has = existing.get(data_key) + if option_key == 'authorized_keys': + has = list(map(lambda x: x['key']['fingerprint'], existing.get(data_key))) + else: + has = existing.get(data_key) if isinstance(has, list): has = sorted(has) if not isinstance(should, list): @@ -415,7 +422,7 @@ def main(): result, dummy = fetch_url_json( module, url, - data=urlencode(data), + data=urlencode(data,True), headers=headers, method='POST', ) From 4056422220aecbc551643d81ad6e93b7e9c69337 Mon Sep 17 00:00:00 2001 From: Markus Katharina Brechtel Date: Tue, 28 Dec 2021 22:15:11 +0100 Subject: [PATCH 02/10] code style --- plugins/modules/boot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/boot.py b/plugins/modules/boot.py index 6909f6b..278dc9b 100644 --- a/plugins/modules/boot.py +++ b/plugins/modules/boot.py @@ -422,7 +422,7 @@ def main(): result, dummy = fetch_url_json( module, url, - data=urlencode(data,True), + data=urlencode(data, True), headers=headers, method='POST', ) From c1eef733446ad5b7649e139f4c0f9ea71bb643ca Mon Sep 17 00:00:00 2001 From: Markus Katharina Brechtel Date: Tue, 28 Dec 2021 22:17:38 +0100 Subject: [PATCH 03/10] boot idempotency check comment --- plugins/modules/boot.py | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/modules/boot.py b/plugins/modules/boot.py index 278dc9b..13849b8 100644 --- a/plugins/modules/boot.py +++ b/plugins/modules/boot.py @@ -396,6 +396,7 @@ def main(): should = module.params[option_name][option_key] if should is None: continue + # unfold the return object for the idempotence check to work correctly if option_key == 'authorized_keys': has = list(map(lambda x: x['key']['fingerprint'], existing.get(data_key))) else: From 7d6df717746968b6c2b68ae46fef18c87bd9b7db Mon Sep 17 00:00:00 2001 From: Markus Katharina Brechtel Date: Tue, 28 Dec 2021 22:27:38 +0100 Subject: [PATCH 04/10] changelog fragment --- changelogs/fragments/33-fix-boot-ssh-key-setting.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/33-fix-boot-ssh-key-setting.yaml diff --git a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml new file mode 100644 index 0000000..4ae1379 --- /dev/null +++ b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml @@ -0,0 +1,3 @@ +bugfixes: + - apt_repository - fix crash caused by ``cache.update()`` raising an ``IOError`` + due to a timeout in ``apt update`` (https://github.com/ansible/ansible/issues/51995). From 635d730067ce171fec54f05ff04de8fce57d1745 Mon Sep 17 00:00:00 2001 From: Markus Katharina Brechtel Date: Tue, 28 Dec 2021 23:07:12 +0100 Subject: [PATCH 05/10] changelog fragment with save --- changelogs/fragments/33-fix-boot-ssh-key-setting.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml index 4ae1379..d7e1a94 100644 --- a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml +++ b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml @@ -1,3 +1,2 @@ bugfixes: - - apt_repository - fix crash caused by ``cache.update()`` raising an ``IOError`` - due to a timeout in ``apt update`` (https://github.com/ansible/ansible/issues/51995). + - bugfix for incorrect handling of ssh-keys for the community.hcloud.boot module. Trying to set the boot record with the module lets the API return errors (reported in https://github.com/ansible-collections/community.hrobot/issues/32). This is and the idempotence for ssh-keys is fixed with the https://github.com/ansible-collections/community.hrobot/pull/33 PR. From 56df49f88b30388796bbac98fd53cdc8a6343a6e Mon Sep 17 00:00:00 2001 From: Markus Katharina Brechtel Date: Tue, 28 Dec 2021 23:08:32 +0100 Subject: [PATCH 06/10] fix changelog fragment wording --- changelogs/fragments/33-fix-boot-ssh-key-setting.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml index d7e1a94..57065c4 100644 --- a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml +++ b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml @@ -1,2 +1,2 @@ bugfixes: - - bugfix for incorrect handling of ssh-keys for the community.hcloud.boot module. Trying to set the boot record with the module lets the API return errors (reported in https://github.com/ansible-collections/community.hrobot/issues/32). This is and the idempotence for ssh-keys is fixed with the https://github.com/ansible-collections/community.hrobot/pull/33 PR. + - bugfix for incorrect handling of ssh-keys for the community.hcloud.boot module. Trying to set the boot record with the module lets the API return errors (reported in https://github.com/ansible-collections/community.hrobot/issues/32). This is and the idempotence for ssh-keys is fixed with the https://github.com/ansible-collections/community.hrobot/pull/33 MR. From a2f49bca70c8402f52945f4a9eefd2680f5bc02a Mon Sep 17 00:00:00 2001 From: Markus Katharina Brechtel Date: Wed, 29 Dec 2021 12:51:04 +0100 Subject: [PATCH 07/10] Update changelogs/fragments/33-fix-boot-ssh-key-setting.yaml Co-authored-by: Felix Fontein --- changelogs/fragments/33-fix-boot-ssh-key-setting.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml index 57065c4..3480829 100644 --- a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml +++ b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml @@ -1,2 +1,2 @@ bugfixes: - - bugfix for incorrect handling of ssh-keys for the community.hcloud.boot module. Trying to set the boot record with the module lets the API return errors (reported in https://github.com/ansible-collections/community.hrobot/issues/32). This is and the idempotence for ssh-keys is fixed with the https://github.com/ansible-collections/community.hrobot/pull/33 MR. + - boot - fix incorrect handling of ssh-keys (https://github.com/ansible-collections/community.hrobot/issues/32, https://github.com/ansible-collections/community.hrobot/pull/33). From e5b0791d527eeac2337b42511f096d9a84931017 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 4 Jan 2022 05:42:12 +0100 Subject: [PATCH 08/10] Apply suggestions from code review --- plugins/modules/boot.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/plugins/modules/boot.py b/plugins/modules/boot.py index 13849b8..1d524d3 100644 --- a/plugins/modules/boot.py +++ b/plugins/modules/boot.py @@ -384,11 +384,7 @@ def main(): option = module.params[option_name][option_key] if option is None or option == []: continue - - if isinstance(option, list): - data[data_key+'[]'] = option - else: - data[data_key] = option + data[data_key] = option if existing.get('active'): # Idempotence check needs_change = False @@ -397,10 +393,9 @@ def main(): if should is None: continue # unfold the return object for the idempotence check to work correctly - if option_key == 'authorized_keys': - has = list(map(lambda x: x['key']['fingerprint'], existing.get(data_key))) - else: - has = existing.get(data_key) + has = existing.get(data_key) + if has and option_key == 'authorized_keys': + has = [x['key']['fingerprint'] for x in has] if isinstance(has, list): has = sorted(has) if not isinstance(should, list): From 2f90009ee9a42dec53b28a334ba19be6a23e9f6c Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 4 Jan 2022 05:56:02 +0100 Subject: [PATCH 09/10] Adjust tests. --- tests/unit/plugins/modules/test_boot.py | 93 +++++++++++++++++++++---- 1 file changed, 80 insertions(+), 13 deletions(-) diff --git a/tests/unit/plugins/modules/test_boot.py b/tests/unit/plugins/modules/test_boot.py index 86c01df..427676c 100644 --- a/tests/unit/plugins/modules/test_boot.py +++ b/tests/unit/plugins/modules/test_boot.py @@ -157,15 +157,40 @@ def test_rescue_idempotent_2(self, mocker): 'os': 'linux', 'arch': 32, 'authorized_keys': [ - 'abc', - 'def', - 'afz', + 'e4:47:42:71:81:62:bf:06:1c:23:fa:f3:8f:7b:6f:d0', + 'aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99', + '0f:1e:2d:3c:4b:5a:69:78:87:96:a5:b4:c3:d2:e1:f0', ], }, }, [ FetchUrlCall('GET', 200) .result_json(_amend_boot({ - 'rescue': create_rescue_active(os='linux', arch=32, authorized_key=['def', 'afz', 'abc']), + 'rescue': create_rescue_active(os='linux', arch=32, authorized_key=[ + { + 'key': { + 'fingerprint': 'e4:47:42:71:81:62:bf:06:1c:23:fa:f3:8f:7b:6f:d0', + 'name': 'baz', + 'size': 4096, + 'type': 'RSA', + }, + }, + { + 'key': { + 'fingerprint': 'aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99', + 'name': 'foo bar', + 'size': 2048, + 'type': 'RSA', + }, + }, + { + 'key': { + 'fingerprint': '0f:1e:2d:3c:4b:5a:69:78:87:96:a5:b4:c3:d2:e1:f0', + 'name': 'test', + 'size': 3072, + 'type': 'RSA', + }, + }, + ]), })) .expect_url('{0}/boot/23'.format(BASE_URL)), ]) @@ -333,15 +358,40 @@ def test_install_linux_idempotent_2(self, mocker): 'arch': 32, 'lang': 'de', 'authorized_keys': [ - 'abc', - 'def', - 'afz', + 'e4:47:42:71:81:62:bf:06:1c:23:fa:f3:8f:7b:6f:d0', + '0f:1e:2d:3c:4b:5a:69:78:87:96:a5:b4:c3:d2:e1:f0', + 'aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99', ], }, }, [ FetchUrlCall('GET', 200) .result_json(_amend_boot({ - 'linux': create_linux_active(dist='Arch Linux latest minimal', arch=32, lang='de', authorized_key=['def', 'afz', 'abc']), + 'linux': create_linux_active(dist='Arch Linux latest minimal', arch=32, lang='de', authorized_key=[ + { + 'key': { + 'fingerprint': 'e4:47:42:71:81:62:bf:06:1c:23:fa:f3:8f:7b:6f:d0', + 'name': 'abc', + 'size': 4096, + 'type': 'RSA', + }, + }, + { + 'key': { + 'fingerprint': 'aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99', + 'name': 'buzz', + 'size': 2048, + 'type': 'RSA', + }, + }, + { + 'key': { + 'fingerprint': '0f:1e:2d:3c:4b:5a:69:78:87:96:a5:b4:c3:d2:e1:f0', + 'name': 'afz', + 'size': 2048, + 'type': 'RSA', + }, + }, + ]), })) .expect_url('{0}/boot/23'.format(BASE_URL)), ]) @@ -404,8 +454,8 @@ def test_install_linux_reactivate(self, mocker): 'arch': 32, 'lang': 'fr', 'authorized_keys': [ - 'asdf', - 'foo bar', + 'e4:47:42:71:81:62:bf:06:1c:23:fa:f3:8f:7b:6f:d0', + 'aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99', ], }, }, [ @@ -421,10 +471,27 @@ def test_install_linux_reactivate(self, mocker): .expect_form_value('arch', '32') .expect_form_value('lang', 'fr') .expect_form_present('authorized_key') - # .expect_form_value('authorized_key', 'asdf') - # .expect_form_value('authorized_key', 'foo bar') + # .expect_form_value('authorized_key', 'e4:47:42:71:81:62:bf:06:1c:23:fa:f3:8f:7b:6f:d0') + # .expect_form_value('authorized_key', 'aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99') .result_json({ - 'linux': create_linux_active(dist='Debian 11 base', lang='fr', arch=32, authorized_key=['foo bar', 'asdf']), + 'linux': create_linux_active(dist='Debian 11 base', lang='fr', arch=32, authorized_key=[ + { + 'key': { + 'fingerprint': 'aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99', + 'name': 'foo bar', + 'size': 4096, + 'type': 'RSA', + }, + }, + { + 'key': { + 'fingerprint': 'e4:47:42:71:81:62:bf:06:1c:23:fa:f3:8f:7b:6f:d0', + 'name': 'bar', + 'size': 2048, + 'type': 'RSA', + }, + }, + ]), }) .expect_url('{0}/boot/23/linux'.format(BASE_URL)), ]) From ee5124537a33c55e67b1a19519e7494e0fe5114c Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 4 Jan 2022 06:04:04 +0100 Subject: [PATCH 10/10] Update changelog fragment. --- changelogs/fragments/33-fix-boot-ssh-key-setting.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml index 3480829..3134b04 100644 --- a/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml +++ b/changelogs/fragments/33-fix-boot-ssh-key-setting.yaml @@ -1,2 +1,2 @@ bugfixes: - - boot - fix incorrect handling of ssh-keys (https://github.com/ansible-collections/community.hrobot/issues/32, https://github.com/ansible-collections/community.hrobot/pull/33). + - boot - fix incorrect handling of SSH authorized keys (https://github.com/ansible-collections/community.hrobot/issues/32, https://github.com/ansible-collections/community.hrobot/pull/33).