Added explicit encoding for key_data string. #38
Conversation
|
Can somebody explain to me why no one is checking or merging this PR? Is there any contribution guidance? I didn't find any. |
|
@phospi this lgtm, but I'm just a user and don't have any privs on the project. I recommend trying to ping someone in IRC in the #ansible-aws channel |
|
@phospi I don't know why there still has been zero response to this. In the meantime you should consider adding a changelog as you'll likely be asked to once someone looks at this. |
There was a problem hiding this comment.
@phospi Thanks very much for taking the time to make a PR. Things are still kind of in flux with this repo, I think. It still needs the bot hooked up so module authors are pinged. Ideally @RickMendes would review as the author. Reviews tend to happen whenever community members have time, as just about everyone is a volunteer.
plugins/modules/ec2_win_password.py
Outdated
| @@ -181,7 +181,7 @@ def main(): | |||
| module.fail_json(msg="unable to parse key file") | |||
| elif key_data is not None and key_file is None: | |||
| try: | |||
| key = load_pem_private_key(key_data, b_key_passphrase, default_backend()) | |||
| key = load_pem_private_key(key_data.encode('ascii'), b_key_passphrase, default_backend()) | |||
There was a problem hiding this comment.
It looks like load_pem_private_key expects a bytes like object so the key_data can just use to_bytes (like b_key_passphrase).
There was a problem hiding this comment.
This is how I did it for b_key_passphrase:
b_key_passphrase = to_bytes(module.params.get('key_passphrase'), err ors='surrogate_or_strict')
OMG - I haven't seen this code since 2016.
|
@RickMendes @s-hertel Would you approve this PR? |
|
@phospi I would feel better about approving this if you were using a bytes object instead of an encoded string. It may work today, but you are still passing a different object than load_pem_private_key expects. |
Switched from AnsibleModule to AnsibleAWSModule
|
@RickMendes Good point. I did the modification. I did some refactoring as well. The main reason is that it is now easier to use with testing. I added a first test case to ensure basic functionality. |
|
@phospi Thanks for making those changes. I approve 👍 |
|
Re-approved 👍 |
|
Thanks so much @phospi for the patch! |
Added explicit encoding for key_data string. This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@2f8c7b6
Added explicit encoding for key_data string. This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@2f8c7b6
Added explicit encoding for key_data string. This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@2f8c7b6
Added explicit encoding for key_data string. This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@1404931
Added explicit encoding for key_data string.
Added explicit encoding for key_data string.
Added explicit encoding for key_data string.
…ions#38) * Move last boto 2 modules to AnsibleAWSModule * Move last boto3 modules to AnsibleAWSModule * Remove last use of ec2_argument_spec in modules * HAS_BOTO3 logic no longer required - Using AnsibleAWSModule * Be more consistent in our use of HAS_BOTO * Split imports by line - much easier to work with during refactoring/cleanup * Use module.client() to get boto3 clients * Shuffle import orders for consistency * Pull camel_dict_to_snake_dict from its canonical source * Use module.fail_json_aws and is_boto3_error_code to simplify error handling * tweak integration tests to allow more information in the error messages
SUMMARY
Explicit encoding for pem key variable was added and tested with:
Fixes #37
ISSUE TYPE
COMPONENT NAME
ec2_win_password
ADDITIONAL INFORMATION
This pull request is supposed to ensure compatibility with python2 and python3 for module ec2_win_password