openstack/oslo.concurrency
Code Issues Proposed changes

Port processutils to Python 3

Browse Source 
Add encoding and errors parameters to execute() and ssh_execute(). By
default, use the locale encoding in strict mode on Python 2, or the
locale encoding with the 'surrogateescape' error handler on Python 3.

Fix also unit tests to use bytes strings for stdin, stdout and stderr.

Without this change, tests are failing with Python 3 when run with:
PYTHON="python -bb" testr run

Using -bb, Python 3 raises a TypeError when a bytes string is casted
to a text string, which occurred in many places.

Change-Id: I655d5abf932c9a104e3ab487e23c372377f7096a
This commit is contained in:
Victor Stinner 2014-12-11 23:55:50 +01:00
parent deb0152cbc
commit 7c7493feb5
3 changed files with 36 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
oslo_concurrency/processutils.py
View File

@ -23,6 +23,7 @@ import os
import random
import shlex
import signal
import sys
import time
from oslo.utils import importutils
@ -142,6 +143,13 @@ def execute(*cmd, **kwargs):
last attempt, and LOG_ALL_ERRORS requires
logging on each occurence of an error.
:type log_errors: integer.
:param encoding: encoding used to decode stdout and stderr,
sys.getfilesystemencoding() by default.
:type encoding: str
:param errors: error handler used to decode stdout and stderr,
default: 'surrogateescape' on Python 3,
'strict' on Python 2.
:type errors: str
:returns: (stdout, stderr) from process execution
:raises: :class:`UnknownArgumentError` on
receiving unknown arguments
@ -160,6 +168,8 @@ def execute(*cmd, **kwargs):
shell = kwargs.pop('shell', False)
loglevel = kwargs.pop('loglevel', logging.DEBUG)
log_errors = kwargs.pop('log_errors', None)
encoding = kwargs.pop('encoding', sys.getfilesystemencoding())
errors = kwargs.pop('errors', 'surrogateescape' if six.PY3 else 'strict')
if isinstance(check_exit_code, bool):
ignore_exit_code = not check_exit_code
@ -214,6 +224,11 @@ def execute(*cmd, **kwargs):
end_time = time.time() - start_time
LOG.log(loglevel, 'CMD "%s" returned: %s in %ss' %
(sanitized_cmd, _returncode, end_time))
if result is not None:
(stdout, stderr) = result
stdout = stdout.decode(encoding, errors)
stderr = stderr.decode(encoding, errors)
result = (stdout, stderr)
if not ignore_exit_code and _returncode not in check_exit_code:
(stdout, stderr) = result
sanitized_stdout = strutils.mask_password(stdout)
@ -292,7 +307,9 @@ def trycmd(*args, **kwargs):
def ssh_execute(ssh, cmd, process_input=None,
addl_env=None, check_exit_code=True):
addl_env=None, check_exit_code=True,
encoding=sys.getfilesystemencoding(),
errors='surrogateescape' if six.PY3 else 'strict'):
sanitized_cmd = strutils.mask_password(cmd)
LOG.debug('Running cmd (SSH): %s', sanitized_cmd)
if addl_env:
@ -308,8 +325,10 @@ def ssh_execute(ssh, cmd, process_input=None,
# NOTE(justinsb): This seems suspicious...
# ...other SSH clients have buffering issues with this approach
stdout = stdout_stream.read()
stdout = stdout.decode(encoding, errors)
sanitized_stdout = strutils.mask_password(stdout)
stderr = stderr_stream.read()
stderr = stderr.decode(encoding, errors)
sanitized_stderr = strutils.mask_password(stderr)
stdin_stream.close()

16
oslo_concurrency/tests/unit/test_processutils.py
View File

@ -295,7 +295,7 @@ grep foo
out, err = processutils.execute('/usr/bin/env', env_variables=env_vars)
self.assertIn(b'SUPER_UNIQUE_VAR=The answer is 42', out)
self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out)
def test_exception_and_masking(self):
tmpfilename = self.create_tempfiles(
@ -426,7 +426,7 @@ class FakeSshChannel(object):
return self.rc
class FakeSshStream(six.StringIO):
class FakeSshStream(six.BytesIO):
def setup_channel(self, rc):
self.channel = FakeSshChannel(rc)
@ -436,11 +436,11 @@ class FakeSshConnection(object):
self.rc = rc
def exec_command(self, cmd):
stdout = FakeSshStream('stdout')
stdout = FakeSshStream(b'stdout')
stdout.setup_channel(self.rc)
return (six.StringIO(),
return (six.BytesIO(),
stdout,
six.StringIO('stderr'))
six.BytesIO(b'stderr'))
class SshExecuteTestCase(test_base.BaseTestCase):
@ -465,13 +465,13 @@ class SshExecuteTestCase(test_base.BaseTestCase):
def _test_compromising_ssh(self, rc, check):
fixture = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG))
fake_stdin = six.StringIO()
fake_stdin = six.BytesIO()
fake_stdout = mock.Mock()
fake_stdout.channel.recv_exit_status.return_value = rc
fake_stdout.read.return_value = 'password="secret"'
fake_stdout.read.return_value = b'password="secret"'
fake_stderr = six.StringIO('password="foobar"')
fake_stderr = six.BytesIO(b'password="foobar"')
command = 'ls --password="bar"'

16
tests/test_processutils.py
View File

@ -296,7 +296,7 @@ grep foo
out, err = processutils.execute('/usr/bin/env', env_variables=env_vars)
self.assertIn(b'SUPER_UNIQUE_VAR=The answer is 42', out)
self.assertIn('SUPER_UNIQUE_VAR=The answer is 42', out)
def test_exception_and_masking(self):
tmpfilename = self.create_tempfiles(
@ -427,7 +427,7 @@ class FakeSshChannel(object):
return self.rc
class FakeSshStream(six.StringIO):
class FakeSshStream(six.BytesIO):
def setup_channel(self, rc):
self.channel = FakeSshChannel(rc)
@ -437,11 +437,11 @@ class FakeSshConnection(object):
self.rc = rc
def exec_command(self, cmd):
stdout = FakeSshStream('stdout')
stdout = FakeSshStream(b'stdout')
stdout.setup_channel(self.rc)
return (six.StringIO(),
return (six.BytesIO(),
stdout,
six.StringIO('stderr'))
six.BytesIO(b'stderr'))
class SshExecuteTestCase(test_base.BaseTestCase):
@ -466,13 +466,13 @@ class SshExecuteTestCase(test_base.BaseTestCase):
def _test_compromising_ssh(self, rc, check):
fixture = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG))
fake_stdin = six.StringIO()
fake_stdin = six.BytesIO()
fake_stdout = mock.Mock()
fake_stdout.channel.recv_exit_status.return_value = rc
fake_stdout.read.return_value = 'password="secret"'
fake_stdout.read.return_value = b'password="secret"'
fake_stderr = six.StringIO('password="foobar"')
fake_stderr = six.BytesIO(b'password="foobar"')
command = 'ls --password="bar"'