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 use of bytes in cliconf plugins for Python 3 #37715

Merged
merged 6 commits into from
Apr 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 5 additions & 7 deletions lib/ansible/plugins/cliconf/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,11 @@ class CliconfBase(with_metaclass(ABCMeta, object)):
"""
A base class for implementing cli connections

.. note:: Unlike most of Ansible, nearly all strings in
:class:`CliconfBase` plugins are byte strings. This is because of
how close to the underlying platform these plugins operate. Remember
to mark literal strings as byte string (``b"string"``) and to use
:func:`~ansible.module_utils._text.to_bytes` and
:func:`~ansible.module_utils._text.to_text` to avoid unexpected
problems.
.. note:: String inputs to :meth:`send_command` will be cast to byte strings
within this method and as such are not required to be made byte strings
beforehand. Please avoid using literal byte strings (``b'string'``) in
:class:`CliConfBase` plugins as this can lead to unexpected errors when
running on Python 3

List of supported rpc's:
:get_config: Retrieves the specified configuration from the device
Expand Down
17 changes: 7 additions & 10 deletions lib/ansible/plugins/cliconf/edgeos.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from itertools import chain

from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils._text import to_text
from ansible.module_utils.network.common.utils import to_list
from ansible.plugins.cliconf import CliconfBase

Expand All @@ -21,7 +21,7 @@ def get_device_info(self):
device_info = {}

device_info['network_os'] = 'edgeos'
reply = self.get(b'show version')
reply = self.get('show version')
data = to_text(reply, errors='surrogate_or_strict').strip()

match = re.search(r'Version:\s*v?(\S+)', data)
Expand All @@ -32,24 +32,21 @@ def get_device_info(self):
if match:
device_info['network_os_model'] = match.group(1)

reply = self.get(b'show host name')
reply = self.get('show host name')
reply = to_text(reply, errors='surrogate_or_strict').strip()
device_info['network_os_hostname'] = reply

return device_info

def get_config(self, source='running', format='text'):
return self.send_command(b'show configuration commands')
return self.send_command('show configuration commands')

def edit_config(self, command):
for cmd in chain([b'configure'], to_list(command)):
for cmd in chain(['configure'], to_list(command)):
self.send_command(cmd)

def get(self, command, prompt=None, answer=None, sendonly=False):
return self.send_command(to_bytes(command),
prompt=to_bytes(prompt),
answer=to_bytes(answer),
sendonly=sendonly)
return self.send_command(command, prompt=prompt, answer=answer, sendonly=sendonly)

def commit(self, comment=None):
if comment:
Expand All @@ -59,7 +56,7 @@ def commit(self, comment=None):
self.send_command(command)

def discard_changes(self, *args, **kwargs):
self.send_command(b'discard')
self.send_command('discard')

def get_capabilities(self):
result = {}
Expand Down
10 changes: 5 additions & 5 deletions lib/ansible/plugins/cliconf/eos.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ def get_device_info(self):
device_info = {}

device_info['network_os'] = 'eos'
reply = self.get(b'show version | json')
reply = self.get('show version | json')
data = json.loads(reply)

device_info['network_os_version'] = data['version']
device_info['network_os_model'] = data['modelName']

reply = self.get(b'show hostname | json')
reply = self.get('show hostname | json')
data = json.loads(reply)

device_info['network_os_hostname'] = data['hostname']
Expand All @@ -52,17 +52,17 @@ def get_config(self, source='running', format='text', flags=None):
if source not in lookup:
return self.invalid_params("fetching configuration from %s is not supported" % source)

cmd = b'show %s ' % lookup[source]
cmd = 'show %s ' % lookup[source]
if format and format is not 'text':
cmd += b'| %s ' % format
cmd += '| %s ' % format

cmd += ' '.join(to_list(flags))
cmd = cmd.strip()
return self.send_command(cmd)

@enable_mode
def edit_config(self, command):
for cmd in chain([b'configure'], to_list(command), [b'end']):
for cmd in chain(['configure'], to_list(command), ['end']):
self.send_command(cmd)

def get(self, command, prompt=None, answer=None, sendonly=False):
Expand Down
4 changes: 2 additions & 2 deletions lib/ansible/plugins/cliconf/ios.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

from itertools import chain

from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils._text import to_text
from ansible.module_utils.network.common.utils import to_list
from ansible.plugins.cliconf import CliconfBase, enable_mode

Expand All @@ -35,7 +35,7 @@ def get_device_info(self):
device_info = {}

device_info['network_os'] = 'ios'
reply = self.get(b'show version')
reply = self.get('show version')
data = to_text(reply, errors='surrogate_or_strict').strip()

match = re.search(r'Version (\S+),', data)
Expand Down
12 changes: 6 additions & 6 deletions lib/ansible/plugins/cliconf/iosxr.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

from itertools import chain

from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils._text import to_text
from ansible.module_utils.network.common.utils import to_list
from ansible.plugins.cliconf import CliconfBase

Expand All @@ -35,7 +35,7 @@ def get_device_info(self):
device_info = {}

device_info['network_os'] = 'iosxr'
reply = self.get(b'show version | utility head -n 20')
reply = self.get('show version | utility head -n 20')
data = to_text(reply, errors='surrogate_or_strict').strip()

match = re.search(r'Version (\S+)$', data, re.M)
Expand All @@ -61,9 +61,9 @@ def get_config(self, source='running', format='text', filter=None):
if source not in lookup:
return self.invalid_params("fetching configuration from %s is not supported" % source)
if filter:
cmd = to_bytes('show {0} {1}'.format(lookup[source], filter), errors='surrogate_or_strict')
cmd = 'show {0} {1}'.format(lookup[source], filter)
else:
cmd = to_bytes('show {0}'.format(lookup[source]), errors='surrogate_or_strict')
cmd = 'show {0}'.format(lookup[source])

return self.send_command(cmd)

Expand Down Expand Up @@ -94,10 +94,10 @@ def commit(self, comment=None):
command = 'commit comment {0}'.format(comment)
else:
command = 'commit'
self.send_command(to_bytes(command))
self.send_command(command)

def discard_changes(self):
self.send_command(b'abort')
self.send_command('abort')

def get_capabilities(self):
result = {}
Expand Down
16 changes: 8 additions & 8 deletions lib/ansible/plugins/cliconf/junos.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import re
from itertools import chain

from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils._text import to_text
from ansible.module_utils.network.common.utils import to_list
from ansible.plugins.cliconf import CliconfBase

Expand Down Expand Up @@ -78,15 +78,15 @@ def commit(self, *args, **kwargs):
comment: Optional commit description.
"""
comment = kwargs.get('comment', None)
command = b'commit'
command = 'commit'
if comment:
command += b' comment {0}'.format(comment)
command += b' and-quit'
command += ' comment {0}'.format(comment)
command += ' and-quit'
return self.send_command(command)

def discard_changes(self):
command = b'rollback 0'
for cmd in chain(to_list(command), b'exit'):
command = 'rollback 0'
for cmd in chain(to_list(command), 'exit'):
self.send_command(cmd)

def get_capabilities(self):
Expand All @@ -97,7 +97,7 @@ def get_capabilities(self):
return json.dumps(result)

def compare_configuration(self, rollback_id=None):
command = b'show | compare'
command = 'show | compare'
if rollback_id is not None:
command += b' rollback %s' % int(rollback_id)
command += ' rollback %s' % int(rollback_id)
return self.send_command(command)
2 changes: 1 addition & 1 deletion lib/ansible/plugins/cliconf/nxos.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def get_device_info(self):
def get_config(self, source='running', format='text', flags=None):
lookup = {'running': 'running-config', 'startup': 'startup-config'}

cmd = 'show {} '.format(lookup[source])
cmd = 'show {0} '.format(lookup[source])
if flags:
cmd += ' '.join(flags)
cmd = cmd.strip()
Expand Down
14 changes: 7 additions & 7 deletions lib/ansible/plugins/cliconf/vyos.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

from itertools import chain

from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils._text import to_text
from ansible.module_utils.network.common.utils import to_list
from ansible.plugins.cliconf import CliconfBase

Expand All @@ -35,7 +35,7 @@ def get_device_info(self):
device_info = {}

device_info['network_os'] = 'vyos'
reply = self.get(b'show version')
reply = self.get('show version')
data = to_text(reply, errors='surrogate_or_strict').strip()

match = re.search(r'Version:\s*(\S+)', data)
Expand All @@ -46,17 +46,17 @@ def get_device_info(self):
if match:
device_info['network_os_model'] = match.group(1)

reply = self.get(b'show host name')
reply = self.get('show host name')
device_info['network_os_hostname'] = to_text(reply, errors='surrogate_or_strict').strip()

return device_info

def get_config(self, source='running', format='text'):
return self.send_command(b'show configuration commands')
return self.send_command('show configuration commands')

def edit_config(self, command):
for cmd in chain(['configure'], to_list(command)):
self.send_command(to_bytes(cmd))
self.send_command(cmd)

def get(self, command, prompt=None, answer=None, sendonly=False):
return self.send_command(command, prompt=prompt, answer=answer, sendonly=sendonly)
Expand All @@ -66,10 +66,10 @@ def commit(self, comment=None):
command = 'commit comment "{0}"'.format(comment)
else:
command = 'commit'
self.send_command(to_bytes(command))
self.send_command(command)

def discard_changes(self, *args, **kwargs):
self.send_command(b'exit discard')
self.send_command('exit discard')

def get_capabilities(self):
result = {}
Expand Down
2 changes: 0 additions & 2 deletions test/sanity/pylint/ignore.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ lib/ansible/modules/storage/infinidat/infini_vol.py ansible-format-automatic-spe
lib/ansible/modules/storage/purestorage/purefa_host.py ansible-format-automatic-specification
lib/ansible/modules/storage/purestorage/purefa_pg.py ansible-format-automatic-specification
lib/ansible/modules/system/firewalld.py ansible-format-automatic-specification
lib/ansible/plugins/cliconf/junos.py ansible-no-format-on-bytestring 3
lib/ansible/plugins/cliconf/nxos.py ansible-format-automatic-specification
test/runner/injector/importer.py missing-docstring 3.7
test/runner/injector/injector.py missing-docstring 3.7
test/runner/lib/ansible_util.py missing-docstring 3.7
Expand Down