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

Fix crypttab python3 compatibility issue #30457

Merged
merged 1 commit into from
Sep 27, 2017

Conversation

jpiron
Copy link
Contributor

@jpiron jpiron commented Sep 16, 2017

SUMMARY

The crypttab module doesn't work with Python3.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

crypttab

ANSIBLE VERSION
ansible 2.5.0 (crypttab_python3_compatibility_fix 8c4532203c) last updated 2017/09/16 14:27:26 (GMT +200)
  config file = None
  configured module search path = [u'/home/john/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/john/workspace/ansible/lib/ansible
  executable location = /home/john/workspace/ansible/bin/ansible
  python version = 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609]
ADDITIONAL INFORMATION

Here is how to reproduce:
Python2 working case:

./hacking/test-module -I 'ansible_python_interpreter=/usr/bin/python2' -m lib/ansible/modules/system/crypttab.py -a '{"name":"foo","backing_device":"/dev/bar","state":"present","path":"/tmp/crypttab"}'
* including generated source, if any, saving to: /home/john/.ansible_module_generated
* ansiballz module detected; extracted module source to: /home/john/debug_dir
***********************************
RAW OUTPUT

{"group": "john", "name": "foo", "backing_device": "/dev/bar", "warnings": ["Module did not set no_log for password"], "changed": true, "invocation": {"module_args": {"name": "foo", "backing_device": "/dev/bar", "state": "present", "path": "/tmp/crypttab", "password": null, "opts": null}}, "state": "file", "gid": 1000, "mode": "0664", "msg": "added line", "owner": "john", "path": "/tmp/crypttab", "password": null, "uid": 1000, "opts": null, "size": 13}


***********************************
PARSED OUTPUT
{
    "backing_device": "/dev/bar", 
    "changed": true, 
    "gid": 1000, 
    "group": "john", 
    "invocation": {
        "module_args": {
            "backing_device": "/dev/bar", 
            "name": "foo", 
            "opts": null, 
            "password": null, 
            "path": "/tmp/crypttab", 
            "state": "present"
        }
    }, 
    "mode": "0664", 
    "msg": "added line", 
    "name": "foo", 
    "opts": null, 
    "owner": "john", 
    "password": null, 
    "path": "/tmp/crypttab", 
    "size": 13, 
    "state": "file", 
    "uid": 1000, 
    "warnings": [
        "Module did not set no_log for password"
    ]
}

Python3 not working case:

./hacking/test-module -I 'ansible_python_interpreter=/usr/bin/python3' -m lib/ansible/modules/system/crypttab.py -a '{"name":"foo","backing_device":"/dev/bar","state":"present","path":"/tmp/crypttab"}'
* including generated source, if any, saving to: /home/john/.ansible_module_generated
* ansiballz module detected; extracted module source to: /home/john/debug_dir
***********************************
RAW OUTPUT

Traceback (most recent call last):
  File "/home/john/debug_dir/ansible_module_crypttab.py", line 372, in <module>
    main()
  File "/home/john/debug_dir/ansible_module_crypttab.py", line 172, in main
    f.write(str(crypttab))
TypeError: a bytes-like object is required, not 'str'

***********************************
INVALID OUTPUT FORMAT

Traceback (most recent call last):
  File "./hacking/test-module", line 220, in runtest
    results = json.loads(out)
  File "/usr/lib/python2.7/json/__init__.py", line 339, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python2.7/json/decoder.py", line 364, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python2.7/json/decoder.py", line 382, in raw_decode
    raise ValueError("No JSON object could be decoded")
ValueError: No JSON object could be decoded

@ansibot
Copy link
Contributor

ansibot commented Sep 16, 2017

cc @groks
click here for bot help

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 bugfix_pull_request community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. python3 support:community This issue/PR relates to code supported by the Ansible community. labels Sep 16, 2017
@groks
Copy link
Contributor

groks commented Sep 16, 2017

shipit

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Sep 16, 2017
@abadger
Copy link
Contributor

abadger commented Sep 21, 2017

We want to do the opposite. open the file in binary mode and explicitly change the data to bytes. This gives us greater control over the encoding and what to do on errors.

from ansible.module_utils._text import to_bytes
[...]
            f = open(path, 'wb')
            f.write(to_bytes(crypttab, errors='surrogate_or_strict'))

@abadger
Copy link
Contributor

abadger commented Sep 22, 2017

@jpiron Is changing your PR as I mentioned above something you'll have the time to do?

In python2 str gives byte string. In Python3 it gives unicode string so it
can't be written in a binary mode opened file.
Use to_bytes helper function to ensure content being written will be
properly encoded in both python2 and python3.
@jpiron jpiron force-pushed the crypttab_python3_compatibility_fix branch from 8c45322 to eb82dea Compare September 23, 2017 07:15
@jpiron
Copy link
Contributor Author

jpiron commented Sep 23, 2017

Indeed much better than my naïve fix :)
I updated the PR accordingly.

@abadger abadger merged commit 54859a2 into ansible:devel Sep 27, 2017
abadger pushed a commit that referenced this pull request Sep 27, 2017
In python2 str gives byte string. In Python3 it gives unicode string so it
can't be written in a binary mode opened file.
Use to_bytes helper function to ensure content being written will be
properly encoded in both python2 and python3.
(cherry picked from commit 54859a2)
@abadger
Copy link
Contributor

abadger commented Sep 27, 2017

Thanks @jpiron Merged to devel and cherry-picked to stable-2.4 for the 2.4.1 release.

@abadger abadger moved this from TODO to Done in Python 3 compatibility Sep 28, 2017
prasadkatti pushed a commit to prasadkatti/ansible that referenced this pull request Oct 1, 2017
In python2 str gives byte string. In Python3 it gives unicode string so it
can't be written in a binary mode opened file.
Use to_bytes helper function to ensure content being written will be
properly encoded in both python2 and python3.
BondAnthony pushed a commit to BondAnthony/ansible that referenced this pull request Oct 5, 2017
In python2 str gives byte string. In Python3 it gives unicode string so it
can't be written in a binary mode opened file.
Use to_bytes helper function to ensure content being written will be
properly encoded in both python2 and python3.
@jpiron jpiron deleted the crypttab_python3_compatibility_fix branch October 26, 2017 09:14
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. python3 support:community This issue/PR relates to code supported by the Ansible community.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants