From 196d28e7668dd600b3f8b503fcc57d6b7748f43a Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Mon, 19 Nov 2018 17:49:24 +0000 Subject: [PATCH] Quote and escape extra vars passed to ansible Arguments passed as extra variables to Ansible require double escaping. This can be seen when running the following command: kayobe physical network configure --interface-description-limit 'interface 42' This gets passed to ansible-playbook as: ['-e', 'physical_network_interface_description_limit=interface 42'] Ansible for some reason loses the 42 from the description. This can be worked around via quoting: kayobe physical network configure --interface-description-limit '"interface 42"' Which results in: ['-e', 'physical_network_interface_description_limit="interface 42"'] This change adds quoting and escaping of variables passed to ansible-playbook in this way. Note that this change does not modify any extra variables passed to kayobe via the -e argument, instead keeping the behaviour of this argument the same as that of the Ansible -e argument. Change-Id: I51d5112b8f94998f948fe5b8cb9a927ee7ed8cec Story: 2004379 Task: 27993 --- kayobe/ansible.py | 4 ++++ kayobe/kolla_ansible.py | 4 ++++ kayobe/tests/unit/test_ansible.py | 2 +- kayobe/tests/unit/test_kolla_ansible.py | 2 +- kayobe/tests/unit/test_utils.py | 13 +++++++++++++ kayobe/utils.py | 16 ++++++++++++++++ .../quote-escape-vars-3d42c7de8dc39d15.yaml | 6 ++++++ 7 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/quote-escape-vars-3d42c7de8dc39d15.yaml diff --git a/kayobe/ansible.py b/kayobe/ansible.py index 8655272b9..48dfcdd7b 100644 --- a/kayobe/ansible.py +++ b/kayobe/ansible.py @@ -133,9 +133,13 @@ def build_args(parsed_args, playbooks, cmd += ["-e", "@%s" % vars_file] if parsed_args.extra_vars: for extra_var in parsed_args.extra_vars: + # Don't quote or escape variables passed via the kayobe -e CLI + # argument, to match Ansible's behaviour. cmd += ["-e", extra_var] if extra_vars: for extra_var_name, extra_var_value in extra_vars.items(): + # Quote and escape variables originating within the python CLI. + extra_var_value = utils.quote_and_escape(extra_var_value) cmd += ["-e", "%s=%s" % (extra_var_name, extra_var_value)] if parsed_args.become: cmd += ["--become"] diff --git a/kayobe/kolla_ansible.py b/kayobe/kolla_ansible.py index 4aa9c94d3..0b236a879 100644 --- a/kayobe/kolla_ansible.py +++ b/kayobe/kolla_ansible.py @@ -117,9 +117,13 @@ def build_args(parsed_args, command, inventory_filename, extra_vars=None, os.path.join(parsed_args.kolla_config_path, "passwords.yml")] if parsed_args.kolla_extra_vars: for extra_var in parsed_args.kolla_extra_vars: + # Don't quote or escape variables passed via the kayobe -e CLI + # argument, to match Ansible's behaviour. cmd += ["-e", extra_var] if extra_vars: for extra_var_name, extra_var_value in extra_vars.items(): + # Quote and escape variables originating within the python CLI. + extra_var_value = utils.quote_and_escape(extra_var_value) cmd += ["-e", "%s=%s" % (extra_var_name, extra_var_value)] if parsed_args.kolla_limit or limit: limits = [l for l in [parsed_args.kolla_limit, limit] if l] diff --git a/kayobe/tests/unit/test_ansible.py b/kayobe/tests/unit/test_ansible.py index 0c1806979..56765f272 100644 --- a/kayobe/tests/unit/test_ansible.py +++ b/kayobe/tests/unit/test_ansible.py @@ -251,7 +251,7 @@ class TestCase(unittest.TestCase): "-e", "@/etc/kayobe/vars-file1.yml", "-e", "@/etc/kayobe/vars-file2.yaml", "-e", "ev_name1=ev_value1", - "-e", "ev_name2=ev_value2", + "-e", "ev_name2='ev_value2'", "--check", "--limit", "group1:host1:&group2:host2", "--tags", "tag1,tag2,tag3,tag4", diff --git a/kayobe/tests/unit/test_kolla_ansible.py b/kayobe/tests/unit/test_kolla_ansible.py index df947804f..ddc717454 100644 --- a/kayobe/tests/unit/test_kolla_ansible.py +++ b/kayobe/tests/unit/test_kolla_ansible.py @@ -190,7 +190,7 @@ class TestCase(unittest.TestCase): "-v", "--inventory", "/etc/kolla/inventory/overcloud", "-e", "ev_name1=ev_value1", - "-e", "ev_name2=ev_value2", + "-e", "ev_name2='ev_value2'", "--tags", "tag1,tag2,tag3,tag4", "--arg1", "--arg2", ] diff --git a/kayobe/tests/unit/test_utils.py b/kayobe/tests/unit/test_utils.py index 1c748955a..7c9efee3b 100644 --- a/kayobe/tests/unit/test_utils.py +++ b/kayobe/tests/unit/test_utils.py @@ -96,3 +96,16 @@ key2: value2 mock_call.side_effect = subprocess.CalledProcessError(1, "command") self.assertRaises(subprocess.CalledProcessError, utils.run_command, ["command", "to", "run"]) + + def test_quote_and_escape_no_whitespace(self): + self.assertEqual("'foo'", utils.quote_and_escape("foo")) + + def test_quote_and_escape_whitespace(self): + self.assertEqual("'foo bar'", utils.quote_and_escape("foo bar")) + + def test_quote_and_escape_whitespace_with_quotes(self): + self.assertEqual("'foo '\\''bar'\\'''", + utils.quote_and_escape("foo 'bar'")) + + def test_quote_and_escape_non_string(self): + self.assertEqual(True, utils.quote_and_escape(True)) diff --git a/kayobe/utils.py b/kayobe/utils.py index 6e41a0287..2a666d166 100644 --- a/kayobe/utils.py +++ b/kayobe/utils.py @@ -116,3 +116,19 @@ def run_command(cmd, quiet=False, check_output=False, **kwargs): return subprocess.check_output(cmd, **kwargs) else: subprocess.check_call(cmd, **kwargs) + + +def quote_and_escape(value): + """Quote and escape a string. + + Adds enclosing single quotes to the string passed, and escapes single + quotes within the string using backslashes. This is useful for passing + 'extra vars' to Ansible. Without this, Ansible only uses the part of the + string up to the first whitespace. + + :param value: the string to quote and escape. + :returns: the quoted and escaped string. + """ + if not isinstance(value, six.string_types): + return value + return "'" + value.replace("'", "'\\''") + "'" diff --git a/releasenotes/notes/quote-escape-vars-3d42c7de8dc39d15.yaml b/releasenotes/notes/quote-escape-vars-3d42c7de8dc39d15.yaml new file mode 100644 index 000000000..51c575315 --- /dev/null +++ b/releasenotes/notes/quote-escape-vars-3d42c7de8dc39d15.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where CLI arguments containing whitespace that are passed to + Ansible needed to be quoted. See `Story 2004379 + `__ for details.