Add modules metal_project_ssh_key{_info}, docs and tests #107

AlexBacho · 2023-08-01T13:13:15Z

implements #65

t0mk · 2023-08-02T07:13:47Z

@AlexBacho 👍 thanks for the PR, I'll take a look and run the tests.

t0mk

We usually call the PRs based on the content, not by the issue they fix, so this could be "Add modules metal_project_ssh_key{_info}, docs and tests" instead of "Feature 65"

t0mk · 2023-08-02T08:02:58Z

plugins/modules/metal_project_ssh_key.py

+        required_one_of=[("label", "id")],
+        required_together=[("label", "key")],
+    )
+    print("!!!!!!!!!!!!!")


Maybe you forgot debug prints here?

print() usually doesn't work in Ansible modules, because Ansible is hijacking stdout <- IMO one of the worst things about developing Ansible modules. I use
https://github.com/zestyping/q
.. for debugging

t0mk · 2023-08-02T08:10:58Z

requirements.txt

@@ -1,2 +1,3 @@
 equinix-metal>=0.2.2
 ansible-specdoc>=0.0.13
+pydantic==1.10.2


this shuold come in a separate PR

all integration tests fail without this, will need to address it before merging this pr

t0mk · 2023-08-02T08:18:52Z

plugins/modules/metal_project_ssh_key.py

+  - equinix.cloud.metal_project_ssh_key:
+      label: "test_key"
+      key: "ssh-dss AAAAB3NzaC1kc3MAAACBAPLEVntPO3L7VUbEwWZ2ErkQJ3KJ8o9kFXJrPcpvVfdNag4jIhQDqbtAUgUy6BclhhbfH9l5nlGTprrpEFkxm/GL91qJUX6xrPkDMjMqx2wSKa4YraReOrCOfkqqEkC3o3G/gYSuvTzLgp2rmPiflypftZyzNM4JZT8jDwFGotJhAAAAFQDPk43bayONtUxjkAcOf+6zP1qb6QAAAIBZHHH0tIlth5ot+Xa/EYuB/M4qh77EkrWUbER0Kki7suskw/ffdKQ0y/v+ZhoAHtBU7BeE3HmP98Vrha1i4cOU+A7DCqV+lK/a+5LoEpua0M2M+VzNSGluYuV4qGpAOxNh3mxUi2R7yXxheN1oks1ROJ/bqkF4BJQXU9Nv49GkZgAAAIByWcsFeOitvzyDaNJOZzEHv9fqGuj0L3maRVWb6O47HGzlMzniIy8WjL2dfgm2/ek+NxVR/yFnYTKDPr6+0uqSD/cb4eHaFbIj7v+k7H8hA1Ioz+duJ1ONAjn6KwneXxOXu15bYIR49P7Go0s9jCdSAP/r9NE5TnE3yiRiQzgEzw== tomk@node"
+      project_id: "local.project_id"


project IDs (and any IDs in Equinix MEtal) are uuids (e.g. 7c7e9e99-71cf-4425-90d7-b9c950d25705) , so the example should also show some uuid.

Please fix this in all the docs, I won't comment on every occurrence.

t0mk · 2023-08-02T08:21:59Z

README.md

 [equinix.cloud.metal_reserved_ip_block_info](./docs/modules/metal_reserved_ip_block_info.md)|Gather list of reserved IP blocks|
 [equinix.cloud.metal_ssh_key_info](./docs/modules/metal_ssh_key_info.md)|Gather personal SSH keys|
+list project SSH keys


Not sure how this rogue line got here. We don't write README.md by hand. If we need to change readme, we do it in the template/README.template.md

t0mk · 2023-08-02T08:30:45Z

plugins/module_utils/metal/api_routes.py

@@ -50,6 +50,9 @@ def get_routes(mpc):
        ('metal_ssh_key', action.GET): spec_types.Specs(
            equinix_metal.SSHKeysApi(mpc).find_ssh_key_by_id,
        ),
+        ('metal_project_ssh_key', action.GET): spec_types.Specs(


as the API route for getting user ssh keys and project ssh keys by ID is the same, we could remove this particular route, and do this in metal_project_ssh_key.py:

try: module.params_syntax_check() if module.params.get("id"): tolerate_not_found = state == "absent" # API getter is the same for user and project key fetched = module.get_by_id("metal_ssh_key", tolerate_not_found) else: fetched = module.get_one_from_list( # API listing method is different for project keys "metal_project_ssh_key", ["key"], )

That way there will be less code to maintain

t0mk · 2023-08-02T08:32:13Z

plugins/module_utils/metal/api_routes.py

@@ -119,6 +126,9 @@ def get_routes(mpc):
        ('metal_ssh_key', action.DELETE): spec_types.Specs(
            equinix_metal.SSHKeysApi(mpc).delete_ssh_key,
        ),
+        ('metal_project_ssh_key', action.DELETE): spec_types.Specs(


please remove this route, and do

module.delete_by_id("metal_ssh_key")

in metal_project_ssh_key.py

t0mk · 2023-08-02T08:34:06Z

plugins/module_utils/metal/api_routes.py

@@ -174,4 +189,9 @@ def get_routes(mpc):
            {},
            equinix_metal.SSHKeyInput,
        ),
+        ('metal_project_ssh_key', action.UPDATE): spec_types.Specs(


please remove this route, and do

fetched = module.update_by_id(diff, "metal_ssh_key")

in metal_project_ssh_key.py

t0mk · 2023-08-04T08:18:58Z

I'm not sure how to run the integration tests here in the PR. This
https://github.com/equinix-labs/ansible-collection-equinix/blob/main/.github/workflows/integration-tests.yml
..should trigger but didn't, probably because Alex doesn't have a role in the repo. I think we should have a command to run the tests or PR by external people.

What do you think @displague ?

t0mk · 2023-08-04T11:05:20Z

Looks good but let's merge #108 first

…ers, update readme

t0mk

I reviewed before, so here I'm just approving

t0mk

I reviewed before (twice), so here I'm just approving

t0mk reviewed Aug 2, 2023

View reviewed changes

t0mk suggested changes Aug 2, 2023

View reviewed changes

t0mk changed the title ~~Feature 65~~ "Add modules metal_project_ssh_key{_info}, docs and tests Aug 4, 2023

t0mk changed the title ~~"Add modules metal_project_ssh_key{_info}, docs and tests~~ Add modules metal_project_ssh_key{_info}, docs and tests Aug 4, 2023

t0mk previously approved these changes Aug 4, 2023

View reviewed changes

AlexBacho added 9 commits August 4, 2023 14:09

add metal_project_ssh_key and _key_info plus docs

b2b52c6

make project_id unchangeble, add int test

2b50273

rename name and public key to match personal ssh key format, add rout…

bb2ac56

…ers, update readme

create new project for integration tests, fix some issues

3795eaa

remove projects after test run

18fa141

fix tests inventory -> remove my namespace

0ecc088

address PR remarks

c487012

use metal ssh key updater

64654b3

fix idempotence issue

eaec37d

AlexBacho force-pushed the feature-65 branch from f50a8a2 to eaec37d Compare August 4, 2023 12:10

touch commit to trigger integration tests

aac865b

t0mk dismissed their stale review via aac865b August 15, 2023 10:55

t0mk temporarily deployed to external August 15, 2023 10:55 — with GitHub Actions Inactive

t0mk previously approved these changes Aug 15, 2023

View reviewed changes

Merge branch 'main' into feature-65

d2e4b3a

t0mk dismissed their stale review via d2e4b3a August 15, 2023 11:07

t0mk temporarily deployed to external August 15, 2023 11:07 — with GitHub Actions Inactive

t0mk approved these changes Aug 15, 2023

View reviewed changes

t0mk merged commit 2a64e13 into equinix:main Aug 15, 2023
2 checks passed

This was referenced Aug 15, 2023

metal_project_ssh_key #65

Closed

metal_project_ssh_key_info #29

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add modules metal_project_ssh_key{_info}, docs and tests #107

Add modules metal_project_ssh_key{_info}, docs and tests #107

AlexBacho commented Aug 1, 2023

t0mk commented Aug 2, 2023 •

edited

Loading

t0mk left a comment •

edited

Loading

t0mk Aug 2, 2023

t0mk Aug 2, 2023

AlexBacho Aug 2, 2023

t0mk Aug 2, 2023 •

edited

Loading

t0mk Aug 2, 2023

t0mk Aug 2, 2023

t0mk Aug 2, 2023

t0mk Aug 2, 2023

t0mk commented Aug 4, 2023 •

edited

Loading

t0mk commented Aug 4, 2023

t0mk left a comment

t0mk left a comment

Add modules metal_project_ssh_key{_info}, docs and tests #107

Add modules metal_project_ssh_key{_info}, docs and tests #107

Conversation

AlexBacho commented Aug 1, 2023

t0mk commented Aug 2, 2023 • edited Loading

t0mk left a comment • edited Loading

Choose a reason for hiding this comment

t0mk Aug 2, 2023

Choose a reason for hiding this comment

t0mk Aug 2, 2023

Choose a reason for hiding this comment

AlexBacho Aug 2, 2023

Choose a reason for hiding this comment

t0mk Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

t0mk Aug 2, 2023

Choose a reason for hiding this comment

t0mk Aug 2, 2023

Choose a reason for hiding this comment

t0mk Aug 2, 2023

Choose a reason for hiding this comment

t0mk Aug 2, 2023

Choose a reason for hiding this comment

t0mk commented Aug 4, 2023 • edited Loading

t0mk commented Aug 4, 2023

t0mk left a comment

Choose a reason for hiding this comment

t0mk left a comment

Choose a reason for hiding this comment

t0mk commented Aug 2, 2023 •

edited

Loading

t0mk left a comment •

edited

Loading

t0mk Aug 2, 2023 •

edited

Loading

t0mk commented Aug 4, 2023 •

edited

Loading