From 84e0e76e2fbbfcc7869394b5c712cd3f3fe3f371 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Tue, 23 May 2023 15:55:21 -0700 Subject: [PATCH] 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 --- zuul/configloader.py | 81 ++++++++++++++++++++++++++++++++++---------- zuul/model.py | 38 +++++++++++++++++---- zuul/web/__init__.py | 9 +++-- 3 files changed, 101 insertions(+), 27 deletions(-) diff --git a/zuul/configloader.py b/zuul/configloader.py index 4fdaed1014..f569097267 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -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) diff --git a/zuul/model.py b/zuul/model.py index 7c9edcb72d..429ec706a3 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -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) diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py index 78213cf101..43862aeedc 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -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