From bd26b6c0b4c71be356efa727dc39396b41ba2b9a Mon Sep 17 00:00:00 2001 From: Graeme Davidson <9348887+graemedavidson@users.noreply.github.com> Date: Mon, 24 Feb 2020 18:45:19 +0000 Subject: [PATCH] updated single quote handling to not strip and error on odd number (#67500) Fixes #67274 --- ...-edgeos-config-single-quote-stripping.yaml | 2 ++ .../modules/network/edgeos/edgeos_config.py | 27 ++++++++++++++++--- .../edgeos/fixtures/edgeos_config_config.cfg | 8 +++--- .../edgeos/fixtures/edgeos_config_src.cfg | 2 +- .../fixtures/edgeos_config_src_brackets.cfg | 2 +- .../network/edgeos/test_edgeos_config.py | 11 ++++++++ 6 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 changelogs/fragments/67500-fix-edgeos-config-single-quote-stripping.yaml diff --git a/changelogs/fragments/67500-fix-edgeos-config-single-quote-stripping.yaml b/changelogs/fragments/67500-fix-edgeos-config-single-quote-stripping.yaml new file mode 100644 index 00000000000..ae12f4d9d3d --- /dev/null +++ b/changelogs/fragments/67500-fix-edgeos-config-single-quote-stripping.yaml @@ -0,0 +1,2 @@ +bugfixes: + - edgeos_config - fixed issue of handling single quotation marks. Now fails when unmatched (odd numbers) diff --git a/lib/ansible/modules/network/edgeos/edgeos_config.py b/lib/ansible/modules/network/edgeos/edgeos_config.py index 0a44a54ae35..84f016a7286 100644 --- a/lib/ansible/modules/network/edgeos/edgeos_config.py +++ b/lib/ansible/modules/network/edgeos/edgeos_config.py @@ -186,15 +186,34 @@ def get_candidate(module): return config_to_commands(contents) -def diff_config(commands, config): - config = [to_native(c).replace("'", '') for c in config.splitlines()] +def check_command(module, command): + """Tests against a command line to be valid otherwise raise errors + + Error on uneven single quote which breaks ansible waiting for further input. Ansible + will handle even single quote failures correctly. + + :param command: the command line from current or new config + :type command: string + :raises ValueError: + * if contains odd number of single quotes + :return: command string unchanged + :rtype: string + """ + if command.count("'") % 2 != 0: + module.fail_json(msg="Unmatched single (') quote found in command: " + command) + + return command + + +def diff_config(module, commands, config): + config = [to_native(check_command(module, c)) for c in config.splitlines()] updates = list() visited = set() delete_commands = [line for line in commands if line.startswith('delete')] for line in commands: - item = to_native(line).replace("'", '') + item = to_native(check_command(module, line)) if not item.startswith('set') and not item.startswith('delete'): raise ValueError('line must start with either `set` or `delete`') @@ -234,7 +253,7 @@ def run(module, result): candidate = get_candidate(module) # create loadable config that includes only the configuration updates - commands = diff_config(candidate, config) + commands = diff_config(module, candidate, config) result['commands'] = commands diff --git a/test/units/modules/network/edgeos/fixtures/edgeos_config_config.cfg b/test/units/modules/network/edgeos/fixtures/edgeos_config_config.cfg index ac11e35f7b6..e0e79469613 100644 --- a/test/units/modules/network/edgeos/fixtures/edgeos_config_config.cfg +++ b/test/units/modules/network/edgeos/fixtures/edgeos_config_config.cfg @@ -1,10 +1,10 @@ set system host-name 'router' set system domain-name 'acme.com' set system domain-search domain 'acme.com' -set system name-server '208.67.220.220' -set system name-server '208.67.222.222' -set interfaces ethernet eth0 address '1.2.3.4/24' +set system name-server 208.67.220.220 +set system name-server 208.67.222.222 +set interfaces ethernet eth0 address 1.2.3.4/24 set interfaces ethernet eth0 description 'Outside' -set interfaces ethernet eth1 address '10.77.88.1/24' +set interfaces ethernet eth1 address 10.77.88.1/24 set interfaces ethernet eth1 description 'Inside' set interfaces ethernet eth1 disable diff --git a/test/units/modules/network/edgeos/fixtures/edgeos_config_src.cfg b/test/units/modules/network/edgeos/fixtures/edgeos_config_src.cfg index f2b7fde4f4d..471dcb9d2b7 100644 --- a/test/units/modules/network/edgeos/fixtures/edgeos_config_src.cfg +++ b/test/units/modules/network/edgeos/fixtures/edgeos_config_src.cfg @@ -1,5 +1,5 @@ set system host-name er01 delete interfaces ethernet eth0 address -set interfaces ethernet eth1 address '10.77.88.1/24' +set interfaces ethernet eth1 address 10.77.88.1/24 set interfaces ethernet eth1 description 'Inside' set interfaces ethernet eth1 disable diff --git a/test/units/modules/network/edgeos/fixtures/edgeos_config_src_brackets.cfg b/test/units/modules/network/edgeos/fixtures/edgeos_config_src_brackets.cfg index e3c9d820e01..e2bcf1cc6ca 100644 --- a/test/units/modules/network/edgeos/fixtures/edgeos_config_src_brackets.cfg +++ b/test/units/modules/network/edgeos/fixtures/edgeos_config_src_brackets.cfg @@ -4,7 +4,7 @@ interfaces { } ethernet eth1 { address 10.77.88.1/24 - description Inside + description 'Inside' disable } } diff --git a/test/units/modules/network/edgeos/test_edgeos_config.py b/test/units/modules/network/edgeos/test_edgeos_config.py index c927aa4e7d5..dfd5771f138 100644 --- a/test/units/modules/network/edgeos/test_edgeos_config.py +++ b/test/units/modules/network/edgeos/test_edgeos_config.py @@ -92,3 +92,14 @@ class TestEdgeosConfigModule(TestEdgeosModule): 'set system interfaces ethernet eth0 description Outside'] set_module_args(dict(lines=lines, match='none')) self.execute_module(changed=True, commands=lines, sort=False) + + def test_edgeos_config_single_quote_wrapped_values(self): + lines = ["set system interfaces ethernet eth0 description 'tests single quotes'"] + set_module_args(dict(lines=lines)) + commands = ["set system interfaces ethernet eth0 description 'tests single quotes'"] + self.execute_module(changed=True, commands=commands) + + def test_edgeos_config_single_quote_wrapped_values_failure(self): + lines = ["set system interfaces ethernet eth0 description 'test's single quotes'"] + set_module_args(dict(lines=lines)) + self.execute_module(failed=True)