zuul/zuul
Code Issues Proposed changes

Add configuration warnings

Browse Source 
This adds the ability for the configuration loader to accumulate
warning-level configuration errors.  Additionally, the
driver-specific triggers can do the same.

An initial pair of warnings is added for two recently deprecated
attributes on the gerrit trigger.

Change-Id: I61f856b3aea3e0ad3147c52c7aa724f40734d1f5
This commit is contained in:
James E. Blair 2023-06-06 14:39:13 -07:00
parent 2ad18acb66
commit 1a477dc994
12 changed files with 154 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
tests/unit/test_requirements.py
View File

@ -197,6 +197,20 @@ class TestRequirementsVote1(ZuulTestCase):
self.assertEqual(len(self.history), 1)
self.assertEqual(self.history[0].name, job)
tenant = self.scheds.first.sched.abide.tenants.get('tenant-one')
self.assertEqual(len(tenant.layout.loading_errors), 1)
self.assertEqual(tenant.layout.loading_errors[0].name,
'Gerrit require-approval Deprecation')
self.assertEqual(tenant.layout.loading_errors[0].severity,
'warning')
self.assertIn('require-approval',
tenant.layout.loading_errors[0].short_error)
self.assertIn('require-approval',
tenant.layout.loading_errors[0].error)
self.assertIsNotNone(tenant.layout.loading_errors[0].key.context)
self.assertIsNotNone(tenant.layout.loading_errors[0].key.mark)
self.assertIsNotNone(tenant.layout.loading_errors[0].key.error_text)
class TestRequirementsVote2(ZuulTestCase):
"""Requirements with a voting requirement"""
@ -256,6 +270,20 @@ class TestRequirementsVote2(ZuulTestCase):
self.assertEqual(len(self.history), 2)
self.assertEqual(self.history[1].name, job)
tenant = self.scheds.first.sched.abide.tenants.get('tenant-one')
self.assertEqual(len(tenant.layout.loading_errors), 1)
self.assertEqual(tenant.layout.loading_errors[0].name,
'Gerrit require-approval Deprecation')
self.assertEqual(tenant.layout.loading_errors[0].severity,
'warning')
self.assertIn('require-approval',
tenant.layout.loading_errors[0].short_error)
self.assertIn('require-approval',
tenant.layout.loading_errors[0].error)
self.assertIsNotNone(tenant.layout.loading_errors[0].key.context)
self.assertIsNotNone(tenant.layout.loading_errors[0].key.mark)
self.assertIsNotNone(tenant.layout.loading_errors[0].key.error_text)
class TestRequirementsState(ZuulTestCase):
"""Requirements with simple state requirement"""

87
zuul/configloader.py
View File

@ -10,6 +10,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import abc
import collections
from contextlib import contextmanager
from concurrent.futures import ThreadPoolExecutor, as_completed
@ -74,6 +75,10 @@ def check_config_path(path):
"allowed in extra-config-paths")
def indent(s):
return '\n'.join([' ' + x for x in s.split('\n')])
class ConfigurationSyntaxError(Exception):
pass
@ -271,8 +276,50 @@ class YAMLDuplicateKeyError(ConfigurationSyntaxError):
super(YAMLDuplicateKeyError, self).__init__(m)
def indent(s):
return '\n'.join([' ' + x for x in s.split('\n')])
class ConfigurationSyntaxWarning(object, metaclass=abc.ABCMeta):
zuul_error_name = 'Unknown Configuration Warning'
zuul_error_severity = model.SEVERITY_WARNING
zuul_error_message = 'Unknown Configuration Warning'
@abc.abstractmethod
def makeConfigurationError(self, stanza, conf):
"Return a ConfigurationError object"
class DeprecationWarning(ConfigurationSyntaxWarning):
def makeConfigurationError(self, stanza, conf):
context = conf.get('_source_context')
start_mark = conf.get('_start_mark')
intro = textwrap.fill(textwrap.dedent("""\
Zuul encountered a deprecated syntax while parsing its
configuration in the repo {repo} on branch {branch}. The
problem was:""".format(
repo=context.project_name,
branch=context.branch,
)))
m = textwrap.dedent("""\
{intro}
{error}
The problem appears in the following {stanza} stanza:
{content}
{start_mark}""")
m = m.format(intro=intro,
error=indent(self.zuul_error_message),
stanza=stanza,
content=indent(start_mark.snippet.rstrip()),
start_mark=str(start_mark))
return model.ConfigurationError(
context, start_mark, m,
short_error=self.zuul_error_message,
severity=self.zuul_error_severity,
name=self.zuul_error_name)
@contextmanager
@ -297,7 +344,7 @@ def project_configuration_exceptions(context, accumulator):
m = m.format(intro=intro,
error=indent(str(e)))
accumulator.addError(
accumulator.makeError(
context, None, m,
short_error=str(e),
severity=getattr(e, 'zuul_error_severity', model.SEVERITY_ERROR),
@ -324,7 +371,7 @@ def early_configuration_exceptions(context, accumulator):
m = m.format(intro=intro,
error=indent(str(e)))
accumulator.addError(
accumulator.makeError(
context, None, m,
short_error=str(e),
severity=getattr(e, 'zuul_error_severity', model.SEVERITY_ERROR),
@ -365,7 +412,7 @@ def configuration_exceptions(stanza, conf, accumulator):
content=indent(start_mark.snippet.rstrip()),
start_mark=str(start_mark))
accumulator.addError(
accumulator.makeError(
context, start_mark, m,
short_error=str(e),
severity=getattr(e, 'zuul_error_severity', model.SEVERITY_ERROR),
@ -405,13 +452,36 @@ def reference_exceptions(stanza, obj, accumulator):
content=indent(start_mark.snippet.rstrip()),
start_mark=str(start_mark))
accumulator.addError(
accumulator.makeError(
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 LocalAccumulator:
"""An error accumulator that wraps another accumulator (like
LoadingErrors) while holding local context information.
"""
def __init__(self, accumulator, stanza, conf):
self.accumulator = accumulator
self.stanza = stanza
self.conf = conf
def addError(self, error):
"""Add an error to the accumulator with the local context info.
This method currently expects that the input error object is a
subclass of ConfigurationSyntaxWarning and has an addError
method that returns a fully populated ConfigurationError.
Currently all "error" severity level errors are handled by
exception handlers, so this method doesn't expect them.
"""
e = error.makeConfigurationError(self.stanza, self.conf)
self.accumulator.addError(e)
class ZuulSafeLoader(yaml.EncryptedLoader):
zuul_node_types = frozenset(('job', 'nodeset', 'secret', 'pipeline',
'project', 'project-template',
@ -1518,6 +1588,8 @@ class PipelineParser(object):
source.getRejectFilters(reject_config))
seen_connections.add(source_name)
local_accumulator = LocalAccumulator(self.pcontext.loading_errors,
'pipeline', conf)
for connection_name, trigger_config in conf.get('trigger').items():
if self.pcontext.tenant.allowed_triggers is not None and \
connection_name not in self.pcontext.tenant.allowed_triggers:
@ -1527,7 +1599,8 @@ class PipelineParser(object):
pipeline.triggers.append(trigger)
manager.event_filters.extend(
trigger.getEventFilters(connection_name,
conf['trigger'][connection_name]))
conf['trigger'][connection_name],
local_accumulator))
seen_connections.add(connection_name)
pipeline.connections = list(seen_connections)

4
zuul/driver/__init__.py
View File

@ -129,7 +129,7 @@ class TriggerInterface(object, metaclass=abc.ABCMeta):
"""
@abc.abstractmethod
def getTrigger(self, connection, config=None):
def getTrigger(self, connection, config=None, loading_errors=None):
"""Create and return a new trigger object.
This method is required by the interface.
@ -142,6 +142,8 @@ class TriggerInterface(object, metaclass=abc.ABCMeta):
or None.
:arg dict config: The configuration information supplied along
with the trigger in the layout.
:arg dict loading_errors: A LoadingErrors instance to accumulate
errors or warnings.
:returns: A new trigger object.
:rtype: :py:class:`~zuul.trigger.BaseTrigger`

23
zuul/driver/gerrit/gerrittrigger.py
View File

@ -18,13 +18,27 @@ from zuul.trigger import BaseTrigger
from zuul.driver.gerrit.gerritmodel import GerritEventFilter
from zuul.driver.gerrit import gerritsource
from zuul.driver.util import scalar_or_list, to_list
from zuul.configloader import DeprecationWarning
class GerritRequireApprovalDeprecation(DeprecationWarning):
zuul_error_name = 'Gerrit require-approval Deprecation'
zuul_error_message = """The 'require-approval' trigger attribute
is deprecated. Use 'require' instead."""
class GerritRejectApprovalDeprecation(DeprecationWarning):
zuul_error_name = 'Gerrit reject-approval Deprecation'
zuul_error_message = """The 'reject-approval' trigger attribute
is deprecated. Use 'reject' instead."""
class GerritTrigger(BaseTrigger):
name = 'gerrit'
log = logging.getLogger("zuul.GerritTrigger")
def getEventFilters(self, connection_name, trigger_conf):
def getEventFilters(self, connection_name, trigger_conf,
error_accumulator):
efilters = []
for trigger in to_list(trigger_conf):
approvals = {}
@ -42,6 +56,13 @@ class GerritTrigger(BaseTrigger):
if not usernames:
usernames = to_list(trigger.get('username_filter'))
ignore_deletes = trigger.get('ignore-deletes', True)
if 'require-approval' in trigger:
error_accumulator.addError(
GerritRequireApprovalDeprecation())
if 'reject-approval' in trigger:
error_accumulator.addError(
GerritRejectApprovalDeprecation())
f = GerritEventFilter(
connection_name=connection_name,
trigger=self,

3
zuul/driver/git/gittrigger.py
View File

@ -23,7 +23,8 @@ class GitTrigger(BaseTrigger):
name = 'git'
log = logging.getLogger("zuul.GitTrigger")
def getEventFilters(self, connection_name, trigger_conf):
def getEventFilters(self, connection_name, trigger_conf,
error_accumulator):
efilters = []
for trigger in to_list(trigger_conf):
f = GitEventFilter(

3
zuul/driver/github/githubtrigger.py
View File

@ -35,7 +35,8 @@ class GithubTrigger(BaseTrigger):
super().__init__(driver, connection, config=config)
def getEventFilters(self, connection_name, trigger_config):
def getEventFilters(self, connection_name, trigger_config,
error_accumulator):
efilters = []
for trigger in to_list(trigger_config):
f = GithubEventFilter(

3
zuul/driver/gitlab/gitlabtrigger.py
View File

@ -23,7 +23,8 @@ class GitlabTrigger(BaseTrigger):
name = 'gitlab'
log = logging.getLogger("zuul.trigger.GitlabTrigger")
def getEventFilters(self, connection_name, trigger_config):
def getEventFilters(self, connection_name, trigger_config,
error_accumulator):
efilters = []
for trigger in to_list(trigger_config):
f = GitlabEventFilter(

3
zuul/driver/pagure/paguretrigger.py
View File

@ -23,7 +23,8 @@ class PagureTrigger(BaseTrigger):
name = 'pagure'
log = logging.getLogger("zuul.trigger.PagureTrigger")
def getEventFilters(self, connection_name, trigger_config):
def getEventFilters(self, connection_name, trigger_config,
error_accumulator):
efilters = []
for trigger in to_list(trigger_config):
f = PagureEventFilter(

3
zuul/driver/timer/timertrigger.py
View File

@ -23,7 +23,8 @@ from zuul.driver.util import to_list
class TimerTrigger(BaseTrigger):
name = 'timer'
def getEventFilters(self, connection_name, trigger_conf):
def getEventFilters(self, connection_name, trigger_conf,
error_accumulator):
efilters = []
for trigger in to_list(trigger_conf):
f = TimerEventFilter(connection_name=connection_name,

3
zuul/driver/zuul/zuultrigger.py
View File

@ -29,7 +29,8 @@ class ZuulTrigger(BaseTrigger):
self._handle_parent_change_enqueued_events = False
self._handle_project_change_merged_events = False
def getEventFilters(self, connection_name, trigger_conf):
def getEventFilters(self, connection_name, trigger_conf,
error_accumulator):
efilters = []
for trigger in to_list(trigger_conf):
f = ZuulEventFilter(

11
zuul/model.py
View File

@ -334,14 +334,17 @@ class LoadingErrors(object):
self.errors = []
self.error_keys = set()
def addError(self, context, mark, error, short_error=None,
severity=None, name=None):
def makeError(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)
self.addError(e)
def addError(self, error):
self.errors.append(error)
self.error_keys.add(error.key)
def __getitem__(self, index):
return self.errors[index]

3
zuul/trigger/__init__.py
View File

@ -26,7 +26,8 @@ class BaseTrigger(object, metaclass=abc.ABCMeta):
self.config = config or {}
@abc.abstractmethod
def getEventFilters(self, connection_name, trigger_conf):
def getEventFilters(self, connection_name, trigger_conf,
error_accumulator):
"""Return a list of EventFilter's for the scheduler to match against.
"""