Provide file locations of config syntax errors
Yaml parse errors come with nice pointers for the line and column
in the file where the error occurred. Provide the same for
Zuul configuration language syntax errors.
This adds a yaml.Mark object (which points to a file location) to
each top-level zuul configuration object. We could add it to every
dictionary we parse, but at the moment, that seems excessive.
This uses the same mechanism to add the already existing zuul
source context to the same objects, and retires the previous method.
The new error message looks like:
----
Zuul encountered a syntax error while parsing its configuration in the
repo org/project on branch master. The error was:
extra keys not allowed @ data['foo']
The error appears in a job stanza with the content:
{'foo': 'error', 'name': 'project-test2'}
in "org/project/.zuul.yaml@master", line 2, column 3:
- job:
^
----
Change-Id: Iea496a9dca6d4cef51854afc5d3e042b3b99411a
This commit is contained in:
James E. Blair
parent
b9c0d77c2b
commit
ec7ff30198
|
|
@ -18,6 +18,7 @@ import random
|
|||
|
||||
import fixtures
|
||||
import testtools
|
||||
import yaml
|
||||
|
||||
from zuul import model
|
||||
from zuul import configloader
|
||||
|
|
@ -32,15 +33,15 @@ class TestJob(BaseTestCase):
|
|||
self.project = model.Project('project', None)
|
||||
self.context = model.SourceContext(self.project, 'master',
|
||||
'test', True)
|
||||
self.start_mark = yaml.Mark('name', 0, 0, 0, '', 0)
|
||||
|
||||
@property
|
||||
def job(self):
|
||||
tenant = model.Tenant('tenant')
|
||||
layout = model.Layout()
|
||||
project = model.Project('project', None)
|
||||
context = model.SourceContext(project, 'master', 'test', True)
|
||||
job = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'job',
|
||||
'irrelevant-files': [
|
||||
'^docs/.*$'
|
||||
|
|
@ -143,10 +144,10 @@ class TestJob(BaseTestCase):
|
|||
layout.addPipeline(pipeline)
|
||||
queue = model.ChangeQueue(pipeline)
|
||||
project = model.Project('project', None)
|
||||
context = model.SourceContext(project, 'master', 'test', True)
|
||||
|
||||
base = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'base',
|
||||
'timeout': 30,
|
||||
'pre-run': 'base-pre',
|
||||
|
|
@ -158,7 +159,8 @@ class TestJob(BaseTestCase):
|
|||
})
|
||||
layout.addJob(base)
|
||||
python27 = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'python27',
|
||||
'parent': 'base',
|
||||
'pre-run': 'py27-pre',
|
||||
|
|
@ -171,7 +173,8 @@ class TestJob(BaseTestCase):
|
|||
})
|
||||
layout.addJob(python27)
|
||||
python27diablo = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'python27',
|
||||
'branches': [
|
||||
'stable/diablo'
|
||||
|
|
@ -188,7 +191,8 @@ class TestJob(BaseTestCase):
|
|||
layout.addJob(python27diablo)
|
||||
|
||||
python27essex = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'python27',
|
||||
'branches': [
|
||||
'stable/essex'
|
||||
|
|
@ -199,7 +203,8 @@ class TestJob(BaseTestCase):
|
|||
layout.addJob(python27essex)
|
||||
|
||||
project_config = configloader.ProjectParser.fromYaml(tenant, layout, [{
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'project',
|
||||
'gate': {
|
||||
'jobs': [
|
||||
|
|
@ -296,18 +301,18 @@ class TestJob(BaseTestCase):
|
|||
def test_job_auth_inheritance(self):
|
||||
tenant = model.Tenant('tenant')
|
||||
layout = model.Layout()
|
||||
project = model.Project('project', None)
|
||||
context = model.SourceContext(project, 'master', 'test', True)
|
||||
|
||||
base = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'base',
|
||||
'timeout': 30,
|
||||
})
|
||||
layout.addJob(base)
|
||||
pypi_upload_without_inherit = configloader.JobParser.fromYaml(
|
||||
tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'pypi-upload-without-inherit',
|
||||
'parent': 'base',
|
||||
'timeout': 40,
|
||||
|
|
@ -320,7 +325,8 @@ class TestJob(BaseTestCase):
|
|||
layout.addJob(pypi_upload_without_inherit)
|
||||
pypi_upload_with_inherit = configloader.JobParser.fromYaml(
|
||||
tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'pypi-upload-with-inherit',
|
||||
'parent': 'base',
|
||||
'timeout': 40,
|
||||
|
|
@ -334,7 +340,8 @@ class TestJob(BaseTestCase):
|
|||
layout.addJob(pypi_upload_with_inherit)
|
||||
pypi_upload_with_inherit_false = configloader.JobParser.fromYaml(
|
||||
tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'pypi-upload-with-inherit-false',
|
||||
'parent': 'base',
|
||||
'timeout': 40,
|
||||
|
|
@ -348,21 +355,24 @@ class TestJob(BaseTestCase):
|
|||
layout.addJob(pypi_upload_with_inherit_false)
|
||||
in_repo_job_without_inherit = configloader.JobParser.fromYaml(
|
||||
tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'in-repo-job-without-inherit',
|
||||
'parent': 'pypi-upload-without-inherit',
|
||||
})
|
||||
layout.addJob(in_repo_job_without_inherit)
|
||||
in_repo_job_with_inherit = configloader.JobParser.fromYaml(
|
||||
tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'in-repo-job-with-inherit',
|
||||
'parent': 'pypi-upload-with-inherit',
|
||||
})
|
||||
layout.addJob(in_repo_job_with_inherit)
|
||||
in_repo_job_with_inherit_false = configloader.JobParser.fromYaml(
|
||||
tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'in-repo-job-with-inherit-false',
|
||||
'parent': 'pypi-upload-with-inherit-false',
|
||||
})
|
||||
|
|
@ -381,24 +391,25 @@ class TestJob(BaseTestCase):
|
|||
pipeline = model.Pipeline('gate', layout)
|
||||
layout.addPipeline(pipeline)
|
||||
queue = model.ChangeQueue(pipeline)
|
||||
project = model.Project('project', None)
|
||||
context = model.SourceContext(project, 'master', 'test', True)
|
||||
|
||||
base = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'base',
|
||||
'timeout': 30,
|
||||
})
|
||||
layout.addJob(base)
|
||||
python27 = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'python27',
|
||||
'parent': 'base',
|
||||
'timeout': 40,
|
||||
})
|
||||
layout.addJob(python27)
|
||||
python27diablo = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'python27',
|
||||
'branches': [
|
||||
'stable/diablo'
|
||||
|
|
@ -408,7 +419,8 @@ class TestJob(BaseTestCase):
|
|||
layout.addJob(python27diablo)
|
||||
|
||||
project_config = configloader.ProjectParser.fromYaml(tenant, layout, [{
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'project',
|
||||
'gate': {
|
||||
'jobs': [
|
||||
|
|
@ -418,7 +430,7 @@ class TestJob(BaseTestCase):
|
|||
}])
|
||||
layout.addProjectConfig(project_config)
|
||||
|
||||
change = model.Change(project)
|
||||
change = model.Change(self.project)
|
||||
change.branch = 'master'
|
||||
item = queue.enqueueChange(change)
|
||||
item.current_build_set.layout = layout
|
||||
|
|
@ -455,16 +467,17 @@ class TestJob(BaseTestCase):
|
|||
layout.addPipeline(pipeline)
|
||||
queue = model.ChangeQueue(pipeline)
|
||||
project = model.Project('project', None)
|
||||
context = model.SourceContext(project, 'master', 'test', True)
|
||||
|
||||
base = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'base',
|
||||
'timeout': 30,
|
||||
})
|
||||
layout.addJob(base)
|
||||
python27 = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'python27',
|
||||
'parent': 'base',
|
||||
'timeout': 40,
|
||||
|
|
@ -473,7 +486,8 @@ class TestJob(BaseTestCase):
|
|||
layout.addJob(python27)
|
||||
|
||||
project_config = configloader.ProjectParser.fromYaml(tenant, layout, [{
|
||||
'_source_context': context,
|
||||
'_source_context': self.context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'project',
|
||||
'gate': {
|
||||
'jobs': [
|
||||
|
|
@ -504,6 +518,7 @@ class TestJob(BaseTestCase):
|
|||
|
||||
base = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': base_context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'base',
|
||||
})
|
||||
layout.addJob(base)
|
||||
|
|
@ -513,6 +528,7 @@ class TestJob(BaseTestCase):
|
|||
'test', True)
|
||||
base2 = configloader.JobParser.fromYaml(tenant, layout, {
|
||||
'_source_context': other_context,
|
||||
'_start_mark': self.start_mark,
|
||||
'name': 'base',
|
||||
})
|
||||
with testtools.ExpectedException(
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import logging
|
|||
import six
|
||||
import yaml
|
||||
import pprint
|
||||
import textwrap
|
||||
|
||||
import voluptuous as vs
|
||||
|
||||
|
|
@ -44,6 +45,10 @@ class ConfigurationSyntaxError(Exception):
|
|||
pass
|
||||
|
||||
|
||||
def indent(s):
|
||||
return '\n'.join([' ' + x for x in s.split('\n')])
|
||||
|
||||
|
||||
@contextmanager
|
||||
def configuration_exceptions(stanza, conf):
|
||||
try:
|
||||
|
|
@ -51,28 +56,51 @@ def configuration_exceptions(stanza, conf):
|
|||
except vs.Invalid as e:
|
||||
conf = copy.deepcopy(conf)
|
||||
context = conf.pop('_source_context')
|
||||
m = """
|
||||
Zuul encountered a syntax error while parsing its configuration in the
|
||||
repo {repo} on branch {branch}. The error was:
|
||||
start_mark = conf.pop('_start_mark')
|
||||
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,
|
||||
)))
|
||||
|
||||
{error}
|
||||
m = textwrap.dedent("""\
|
||||
{intro}
|
||||
|
||||
The offending content was a {stanza} stanza with the content:
|
||||
{error}
|
||||
|
||||
{content}
|
||||
"""
|
||||
m = m.format(repo=context.project.name,
|
||||
branch=context.branch,
|
||||
error=str(e),
|
||||
The error appears in a {stanza} stanza with the content:
|
||||
|
||||
{content}
|
||||
|
||||
{start_mark}""")
|
||||
|
||||
m = m.format(intro=intro,
|
||||
error=indent(str(e)),
|
||||
stanza=stanza,
|
||||
content=pprint.pformat(conf))
|
||||
content=indent(pprint.pformat(conf)),
|
||||
start_mark=str(start_mark))
|
||||
raise ConfigurationSyntaxError(m)
|
||||
|
||||
|
||||
class ZuulSafeLoader(yaml.SafeLoader):
|
||||
zuul_node_types = frozenset(('job', 'nodeset', 'pipeline',
|
||||
'project', 'project-template'))
|
||||
|
||||
def __init__(self, stream, context):
|
||||
super(ZuulSafeLoader, self).__init__(stream)
|
||||
self.name = str(context)
|
||||
self.zuul_context = context
|
||||
|
||||
def construct_mapping(self, node, deep=False):
|
||||
r = super(ZuulSafeLoader, self).construct_mapping(node, deep)
|
||||
keys = frozenset(r.keys())
|
||||
if len(keys) == 1 and keys.intersection(self.zuul_node_types):
|
||||
d = r.values()[0]
|
||||
if isinstance(d, dict):
|
||||
d['_start_mark'] = node.start_mark
|
||||
d['_source_context'] = self.zuul_context
|
||||
return r
|
||||
|
||||
|
||||
def safe_load_yaml(stream, context):
|
||||
|
|
@ -104,6 +132,7 @@ class NodeSetParser(object):
|
|||
nodeset = {vs.Required('name'): str,
|
||||
vs.Required('nodes'): [node],
|
||||
'_source_context': model.SourceContext,
|
||||
'_start_mark': yaml.Mark,
|
||||
}
|
||||
|
||||
return vs.Schema(nodeset)
|
||||
|
|
@ -172,6 +201,7 @@ class JobParser(object):
|
|||
'post-run': to_list(str),
|
||||
'run': str,
|
||||
'_source_context': model.SourceContext,
|
||||
'_start_mark': yaml.Mark,
|
||||
'roles': to_list(role),
|
||||
'repos': to_list(str),
|
||||
'vars': dict,
|
||||
|
|
@ -322,6 +352,7 @@ class ProjectTemplateParser(object):
|
|||
'merge', 'merge-resolve',
|
||||
'cherry-pick'),
|
||||
'_source_context': model.SourceContext,
|
||||
'_start_mark': yaml.Mark,
|
||||
}
|
||||
|
||||
for p in layout.pipelines.values():
|
||||
|
|
@ -337,6 +368,7 @@ class ProjectTemplateParser(object):
|
|||
conf = copy.deepcopy(conf)
|
||||
project_template = model.ProjectConfig(conf['name'])
|
||||
source_context = conf['_source_context']
|
||||
start_mark = conf['_start_mark']
|
||||
for pipeline in layout.pipelines.values():
|
||||
conf_pipeline = conf.get(pipeline.name)
|
||||
if not conf_pipeline:
|
||||
|
|
@ -346,11 +378,12 @@ class ProjectTemplateParser(object):
|
|||
project_pipeline.queue_name = conf_pipeline.get('queue')
|
||||
project_pipeline.job_tree = ProjectTemplateParser._parseJobTree(
|
||||
tenant, layout, conf_pipeline.get('jobs', []),
|
||||
source_context)
|
||||
source_context, start_mark)
|
||||
return project_template
|
||||
|
||||
@staticmethod
|
||||
def _parseJobTree(tenant, layout, conf, source_context, tree=None):
|
||||
def _parseJobTree(tenant, layout, conf, source_context,
|
||||
start_mark, tree=None):
|
||||
if not tree:
|
||||
tree = model.JobTree(None)
|
||||
for conf_job in conf:
|
||||
|
|
@ -366,6 +399,7 @@ class ProjectTemplateParser(object):
|
|||
# We are overriding params, so make a new job def
|
||||
attrs['name'] = jobname
|
||||
attrs['_source_context'] = source_context
|
||||
attrs['_start_mark'] = start_mark
|
||||
subtree = tree.addJob(JobParser.fromYaml(
|
||||
tenant, layout, attrs))
|
||||
else:
|
||||
|
|
@ -376,7 +410,8 @@ class ProjectTemplateParser(object):
|
|||
if jobs:
|
||||
# This is the root of a sub tree
|
||||
ProjectTemplateParser._parseJobTree(
|
||||
tenant, layout, jobs, source_context, subtree)
|
||||
tenant, layout, jobs, source_context,
|
||||
start_mark, subtree)
|
||||
else:
|
||||
raise Exception("Job must be a string or dictionary")
|
||||
return tree
|
||||
|
|
@ -393,6 +428,7 @@ class ProjectParser(object):
|
|||
'merge-mode': vs.Any('merge', 'merge-resolve',
|
||||
'cherry-pick'),
|
||||
'_source_context': model.SourceContext,
|
||||
'_start_mark': yaml.Mark,
|
||||
}
|
||||
|
||||
for p in layout.pipelines.values():
|
||||
|
|
@ -530,6 +566,7 @@ class PipelineParser(object):
|
|||
'window-decrease-type': window_type,
|
||||
'window-decrease-factor': window_factor,
|
||||
'_source_context': model.SourceContext,
|
||||
'_start_mark': yaml.Mark,
|
||||
}
|
||||
pipeline['trigger'] = vs.Required(
|
||||
PipelineParser.getDriverSchema('trigger', connections))
|
||||
|
|
@ -795,7 +832,7 @@ class TenantParser(object):
|
|||
def _parseConfigRepoLayout(data, source_context):
|
||||
# This is the top-level configuration for a tenant.
|
||||
config = model.UnparsedTenantConfig()
|
||||
config.extend(safe_load_yaml(data, source_context), source_context)
|
||||
config.extend(safe_load_yaml(data, source_context))
|
||||
return config
|
||||
|
||||
@staticmethod
|
||||
|
|
@ -803,7 +840,7 @@ class TenantParser(object):
|
|||
# TODOv3(jeblair): this should implement some rules to protect
|
||||
# aspects of the config that should not be changed in-repo
|
||||
config = model.UnparsedTenantConfig()
|
||||
config.extend(safe_load_yaml(data, source_context), source_context)
|
||||
config.extend(safe_load_yaml(data, source_context))
|
||||
return config
|
||||
|
||||
@staticmethod
|
||||
|
|
|
|||
|
|
@ -2051,7 +2051,7 @@ class UnparsedTenantConfig(object):
|
|||
r.nodesets = copy.deepcopy(self.nodesets)
|
||||
return r
|
||||
|
||||
def extend(self, conf, source_context=None):
|
||||
def extend(self, conf):
|
||||
if isinstance(conf, UnparsedTenantConfig):
|
||||
self.pipelines.extend(conf.pipelines)
|
||||
self.jobs.extend(conf.jobs)
|
||||
|
|
@ -2066,10 +2066,6 @@ class UnparsedTenantConfig(object):
|
|||
"a list of dictionaries (when parsing %s)" %
|
||||
(conf,))
|
||||
|
||||
if source_context is None:
|
||||
raise Exception("A source context must be provided "
|
||||
"(when parsing %s)" % (conf,))
|
||||
|
||||
for item in conf:
|
||||
if not isinstance(item, dict):
|
||||
raise Exception("Configuration items must be in the form of "
|
||||
|
|
@ -2080,7 +2076,6 @@ class UnparsedTenantConfig(object):
|
|||
"a single key (when parsing %s)" %
|
||||
(conf,))
|
||||
key, value = item.items()[0]
|
||||
value['_source_context'] = source_context
|
||||
if key == 'project':
|
||||
name = value['name']
|
||||
self.projects.setdefault(name, []).append(value)
|
||||
|
|
|
|||
Loading…
Reference in New Issue