diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 8eba62327a..7e8694aeb6 100755 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -495,6 +495,84 @@ class TestInRepoConfig(ZuulTestCase): dict(name='project-test2', result='SUCCESS', changes='1,1 2,1'), ]) + def test_yaml_list_error(self): + in_repo_conf = textwrap.dedent( + """ + job: foo + """) + + file_dict = {'.zuul.yaml': in_repo_conf} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertEqual(A.reported, 1, + "A should report failure") + self.assertIn('not a list', A.messages[0], + "A should have a syntax error reported") + + def test_yaml_dict_error(self): + in_repo_conf = textwrap.dedent( + """ + - job + """) + + file_dict = {'.zuul.yaml': in_repo_conf} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertEqual(A.reported, 1, + "A should report failure") + self.assertIn('not a dictionary', A.messages[0], + "A should have a syntax error reported") + + def test_yaml_key_error(self): + in_repo_conf = textwrap.dedent( + """ + - job: + name: project-test2 + """) + + file_dict = {'.zuul.yaml': in_repo_conf} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertEqual(A.reported, 1, + "A should report failure") + self.assertIn('has more than one key', A.messages[0], + "A should have a syntax error reported") + + def test_yaml_unknown_error(self): + in_repo_conf = textwrap.dedent( + """ + - foobar: + foo: bar + """) + + file_dict = {'.zuul.yaml': in_repo_conf} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertEqual(A.reported, 1, + "A should report failure") + self.assertIn('not recognized', A.messages[0], + "A should have a syntax error reported") + def test_untrusted_syntax_error(self): in_repo_conf = textwrap.dedent( """ diff --git a/zuul/configloader.py b/zuul/configloader.py index 3b09623de8..a923fcaa30 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -119,6 +119,30 @@ def indent(s): return '\n'.join([' ' + x for x in s.split('\n')]) +@contextmanager +def early_configuration_exceptions(context): + try: + yield + except ConfigurationSyntaxError: + raise + except Exception as e: + intro = textwrap.fill(textwrap.dedent("""\ + Zuul encountered a syntax error while parsing its configuration in the + repo {repo} on branch {branch}. The error was:""".format( + repo=context.project.name, + branch=context.branch, + ))) + + m = textwrap.dedent("""\ + {intro} + + {error}""") + + m = m.format(intro=intro, + error=indent(str(e))) + raise ConfigurationSyntaxError(m) + + @contextmanager def configuration_exceptions(stanza, conf): try: @@ -1367,13 +1391,15 @@ class TenantParser(object): def _parseConfigProjectLayout(data, source_context): # This is the top-level configuration for a tenant. config = model.UnparsedTenantConfig() - config.extend(safe_load_yaml(data, source_context)) + with early_configuration_exceptions(source_context): + config.extend(safe_load_yaml(data, source_context)) return config @staticmethod def _parseUntrustedProjectLayout(data, source_context): config = model.UnparsedTenantConfig() - config.extend(safe_load_yaml(data, source_context)) + with early_configuration_exceptions(source_context): + config.extend(safe_load_yaml(data, source_context)) if config.pipelines: with configuration_exceptions('pipeline', config.pipelines[0]): raise PipelineNotPermittedError() diff --git a/zuul/model.py b/zuul/model.py index d23d56f881..1ef8d3ad66 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -21,6 +21,7 @@ import struct import time from uuid import uuid4 import urllib.parse +import textwrap MERGER_MERGE = 1 # "git merge" MERGER_MERGE_RESOLVE = 2 # "git merge -s resolve" @@ -2095,6 +2096,83 @@ class ProjectConfig(object): self.private_key_file = None +class ConfigItemNotListError(Exception): + def __init__(self): + message = textwrap.dedent("""\ + Configuration file is not a list. Each zuul.yaml configuration + file must be a list of items, for example: + + - job: + name: foo + + - project: + name: bar + + Ensure that every item starts with "- " so that it is parsed as a + YAML list. + """) + super(ConfigItemNotListError, self).__init__(message) + + +class ConfigItemNotDictError(Exception): + def __init__(self): + message = textwrap.dedent("""\ + Configuration item is not a dictionary. Each zuul.yaml + configuration file must be a list of dictionaries, for + example: + + - job: + name: foo + + - project: + name: bar + + Ensure that every item in the list is a dictionary with one + key (in this example, 'job' and 'project'). + """) + super(ConfigItemNotDictError, self).__init__(message) + + +class ConfigItemMultipleKeysError(Exception): + def __init__(self): + message = textwrap.dedent("""\ + Configuration item has more than one key. Each zuul.yaml + configuration file must be a list of dictionaries with a + single key, for example: + + - job: + name: foo + + - project: + name: bar + + Ensure that every item in the list is a dictionary with only + one key (in this example, 'job' and 'project'). This error + may be caused by insufficient indentation of the keys under + the configuration item ('name' in this example). + """) + super(ConfigItemMultipleKeysError, self).__init__(message) + + +class ConfigItemUnknownError(Exception): + def __init__(self): + message = textwrap.dedent("""\ + Configuration item not recognized. Each zuul.yaml + configuration file must be a list of dictionaries, for + example: + + - job: + name: foo + + - project: + name: bar + + The dictionary keys must match one of the configuration item + types recognized by zuul (for example, 'job' or 'project'). + """) + super(ConfigItemUnknownError, self).__init__(message) + + class UnparsedAbideConfig(object): """A collection of yaml lists that has not yet been parsed into objects. @@ -2111,25 +2189,18 @@ class UnparsedAbideConfig(object): return if not isinstance(conf, list): - raise Exception("Configuration items must be in the form of " - "a list of dictionaries (when parsing %s)" % - (conf,)) + raise ConfigItemNotListError() + for item in conf: if not isinstance(item, dict): - raise Exception("Configuration items must be in the form of " - "a list of dictionaries (when parsing %s)" % - (conf,)) + raise ConfigItemNotDictError() if len(item.keys()) > 1: - raise Exception("Configuration item dictionaries must have " - "a single key (when parsing %s)" % - (conf,)) + raise ConfigItemMultipleKeysError() key, value = list(item.items())[0] if key == 'tenant': self.tenants.append(value) else: - raise Exception("Configuration item not recognized " - "(when parsing %s)" % - (conf,)) + raise ConfigItemUnknownError() class UnparsedTenantConfig(object): @@ -2168,19 +2239,13 @@ class UnparsedTenantConfig(object): return if not isinstance(conf, list): - raise Exception("Configuration items must be in the form of " - "a list of dictionaries (when parsing %s)" % - (conf,)) + raise ConfigItemNotListError() for item in conf: if not isinstance(item, dict): - raise Exception("Configuration items must be in the form of " - "a list of dictionaries (when parsing %s)" % - (conf,)) + raise ConfigItemNotDictError() if len(item.keys()) > 1: - raise Exception("Configuration item dictionaries must have " - "a single key (when parsing %s)" % - (conf,)) + raise ConfigItemMultipleKeysError() key, value = list(item.items())[0] if key == 'project': name = value['name'] @@ -2198,9 +2263,7 @@ class UnparsedTenantConfig(object): elif key == 'semaphore': self.semaphores.append(value) else: - raise Exception("Configuration item `%s` not recognized " - "(when parsing %s)" % - (item, conf,)) + raise ConfigItemUnknownError() class Layout(object):