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 - Adding backend option and refactoring backend code #236

Merged

Conversation

Ajpantuso
Copy link
Collaborator

SUMMARY

Adding backend option to openssh_keypair so that users can select between the OpenSSH binary and cryptography.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/modules/openssh_keypair

ADDITIONAL INFORMATION
  • Backend code is modeled after other modules in this collection with two backends available to users.
  • Some minor changes were made to the naming conventions in the openssh cryptography module_utils and a utility method also used by openssh_cert was brought under the openssh directory to unify the library structure.
  • Unit tests were updated to the new names in the module_utils and integration tests were enhanced to 1. move setup_bcrypt to it's own folder and 2. iterate through backends if the installed libraries of the test system are sufficient
  • No intentional changes were made to the module's functionality (i.e. no bugfixes), but significant changes were made to the procedural structure, the control flow, and any unnecessary/unused variables/parameters
  • Some cleanup to rationalize try blocks and use targeted exceptions was performed while also keeping calls to fail_json within the backend and having a single call to exit_json in the module main method
  • Otherwise names were updated for improved readability for example self.fingerprint always contains the SHA256 fingerprint rather than sometimes the fingerprint and sometimes the full output of ssh-keygen -lf FILE

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 for this PR! Some first quick comments:

plugins/modules/openssh_keypair.py Outdated Show resolved Hide resolved
@Ajpantuso Ajpantuso changed the title WIP: openssh_keypair - Adding backend option and refactoring backend code openssh_keypair - Adding backend option and refactoring backend code May 23, 2021
@Ajpantuso Ajpantuso marked this pull request as ready for review May 23, 2021 14:59
@felixfontein felixfontein mentioned this pull request May 23, 2021
11 tasks
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.

Looks good! I only have a few smaller comments :)

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.

LGTM!

@felixfontein
Copy link
Contributor

@Ajpantuso do you want to add more things, or should I just merge this?

@Ajpantuso
Copy link
Collaborator Author

@Ajpantuso do you want to add more things, or should I just merge this?

Nothing more for now. Additional choices for private_key_format will be added in a future PR.

@felixfontein felixfontein merged commit c648375 into ansible-collections:main May 23, 2021
@felixfontein
Copy link
Contributor

@Ajpantuso thanks a lot for improving this module!

@Ajpantuso Ajpantuso deleted the openssh_backends_refactor branch May 23, 2021 23:10
@Andersson007
Copy link
Contributor

@Ajpantuso thank you for the great contribution!

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.

3 participants