Move mark comparision into ZuulMark object

A ConfigurationError object makes a ConfigurationErrorKey from
<context, mark, error-string>.  The ConfigurationErrorKey assumes the
"mark" argument is a ZuulMark object that has fields such as "line"
and "snippet".

However there is a path to set config errors via
QueueItem.setConfigError(string) which makes a ConfigurationError with
<None, None, error-string>.  Consequently the "mark" object in the
ConfigurationErrorKey isn't actually a ZuulMark and the __eq__
comparision for ConfigurationErrorKey objects can fail.

This can hit in PipelineManager._findRelevantErrors() which walks the
error list and ends up doing a comparision with

  if ((err.key not in
       parent_layout.loading_errors.error_keys)

I think the result in production is that you can get a range of errors
that do not get reported.  I came to looking at this because I
accidentally added Ansible playbooks under .zuul.d and something was
clearly wrong, but I was not getting any error messages.

The solution here is to move the field matching for the ZuulMark into
an __eq__ method in the ZuulMark object itself, allowing None to be a
valid value.

Adding a file that looks like a playbook triggers this path for the
test_dynamic_split_config test, which passes with this fix.

Change-Id: If59f5556141db4bda54a61b459ffbffb935094ae
This commit is contained in:
Ian Wienand 2020-07-31 10:56:37 +10:00
parent 9c623851c4
commit e1f73ae25b
3 changed files with 13 additions and 2 deletions

View File

@ -0,0 +1,8 @@
# This file is just to induce a config error
- hosts: all
tasks:
- name: Debug
debug:
msg: 'Hello'

View File

@ -346,6 +346,10 @@ class ZuulMark(object):
column=self.column + 1,
)
def __eq__(self, other):
return (self.line == other.line and
self.snippet == other.snippet)
class ZuulSafeLoader(yaml.SafeLoader):
zuul_node_types = frozenset(('job', 'nodeset', 'secret', 'pipeline',

View File

@ -128,8 +128,7 @@ class ConfigurationErrorKey(object):
if not isinstance(other, ConfigurationErrorKey):
return False
return (self.context == other.context and
self.mark.line == other.mark.line and
self.mark.snippet == other.mark.snippet and
self.mark == other.mark and
self.error_text == other.error_text)