Add error information to config-errors API endpoint
This is the first in a series of changes to improve the usability of the web view of config errors. The end goal is to be able to display them in a more structured manner. A secondary goal is to eventually add warnings (eg, deprecation warnings) which is really only feasible if we have structured presentation of errors. This change does the following: * Adds severity and error names to existing configuration errors * And makes them available via the config-errors API endpoint * Reduces the call sites for the error accumulator (LoadingErrors.addError) * Unifies the calling convention for the accumulator (we stop passing in Exception objects) Change-Id: Ia17dd3e7ad8cdfa8a07bb03b871078415d0c145e
This commit is contained in:
James E. Blair
parent
3489d22290
commit
84e0e76e2f
|
|
@ -79,6 +79,9 @@ class ConfigurationSyntaxError(Exception):
|
|||
|
||||
|
||||
class NodeFromGroupNotFoundError(Exception):
|
||||
zuul_error_name = 'Node From Group Not Found'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, nodeset, node, group):
|
||||
message = textwrap.dedent("""\
|
||||
In {nodeset} the group "{group}" contains a
|
||||
|
|
@ -89,6 +92,9 @@ class NodeFromGroupNotFoundError(Exception):
|
|||
|
||||
|
||||
class DuplicateNodeError(Exception):
|
||||
zuul_error_name = 'Duplicate Node'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, nodeset, node):
|
||||
message = textwrap.dedent("""\
|
||||
In nodeset "{nodeset}" the node "{node}" appears multiple times.
|
||||
|
|
@ -99,6 +105,9 @@ class DuplicateNodeError(Exception):
|
|||
|
||||
|
||||
class UnknownConnection(Exception):
|
||||
zuul_error_name = 'Unknown Connection'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, connection_name):
|
||||
message = textwrap.dedent("""\
|
||||
Unknown connection named "{connection}".""")
|
||||
|
|
@ -107,6 +116,9 @@ class UnknownConnection(Exception):
|
|||
|
||||
|
||||
class LabelForbiddenError(Exception):
|
||||
zuul_error_name = 'Label Forbidden'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, label, allowed_labels, disallowed_labels):
|
||||
message = textwrap.dedent("""\
|
||||
Label named "{label}" is not part of the allowed
|
||||
|
|
@ -126,6 +138,9 @@ class LabelForbiddenError(Exception):
|
|||
|
||||
|
||||
class MaxTimeoutError(Exception):
|
||||
zuul_error_name = 'Max Timeout Exceeded'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, job, tenant):
|
||||
message = textwrap.dedent("""\
|
||||
The job "{job}" exceeds tenant max-job-timeout {maxtimeout}.""")
|
||||
|
|
@ -135,6 +150,9 @@ class MaxTimeoutError(Exception):
|
|||
|
||||
|
||||
class DuplicateGroupError(Exception):
|
||||
zuul_error_name = 'Duplicate Nodeset Group'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, nodeset, group):
|
||||
message = textwrap.dedent("""\
|
||||
In {nodeset} the group "{group}" appears multiple times.
|
||||
|
|
@ -145,6 +163,9 @@ class DuplicateGroupError(Exception):
|
|||
|
||||
|
||||
class ProjectNotFoundError(Exception):
|
||||
zuul_error_name = 'Project Not Found'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, project):
|
||||
message = textwrap.dedent("""\
|
||||
The project "{project}" was not found. All projects
|
||||
|
|
@ -156,6 +177,9 @@ class ProjectNotFoundError(Exception):
|
|||
|
||||
|
||||
class TemplateNotFoundError(Exception):
|
||||
zuul_error_name = 'Template Not Found'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, template):
|
||||
message = textwrap.dedent("""\
|
||||
The project template "{template}" was not found.
|
||||
|
|
@ -165,6 +189,9 @@ class TemplateNotFoundError(Exception):
|
|||
|
||||
|
||||
class NodesetNotFoundError(Exception):
|
||||
zuul_error_name = 'Nodeset Not Found'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, nodeset):
|
||||
message = textwrap.dedent("""\
|
||||
The nodeset "{nodeset}" was not found.
|
||||
|
|
@ -174,6 +201,9 @@ class NodesetNotFoundError(Exception):
|
|||
|
||||
|
||||
class PipelineNotPermittedError(Exception):
|
||||
zuul_error_name = 'Pipeline Forbidden'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self):
|
||||
message = textwrap.dedent("""\
|
||||
Pipelines may not be defined in untrusted repos,
|
||||
|
|
@ -183,6 +213,9 @@ class PipelineNotPermittedError(Exception):
|
|||
|
||||
|
||||
class ProjectNotPermittedError(Exception):
|
||||
zuul_error_name = 'Project Forbidden'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self):
|
||||
message = textwrap.dedent("""\
|
||||
Within an untrusted project, the only project definition
|
||||
|
|
@ -192,6 +225,9 @@ class ProjectNotPermittedError(Exception):
|
|||
|
||||
|
||||
class GlobalSemaphoreNotFoundError(Exception):
|
||||
zuul_error_name = 'Global Semaphore Not Found'
|
||||
zuul_error_severity = model.SEVERITY_ERROR
|
||||
|
||||
def __init__(self, semaphore):
|
||||
message = textwrap.dedent("""\
|
||||
The global semaphore "{semaphore}" was not found. All
|
||||
|
|
@ -261,15 +297,18 @@ def project_configuration_exceptions(context, accumulator):
|
|||
|
||||
m = m.format(intro=intro,
|
||||
error=indent(str(e)))
|
||||
accumulator.addError(context, None, m)
|
||||
accumulator.addError(
|
||||
context, None, m,
|
||||
short_error=str(e),
|
||||
severity=getattr(e, 'zuul_error_severity', model.SEVERITY_ERROR),
|
||||
name=getattr(e, 'zuul_error_name', 'Unknown'))
|
||||
|
||||
|
||||
@contextmanager
|
||||
def early_configuration_exceptions(context):
|
||||
def early_configuration_exceptions(context, accumulator):
|
||||
try:
|
||||
yield
|
||||
except ConfigurationSyntaxError:
|
||||
raise
|
||||
# Note: we catch ConfigurationSyntaxErrors here.
|
||||
except Exception as e:
|
||||
intro = textwrap.fill(textwrap.dedent("""\
|
||||
Zuul encountered a syntax error while parsing its configuration in the
|
||||
|
|
@ -285,7 +324,11 @@ def early_configuration_exceptions(context):
|
|||
|
||||
m = m.format(intro=intro,
|
||||
error=indent(str(e)))
|
||||
raise ConfigurationSyntaxError(m)
|
||||
accumulator.addError(
|
||||
context, None, m,
|
||||
short_error=str(e),
|
||||
severity=getattr(e, 'zuul_error_severity', model.SEVERITY_ERROR),
|
||||
name=getattr(e, 'zuul_error_name', 'Unknown'))
|
||||
|
||||
|
||||
@contextmanager
|
||||
|
|
@ -322,7 +365,11 @@ def configuration_exceptions(stanza, conf, accumulator):
|
|||
content=indent(start_mark.snippet.rstrip()),
|
||||
start_mark=str(start_mark))
|
||||
|
||||
accumulator.addError(context, start_mark, m, str(e))
|
||||
accumulator.addError(
|
||||
context, start_mark, m,
|
||||
short_error=str(e),
|
||||
severity=getattr(e, 'zuul_error_severity', model.SEVERITY_ERROR),
|
||||
name=getattr(e, 'zuul_error_name', 'Unknown'))
|
||||
|
||||
|
||||
@contextmanager
|
||||
|
|
@ -358,7 +405,11 @@ def reference_exceptions(stanza, obj, accumulator):
|
|||
content=indent(start_mark.snippet.rstrip()),
|
||||
start_mark=str(start_mark))
|
||||
|
||||
accumulator.addError(context, start_mark, m, str(e))
|
||||
accumulator.addError(
|
||||
context, start_mark, m,
|
||||
short_error=str(e),
|
||||
severity=getattr(e, 'zuul_error_severity', model.SEVERITY_ERROR),
|
||||
name=getattr(e, 'zuul_error_name', 'Unknown'))
|
||||
|
||||
|
||||
class ZuulSafeLoader(yaml.EncryptedLoader):
|
||||
|
|
@ -2359,12 +2410,9 @@ class TenantParser(object):
|
|||
|
||||
def loadProjectYAML(self, data, source_context, loading_errors):
|
||||
config = model.UnparsedConfig()
|
||||
try:
|
||||
with early_configuration_exceptions(source_context):
|
||||
r = safe_load_yaml(data, source_context)
|
||||
config.extend(r)
|
||||
except ConfigurationSyntaxError as e:
|
||||
loading_errors.addError(source_context, None, e)
|
||||
with early_configuration_exceptions(source_context, loading_errors):
|
||||
r = safe_load_yaml(data, source_context)
|
||||
config.extend(r)
|
||||
return config
|
||||
|
||||
def filterConfigProjectYAML(self, data):
|
||||
|
|
@ -2389,12 +2437,9 @@ class TenantParser(object):
|
|||
# Handle pragma items first since they modify the source context
|
||||
# used by other classes.
|
||||
for config_pragma in unparsed_config.pragmas:
|
||||
try:
|
||||
with configuration_exceptions('pragma',
|
||||
config_pragma, loading_errors):
|
||||
pcontext.pragma_parser.fromYaml(config_pragma)
|
||||
except ConfigurationSyntaxError as e:
|
||||
loading_errors.addError(
|
||||
config_pragma['_source_context'],
|
||||
config_pragma['_start_mark'], e)
|
||||
|
||||
for config_pipeline in unparsed_config.pipelines:
|
||||
classes = self._getLoadClasses(tenant, config_pipeline)
|
||||
|
|
|
|||
|
|
@ -112,6 +112,10 @@ SCHEME_GOLANG = 'golang'
|
|||
SCHEME_FLAT = 'flat'
|
||||
SCHEME_UNIQUE = 'unique'
|
||||
|
||||
# Error severity
|
||||
SEVERITY_ERROR = 'error'
|
||||
SEVERITY_WARNING = 'warning'
|
||||
|
||||
|
||||
def add_debug_line(debug_messages, msg, indent=0):
|
||||
if debug_messages is None:
|
||||
|
|
@ -180,6 +184,9 @@ class ConfigurationErrorKey(object):
|
|||
sufficient to determine whether we should show an error to a user.
|
||||
"""
|
||||
|
||||
# Note: this class is serialized to ZK via ConfigurationErrorList,
|
||||
# ensure that it serializes and deserializes appropriately.
|
||||
|
||||
def __init__(self, context, mark, error_text):
|
||||
self.context = context
|
||||
self.mark = mark
|
||||
|
|
@ -240,23 +247,34 @@ class ConfigurationErrorKey(object):
|
|||
|
||||
|
||||
class ConfigurationError(object):
|
||||
|
||||
"""A configuration error"""
|
||||
def __init__(self, context, mark, error, short_error=None):
|
||||
self.error = str(error)
|
||||
|
||||
# Note: this class is serialized to ZK via ConfigurationErrorList,
|
||||
# ensure that it serializes and deserializes appropriately.
|
||||
|
||||
def __init__(self, context, mark, error, short_error=None,
|
||||
severity=None, name=None):
|
||||
self.error = error
|
||||
self.short_error = short_error
|
||||
self.severity = severity or SEVERITY_ERROR
|
||||
self.name = name or 'Unknown'
|
||||
self.key = ConfigurationErrorKey(context, mark, self.error)
|
||||
|
||||
def serialize(self):
|
||||
return {
|
||||
"error": self.error,
|
||||
"short_error": self.short_error,
|
||||
"key": self.key.serialize()
|
||||
"key": self.key.serialize(),
|
||||
"severity": self.severity,
|
||||
"name": self.name,
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def deserialize(cls, data):
|
||||
data["key"] = ConfigurationErrorKey.deserialize(data["key"])
|
||||
# These attributes were added in MODEL_API 13
|
||||
data['severity'] = data.get('severity', SEVERITY_ERROR)
|
||||
data['name'] = data.get('name', 'Unknown')
|
||||
o = cls.__new__(cls)
|
||||
o.__dict__.update(data)
|
||||
return o
|
||||
|
|
@ -269,7 +287,9 @@ class ConfigurationError(object):
|
|||
return False
|
||||
return (self.error == other.error and
|
||||
self.short_error == other.short_error and
|
||||
self.key == other.key)
|
||||
self.key == other.key and
|
||||
self.severity == other.severity and
|
||||
self.name == other.name)
|
||||
|
||||
|
||||
class ConfigurationErrorList(zkobject.ShardedZKObject):
|
||||
|
|
@ -306,8 +326,12 @@ class LoadingErrors(object):
|
|||
self.errors = []
|
||||
self.error_keys = set()
|
||||
|
||||
def addError(self, context, mark, error, short_error=None):
|
||||
e = ConfigurationError(context, mark, error, short_error)
|
||||
def addError(self, context, mark, error, short_error=None,
|
||||
severity=None, name=None):
|
||||
e = ConfigurationError(context, mark, error,
|
||||
short_error=short_error,
|
||||
severity=severity,
|
||||
name=name)
|
||||
self.errors.append(e)
|
||||
self.error_keys.add(e.key)
|
||||
|
||||
|
|
|
|||
|
|
@ -1201,8 +1201,13 @@ class ZuulWebAPI(object):
|
|||
@cherrypy.tools.check_tenant_auth()
|
||||
def config_errors(self, tenant_name, tenant, auth):
|
||||
ret = [
|
||||
{'source_context': e.key.context.toDict(),
|
||||
'error': e.error}
|
||||
{
|
||||
'source_context': e.key.context.toDict(),
|
||||
'error': e.error,
|
||||
'short_error': e.short_error,
|
||||
'severity': e.severity,
|
||||
'name': e.name,
|
||||
}
|
||||
for e in tenant.layout.loading_errors.errors
|
||||
]
|
||||
return ret
|
||||
|
|
|
|||
Loading…
Reference in New Issue