From 2aaab59d04268d4e6f79ca79741930fa17a4ce14 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Mon, 15 Jun 2020 17:02:55 +0530 Subject: [PATCH] nmcli: Typecast parameter values to string as required (#477) --- changelogs/fragments/58115_nmcli.yml | 2 + plugins/modules/net_tools/nmcli.py | 52 ++++++++++--------- .../plugins/modules/net_tools/test_nmcli.py | 37 ++++++------- 3 files changed, 48 insertions(+), 43 deletions(-) create mode 100644 changelogs/fragments/58115_nmcli.yml diff --git a/changelogs/fragments/58115_nmcli.yml b/changelogs/fragments/58115_nmcli.yml new file mode 100644 index 0000000000..40aac2ebdd --- /dev/null +++ b/changelogs/fragments/58115_nmcli.yml @@ -0,0 +1,2 @@ +bugfixes: +- nmcli - typecast parameters to string as required (https://github.com/ansible/ansible/issues/59095). diff --git a/plugins/modules/net_tools/nmcli.py b/plugins/modules/net_tools/nmcli.py index f0bf51ef16..5c45caef81 100644 --- a/plugins/modules/net_tools/nmcli.py +++ b/plugins/modules/net_tools/nmcli.py @@ -567,7 +567,7 @@ except (ImportError, ValueError): HAVE_NM_CLIENT = False from ansible.module_utils.basic import AnsibleModule, missing_required_lib -from ansible.module_utils._text import to_native +from ansible.module_utils._text import to_native, to_text class Nmcli(object): @@ -671,6 +671,10 @@ class Nmcli(object): self.dhcp_client_id = module.params['dhcp_client_id'] def execute_command(self, cmd, use_unsafe_shell=False, data=None): + if isinstance(cmd, list): + cmd = [to_text(item) for item in cmd] + else: + cmd = to_text(cmd) return self.module.run_command(cmd, use_unsafe_shell=use_unsafe_shell, data=data) def merge_secrets(self, proxy, config, setting_name): @@ -851,8 +855,7 @@ class Nmcli(object): cmd = [self.nmcli_bin, 'con', 'mod', self.conn_name, 'connection.master', self.master] # format for modifying team-slave interface if self.mtu is not None: - cmd.append('802-3-ethernet.mtu') - cmd.append(self.mtu) + cmd.extend(['802-3-ethernet.mtu', self.mtu]) return cmd def create_connection_bond(self): @@ -1126,7 +1129,7 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) else: - cmd.append('vlan%s' % self.vlanid) + cmd.append('vlan%s' % to_text(self.vlanid)) cmd.append('ifname') if self.ifname is not None: @@ -1134,10 +1137,10 @@ class Nmcli(object): elif self.conn_name is not None: cmd.append(self.conn_name) else: - cmd.append('vlan%s' % self.vlanid) + cmd.append('vlan%s' % to_text(self.vlanid)) params = {'dev': self.vlandev, - 'id': str(self.vlanid), + 'id': self.vlanid, 'ip4': self.ip4 or '', 'gw4': self.gw4 or '', 'ip6': self.ip6 or '', @@ -1159,10 +1162,10 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) else: - cmd.append('vlan%s' % self.vlanid) + cmd.append('vlan%s' % to_text(self.vlanid)) params = {'vlan.parent': self.vlandev, - 'vlan.id': str(self.vlanid), + 'vlan.id': self.vlanid, 'ipv4.address': self.ip4 or '', 'ipv4.gateway': self.gw4 or '', 'ipv4.dns': self.dns4 or '', @@ -1185,7 +1188,7 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) else: - cmd.append('vxlan%s' % self.vxlanid) + cmd.append('vxlan%s' % to_text(self.vxlan_id)) cmd.append('ifname') if self.ifname is not None: @@ -1193,7 +1196,7 @@ class Nmcli(object): elif self.conn_name is not None: cmd.append(self.conn_name) else: - cmd.append('vxan%s' % self.vxlanid) + cmd.append('vxan%s' % to_text(self.vxlan_id)) params = {'vxlan.id': self.vxlan_id, 'vxlan.local': self.vxlan_local, @@ -1213,7 +1216,7 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) else: - cmd.append('vxlan%s' % self.vxlanid) + cmd.append('vxlan%s' % to_text(self.vxlan_id)) params = {'vxlan.id': self.vxlan_id, 'vxlan.local': self.vxlan_local, @@ -1232,7 +1235,7 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) elif self.ip_tunnel_dev is not None: - cmd.append('ipip%s' % self.ip_tunnel_dev) + cmd.append('ipip%s' % to_text(self.ip_tunnel_dev)) cmd.append('ifname') if self.ifname is not None: @@ -1240,11 +1243,10 @@ class Nmcli(object): elif self.conn_name is not None: cmd.append(self.conn_name) else: - cmd.append('ipip%s' % self.ipip_dev) + cmd.append('ipip%s' % to_text(self.ipip_dev)) if self.ip_tunnel_dev is not None: - cmd.append('dev') - cmd.append(self.ip_tunnel_dev) + cmd.extend(['dev', self.ip_tunnel_dev]) params = {'ip-tunnel.local': self.ip_tunnel_local, 'ip-tunnel.remote': self.ip_tunnel_remote, @@ -1263,7 +1265,7 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) elif self.ip_tunnel_dev is not None: - cmd.append('ipip%s' % self.ip_tunnel_dev) + cmd.append('ipip%s' % to_text(self.ip_tunnel_dev)) params = {'ip-tunnel.local': self.ip_tunnel_local, 'ip-tunnel.remote': self.ip_tunnel_remote, @@ -1281,7 +1283,7 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) elif self.ip_tunnel_dev is not None: - cmd.append('sit%s' % self.ip_tunnel_dev) + cmd.append('sit%s' % to_text(self.ip_tunnel_dev)) cmd.append('ifname') if self.ifname is not None: @@ -1289,11 +1291,10 @@ class Nmcli(object): elif self.conn_name is not None: cmd.append(self.conn_name) else: - cmd.append('sit%s' % self.ipip_dev) + cmd.append('sit%s' % to_text(self.ipip_dev)) if self.ip_tunnel_dev is not None: - cmd.append('dev') - cmd.append(self.ip_tunnel_dev) + cmd.extend(['dev', self.ip_tunnel_dev]) params = {'ip-tunnel.local': self.ip_tunnel_local, 'ip-tunnel.remote': self.ip_tunnel_remote, @@ -1312,7 +1313,7 @@ class Nmcli(object): elif self.ifname is not None: cmd.append(self.ifname) elif self.ip_tunnel_dev is not None: - cmd.append('sit%s' % self.ip_tunnel_dev) + cmd.append('sit%s' % to_text(self.ip_tunnel_dev)) params = {'ip-tunnel.local': self.ip_tunnel_local, 'ip-tunnel.remote': self.ip_tunnel_remote, @@ -1495,10 +1496,11 @@ def main(): if nmcli.conn_name is None: nmcli.module.fail_json(msg="Please specify a name for the connection") # team-slave checks - if nmcli.type == 'team-slave' and nmcli.master is None: - nmcli.module.fail_json(msg="Please specify a name for the master") - if nmcli.type == 'team-slave' and nmcli.ifname is None: - nmcli.module.fail_json(msg="Please specify an interface name for the connection") + if nmcli.type == 'team-slave': + if nmcli.master is None: + nmcli.module.fail_json(msg="Please specify a name for the master when type is %s" % nmcli.type) + if nmcli.ifname is None: + nmcli.module.fail_json(msg="Please specify an interface name for the connection when type is %s" % nmcli.type) if nmcli.state == 'absent': if nmcli.connection_exists(): diff --git a/tests/unit/plugins/modules/net_tools/test_nmcli.py b/tests/unit/plugins/modules/net_tools/test_nmcli.py index 07d23ea7a5..52b45eb1d1 100644 --- a/tests/unit/plugins/modules/net_tools/test_nmcli.py +++ b/tests/unit/plugins/modules/net_tools/test_nmcli.py @@ -5,6 +5,7 @@ import json import pytest +from ansible.module_utils._text import to_text from ansible_collections.community.general.plugins.modules.net_tools import nmcli pytestmark = pytest.mark.usefixtures('patch_ansible_module') @@ -374,8 +375,8 @@ def test_create_bridge(mocked_generic_connection_create): assert args[0][5] == 'con-name' assert args[0][6] == 'non_existent_nw_device' - for param in ['ip4', '10.10.10.10', 'gw4', '10.10.10.1', 'bridge.max-age', 100, 'bridge.stp', 'yes']: - assert param in args[0] + for param in ['ip4', '10.10.10.10', 'gw4', '10.10.10.1', 'bridge.max-age', '100', 'bridge.stp', 'yes']: + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_BRIDGE, indirect=['patch_ansible_module']) @@ -395,8 +396,8 @@ def test_mod_bridge(mocked_generic_connection_modify): assert args[0][1] == 'con' assert args[0][2] == 'mod' assert args[0][3] == 'non_existent_nw_device' - for param in ['ipv4.address', '10.10.10.10', 'ipv4.gateway', '10.10.10.1', 'bridge.max-age', 100, 'bridge.stp', 'yes']: - assert param in args[0] + for param in ['ipv4.address', '10.10.10.10', 'ipv4.gateway', '10.10.10.1', 'bridge.max-age', '100', 'bridge.stp', 'yes']: + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_BRIDGE_SLAVE, indirect=['patch_ansible_module']) @@ -420,8 +421,8 @@ def test_create_bridge_slave(mocked_generic_connection_create): assert args[0][5] == 'con-name' assert args[0][6] == 'non_existent_nw_device' - for param in ['bridge-port.path-cost', 100]: - assert param in args[0] + for param in ['bridge-port.path-cost', '100']: + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_BRIDGE_SLAVE, indirect=['patch_ansible_module']) @@ -442,8 +443,8 @@ def test_mod_bridge_slave(mocked_generic_connection_modify): assert args[0][2] == 'mod' assert args[0][3] == 'non_existent_nw_device' - for param in ['bridge-port.path-cost', 100]: - assert param in args[0] + for param in ['bridge-port.path-cost', '100']: + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_VLAN, indirect=['patch_ansible_module']) @@ -468,7 +469,7 @@ def test_create_vlan_con(mocked_generic_connection_create): assert args[0][6] == 'non_existent_nw_device' for param in ['ip4', '10.10.10.10', 'gw4', '10.10.10.1', 'id', '10']: - assert param in args[0] + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_VLAN, indirect=['patch_ansible_module']) @@ -490,7 +491,7 @@ def test_mod_vlan_conn(mocked_generic_connection_modify): assert args[0][3] == 'non_existent_nw_device' for param in ['ipv4.address', '10.10.10.10', 'ipv4.gateway', '10.10.10.1', 'vlan.id', '10']: - assert param in args[0] + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_VXLAN, indirect=['patch_ansible_module']) @@ -514,8 +515,8 @@ def test_create_vxlan(mocked_generic_connection_create): assert args[0][6] == 'non_existent_nw_device' assert args[0][7] == 'ifname' - for param in ['vxlan.local', '192.168.225.5', 'vxlan.remote', '192.168.225.6', 'vxlan.id', 11]: - assert param in args[0] + for param in ['vxlan.local', '192.168.225.5', 'vxlan.remote', '192.168.225.6', 'vxlan.id', '11']: + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_VXLAN, indirect=['patch_ansible_module']) @@ -535,8 +536,8 @@ def test_vxlan_mod(mocked_generic_connection_modify): assert args[0][2] == 'mod' assert args[0][3] == 'non_existent_nw_device' - for param in ['vxlan.local', '192.168.225.5', 'vxlan.remote', '192.168.225.6', 'vxlan.id', 11]: - assert param in args[0] + for param in ['vxlan.local', '192.168.225.5', 'vxlan.remote', '192.168.225.6', 'vxlan.id', '11']: + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_IPIP, indirect=['patch_ansible_module']) @@ -566,7 +567,7 @@ def test_create_ipip(mocked_generic_connection_create): assert args[0][12] == 'non_existent_ipip_device' for param in ['ip-tunnel.local', '192.168.225.5', 'ip-tunnel.remote', '192.168.225.6']: - assert param in args[0] + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_IPIP, indirect=['patch_ansible_module']) @@ -587,7 +588,7 @@ def test_ipip_mod(mocked_generic_connection_modify): assert args[0][3] == 'non_existent_nw_device' for param in ['ip-tunnel.local', '192.168.225.5', 'ip-tunnel.remote', '192.168.225.6']: - assert param in args[0] + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_SIT, indirect=['patch_ansible_module']) @@ -617,7 +618,7 @@ def test_create_sit(mocked_generic_connection_create): assert args[0][12] == 'non_existent_sit_device' for param in ['ip-tunnel.local', '192.168.225.5', 'ip-tunnel.remote', '192.168.225.6']: - assert param in args[0] + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_SIT, indirect=['patch_ansible_module']) @@ -638,7 +639,7 @@ def test_sit_mod(mocked_generic_connection_modify): assert args[0][3] == 'non_existent_nw_device' for param in ['ip-tunnel.local', '192.168.225.5', 'ip-tunnel.remote', '192.168.225.6']: - assert param in args[0] + assert param in map(to_text, args[0]) @pytest.mark.parametrize('patch_ansible_module', TESTCASE_ETHERNET_DHCP, indirect=['patch_ansible_module'])