From c7ec1490b0d0e6be9072a9c6741caa2643e5457e Mon Sep 17 00:00:00 2001 From: Quique Llorente Date: Wed, 30 Jan 2019 13:28:36 +0100 Subject: [PATCH] Mark as unsafe commit message at inventory If you run zuul at a commit with some jinja2 stuff in the comment it fails, to bypass this this review tag the inventory yaml zuul message with !unsafe ansible yaml tag [1]. Closes-Bug: https://storyboard.openstack.org/#!/story/2004896 [1] https://docs.ansible.com/ansible/latest/user_guide/playbooks_advanced_syntax.html#unsafe-or-raw-strings Change-Id: Ic11c253cf23cc4d1fb80993f5722f37e4c22f6df --- .../playbooks/jinja2-message.yaml | 6 +++ .../inventory/git/common-config/zuul.yaml | 10 ++++ .../inventory/git/org_project2/.zuul.yaml | 6 +++ .../config/inventory/git/org_project2/README | 1 + tests/fixtures/config/inventory/main.yaml | 1 + tests/unit/test_inventory.py | 48 ++++++++++++++++++- zuul/executor/server.py | 6 ++- zuul/lib/yamlutil.py | 25 ++++++++++ 8 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/config/inventory/git/common-config/playbooks/jinja2-message.yaml create mode 100644 tests/fixtures/config/inventory/git/org_project2/.zuul.yaml create mode 100644 tests/fixtures/config/inventory/git/org_project2/README diff --git a/tests/fixtures/config/inventory/git/common-config/playbooks/jinja2-message.yaml b/tests/fixtures/config/inventory/git/common-config/playbooks/jinja2-message.yaml new file mode 100644 index 0000000000..fbfb79138f --- /dev/null +++ b/tests/fixtures/config/inventory/git/common-config/playbooks/jinja2-message.yaml @@ -0,0 +1,6 @@ +- hosts: all + tasks: + - name: Dump commit message + copy: + content: "{{ zuul.message }}" + dest: "{{ zuul.executor.log_root }}/commit-message.txt" diff --git a/tests/fixtures/config/inventory/git/common-config/zuul.yaml b/tests/fixtures/config/inventory/git/common-config/zuul.yaml index f592eb48b8..22eb693df0 100644 --- a/tests/fixtures/config/inventory/git/common-config/zuul.yaml +++ b/tests/fixtures/config/inventory/git/common-config/zuul.yaml @@ -74,3 +74,13 @@ name: hostvars-inventory run: playbooks/hostvars-inventory.yaml nodeset: nodeset2 + +- job: + name: jinja2-message + nodeset: + nodes: + - name: ubuntu-xenial + label: ubuntu-xenial + run: playbooks/jinja2-message.yaml + + diff --git a/tests/fixtures/config/inventory/git/org_project2/.zuul.yaml b/tests/fixtures/config/inventory/git/org_project2/.zuul.yaml new file mode 100644 index 0000000000..759e0abbbc --- /dev/null +++ b/tests/fixtures/config/inventory/git/org_project2/.zuul.yaml @@ -0,0 +1,6 @@ +- project: + check: + jobs: + - jinja2-message + + diff --git a/tests/fixtures/config/inventory/git/org_project2/README b/tests/fixtures/config/inventory/git/org_project2/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/inventory/git/org_project2/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/inventory/main.yaml b/tests/fixtures/config/inventory/main.yaml index 208e274b13..14b382fe08 100644 --- a/tests/fixtures/config/inventory/main.yaml +++ b/tests/fixtures/config/inventory/main.yaml @@ -6,3 +6,4 @@ - common-config untrusted-projects: - org/project + - org/project2 diff --git a/tests/unit/test_inventory.py b/tests/unit/test_inventory.py index 4d61b394cc..9d26d2ca98 100644 --- a/tests/unit/test_inventory.py +++ b/tests/unit/test_inventory.py @@ -16,6 +16,7 @@ import os import yaml +from tests.base import AnsibleZuulTestCase from tests.base import ZuulTestCase @@ -60,7 +61,7 @@ class TestInventory(TestInventoryBase): self.assertIn('src_root', z_vars['executor']) self.assertIn('job', z_vars) self.assertEqual(z_vars['job'], 'single-inventory') - self.assertEqual(z_vars['message'], 'A') + self.assertEqual(str(z_vars['message']), 'A') self.executor_server.release() self.waitUntilSettled() @@ -163,10 +164,55 @@ class TestInventory(TestInventoryBase): self.waitUntilSettled() +class TestAnsibleInventory(AnsibleZuulTestCase): + + tenant_config_file = 'config/inventory/main.yaml' + + def _get_file(self, build, path): + p = os.path.join(build.jobdir.root, path) + with open(p) as f: + return f.read() + + def _jinja2_message(self, expected_message): + + # This test runs a bit long and needs extra time. + self.wait_timeout = 120 + # Keep the jobdir around to check inventory + self.executor_server.keep_jobdir = True + # Output extra ansible info so we might see errors. + self.executor_server.verbose = True + A = self.fake_gerrit.addFakeChange( + 'org/project2', 'master', expected_message) + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='jinja2-message', result='SUCCESS', changes='1,1')]) + + build = self.history[0] + inv_path = os.path.join(build.jobdir.root, 'ansible', 'inventory.yaml') + inventory = yaml.safe_load(open(inv_path, 'r')) + + self.assertEqual( + inventory['all']['vars']['zuul']['message'].unsafe_var, + expected_message) + + obtained_message = self._get_file(self.history[0], + 'work/logs/commit-message.txt') + + self.assertEqual(obtained_message, expected_message) + + def test_jinja2_message_brackets(self): + self._jinja2_message("This message has {{ jinja2 }} in it ") + + def test_jinja2_message_raw(self): + self._jinja2_message("This message has {% raw %} in {% endraw %} it ") + + class TestWindowsInventory(TestInventoryBase): config_file = 'zuul-winrm.conf' def test_windows_inventory(self): + inventory = self._get_build_inventory('hostvars-inventory') windows_host = inventory['all']['hosts']['windows'] self.assertEqual(windows_host['ansible_connection'], 'winrm') diff --git a/zuul/executor/server.py b/zuul/executor/server.py index d8a7bd9e47..184028dffa 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -29,7 +29,7 @@ import traceback import git from urllib.parse import urlsplit -from zuul.lib.yamlutil import yaml +from zuul.lib.yamlutil import yaml, UnsafeTag from zuul.lib.config import get_default from zuul.lib.statsd import get_statsd from zuul.lib import filecomments @@ -591,6 +591,10 @@ def make_inventory_dict(nodes, args, all_vars): for node in nodes: hosts[node['name']] = node['host_vars'] + zuul_vars = all_vars['zuul'] + if 'message' in zuul_vars: + zuul_vars['message'] = UnsafeTag(zuul_vars['message']) + inventory = { 'all': { 'hosts': hosts, diff --git a/zuul/lib/yamlutil.py b/zuul/lib/yamlutil.py index 2c84b06ae5..c1d977bd84 100644 --- a/zuul/lib/yamlutil.py +++ b/zuul/lib/yamlutil.py @@ -25,6 +25,31 @@ except ImportError: Mark = yaml.Mark +class UnsafeTag(yaml.YAMLObject): + yaml_tag = u'!unsafe' + + def __init__(self, unsafe_var): + self.unsafe_var = unsafe_var + + @classmethod + def from_yaml(cls, loader, node): + return UnsafeTag(node.value) + + @classmethod + def to_yaml(cls, dumper, data): + return dumper.represent_scalar(cls.yaml_tag, data.unsafe_var) + + def __str__(self): + return self.unsafe_var + + +# Just calling SafeLoader and SafeDumper without yaml module prefix +# does not work and cyaml is using yaml.SafeConstructor yaml.SafeRepresenter +# underneath so this just fine for both +yaml.SafeLoader.add_constructor(UnsafeTag.yaml_tag, UnsafeTag.from_yaml) +yaml.SafeDumper.add_multi_representer(UnsafeTag, UnsafeTag.to_yaml) + + def safe_load(stream, *args, **kwargs): return yaml.load(stream, *args, Loader=SafeLoader, **kwargs)