Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openssh_keypair: Populate return values when keypair exists and check_mode=true #230

Conversation

Ajpantuso
Copy link
Collaborator

SUMMARY

Enhancing check_mode for openssh_keypair with populated return values if keypair already exists.

Fixes #113

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/modules/openssh_keypair.py

ADDITIONAL INFORMATION

Straight forward change to execute the check logic prior to dumping the return values into the results dictionary when check_mode=true.

Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! BTW, you can ignore the failing third-party check, it currently breaks randomly...

@@ -0,0 +1,2 @@
minor_changes:
- openssh_keypair - enhanced ``check_mode`` to populate return values for existing keypairs (https://github.com/ansible-collections/community.crypto/pull/230).
Copy link
Contributor

@felixfontein felixfontein May 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- openssh_keypair - enhanced ``check_mode`` to populate return values for existing keypairs (https://github.com/ansible-collections/community.crypto/pull/230).
- openssh_keypair - fix ``check_mode`` to populate return values for existing keypairs (https://github.com/ansible-collections/community.crypto/issues/113, https://github.com/ansible-collections/community.crypto/pull/230).

Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually (looking at what needs to be changed) I think this is more a bugfix than a feature.

@@ -0,0 +1,2 @@
minor_changes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
minor_changes:
bugfixes:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

@felixfontein
Copy link
Contributor

@Ajpantuso btw, if you have time to review something, #203, #204, #205 and #206 are looking for reviews ;)

@Ajpantuso
Copy link
Collaborator Author

@Ajpantuso btw, if you have time to review something, #203, #204, #205 and #206 are looking for reviews ;)

Ah, so the tables have turned :). Will take a look before my next contribution here since I saw at least one of these blocking another request.

@felixfontein felixfontein merged commit 80d64e7 into ansible-collections:main May 12, 2021
@felixfontein
Copy link
Contributor

@Ajpantuso thanks for fixing/implementing this! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openssh_keypair: check_mode should return public key
2 participants