zuul/zuul
Code Issues Proposed changes

ConfigItem*Exception : add failure context

Browse Source 
These various exceptions are raised as certain parts of the
configuration are parsed.  Although they give you a lot of information
about the issue, they don't give you any context for what part of your
configuration caused the error to be raised.

They are all working on a particular item of the configuration that I
think it would be very useful to show in the error output.

This was inspired by me putting playbooks in the .zuul.d directory,
where they were read as config files.  In hindsight it was obvious,
but it took me a while to figure out what was going on.  With this it
would have reported something like:

  Zuul encountered a syntax error while parsing its configuration in
  the repo org/project1 on branch master.  The error was:

  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
  ... blah ...

  The incorrect values are:

  hosts: all
  tasks:
  - debug:
      msg: blah
    name: foo

which would have clued me in immediately.

This truncates the message if it is too long; which might be the case
if dumped a playbook or foreign config file.  However I think
practically most errors are typo level on jobs/templates/etc. that
will show nicely.

The YAML version is updated to support non-sorted dumping of the keys.
This is particularly helpful with Python 3.7+ and default ordered
dicts.

Change-Id: I4851a5a796fa452a023e0e6193fc724ae0967a44
This commit is contained in:
Ian Wienand
2020-07-31 10:57:49 +10:00
parent e64fd8fff8
commit c8d93c955c
3 changed files with 132 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
requirements.txt
View File

@@ -6,7 +6,7 @@ virtualenv!=20.0.0,!=20.0.1,>20
python-dateutil
github3.py>=1.1.0
PyYAML>=3.1.0
PyYAML>=5.1.0
paramiko>=2.0.1
GitPython>=2.1.8
python-daemon>=2.0.4

46
tests/unit/test_v3.py
View File

@@ -16,9 +16,10 @@ import io
import json
import logging
import os
import sys
import textwrap
import gc
from unittest import skip
from unittest import skip, skipIf
import paramiko
@@ -1457,11 +1458,13 @@ class TestInRepoConfig(ZuulTestCase):
"A should report failure")
self.assertIn('not a list', A.messages[0],
"A should have a syntax error reported")
self.assertIn('job: foo', A.messages[0],
"A should display the failing list")
def test_yaml_dict_error(self):
in_repo_conf = textwrap.dedent(
"""
- job
- job_not_a_dict
""")
file_dict = {'.zuul.yaml': in_repo_conf}
@@ -1476,6 +1479,8 @@ class TestInRepoConfig(ZuulTestCase):
"A should report failure")
self.assertIn('not a dictionary', A.messages[0],
"A should have a syntax error reported")
self.assertIn('job_not_a_dict', A.messages[0],
"A should list the bad key")
def test_yaml_duplicate_key_error(self):
in_repo_conf = textwrap.dedent(
@@ -1517,6 +1522,41 @@ class TestInRepoConfig(ZuulTestCase):
"A should report failure")
self.assertIn('has more than one key', A.messages[0],
"A should have a syntax error reported")
self.assertIn("job: null\n name: project-test2", A.messages[0],
"A should have the failing section displayed")
# This is non-deterministic without default dict ordering, which
# happended with python 3.7.
@skipIf(sys.version_info < (3, 7), "non-deterministic on < 3.7")
def test_yaml_error_truncation_message(self):
in_repo_conf = textwrap.dedent(
"""
- job:
name: project-test2
this: is
a: long
set: of
keys: that
should: be
truncated: ok
""")
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")
self.assertIn("job: null\n name: project-test2", A.messages[0],
"A should have the failing section displayed")
self.assertIn("...", A.messages[0],
"A should have the failing section truncated")
def test_yaml_unknown_error(self):
in_repo_conf = textwrap.dedent(
@@ -1537,6 +1577,8 @@ class TestInRepoConfig(ZuulTestCase):
"A should report failure")
self.assertIn('not recognized', A.messages[0],
"A should have a syntax error reported")
self.assertIn('foobar:\n foo: bar', A.messages[0],
"A should report the bad keys")
def test_invalid_job_secret_var_name(self):
in_repo_conf = textwrap.dedent(

151
zuul/model.py
View File

@@ -26,6 +26,7 @@ import urllib.parse
import textwrap
import types
import itertools
import yaml
import jsonpath_rw
@@ -3510,81 +3511,103 @@ class ProjectMetadata(object):
self.default_branch = 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:
# TODO(ianw) : this would clearly be better if it recorded the
# original file and made line-relative comments, however the contexts
# the subclasses are raised in don't have that info currently, so this
# is a best-effort to show you something that clues you into the
# error.
class ConfigItemErrorException(Exception):
def __init__(self, conf):
super(ConfigItemErrorException, self).__init__(
self.message + self._generate_extract(conf))
- job:
name: foo
def _generate_extract(self, conf):
context = textwrap.dedent("""\
- project:
name: bar
The incorrect values are around:
Ensure that every item starts with "- " so that it is parsed as a
YAML list.
""")
super(ConfigItemNotListError, self).__init__(message)
# Not sorting the keys makes it look closer to what is in the
# file and works best with >= Python 3.7 where dicts are
# ordered by default. If this is a foreign config file or
# something the dump might be really long; hence the
# truncation.
extract = yaml.dump(conf, sort_keys=False)
lines = extract.split('\n')
if len(lines) > 5:
lines = lines[0:4]
lines.append('...')
return context + '\n'.join(lines)
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:
class ConfigItemNotListError(ConfigItemErrorException):
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
- job:
name: foo
- project:
name: bar
- 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)
Ensure that every item starts with "- " so that it is parsed as a
YAML list.
""")
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:
class ConfigItemNotDictError(ConfigItemErrorException):
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
- job:
name: foo
- project:
name: bar
- 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)
Ensure that every item in the list is a dictionary with one
key (in this example, 'job' and 'project').
""")
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:
class ConfigItemMultipleKeysError(ConfigItemErrorException):
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
- job:
name: foo
- project:
name: bar
- 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)
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).
""")
class ConfigItemUnknownError(ConfigItemErrorException):
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').
""")
class UnparsedAbideConfig(object):
@@ -3606,13 +3629,13 @@ class UnparsedAbideConfig(object):
return
if not isinstance(conf, list):
raise ConfigItemNotListError()
raise ConfigItemNotListError(conf)
for item in conf:
if not isinstance(item, dict):
raise ConfigItemNotDictError()
raise ConfigItemNotDictError(item)
if len(item.keys()) > 1:
raise ConfigItemMultipleKeysError()
raise ConfigItemMultipleKeysError(item)
key, value = list(item.items())[0]
if key == 'tenant':
self.tenants.append(value)
@@ -3621,7 +3644,7 @@ class UnparsedAbideConfig(object):
elif key == 'admin-rule':
self.admin_rules.append(value)
else:
raise ConfigItemUnknownError()
raise ConfigItemUnknownError(item)
class UnparsedConfig(object):
@@ -3682,13 +3705,13 @@ class UnparsedConfig(object):
return
if not isinstance(conf, list):
raise ConfigItemNotListError()
raise ConfigItemNotListError(conf)
for item in conf:
if not isinstance(item, dict):
raise ConfigItemNotDictError()
raise ConfigItemNotDictError(item)
if len(item.keys()) > 1:
raise ConfigItemMultipleKeysError()
raise ConfigItemMultipleKeysError(item)
key, value = list(item.items())[0]
if key == 'project':
self.projects.append(value)
@@ -3707,7 +3730,7 @@ class UnparsedConfig(object):
elif key == 'pragma':
self.pragmas.append(value)
else:
raise ConfigItemUnknownError()
raise ConfigItemUnknownError(item)
class ParsedConfig(object):