zuul/zuul
Code Issues Proposed changes

Add ability to mark individual job attributes as final

Browse Source 
This adds a new job configuration attribute which allows users
to mark individual job attributes as final.  For example, a user
may want to mark required-projects as final in order to ensure
that updates to add new projects are always made to a parent job.

This is similar to override control, but is implemented as a
dictionary rather than yaml tags because there is limited space
to accomodate yaml tags, and this is expected to be used less
frequently and more deliberately than override control.

The dictionary is forward-compatible with other modifiers, such
as a potential expansion to mark individual attributes as
abstract or protected.

Change-Id: Ia71fd286cf84b7bf449f6442f8734abc41734050
This commit is contained in:
James E. Blair 2024-12-10 11:08:10 -08:00
parent 380e131ed5
commit 9928a1d513
7 changed files with 536 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
doc/source/config/job.rst
View File

@ -18,7 +18,9 @@ starting with very basic jobs which describe characteristics that all
jobs on the system should have, progressing through stages of
specialization before arriving at a particular job. A job may inherit
from any other job in any project (however, if the other job is marked
as :attr:`job.final`, jobs may not inherit from it).
as :attr:`job.final`, jobs may not inherit from it, and if any of its
attributes are marked as final with :attr:`job.attribute-control`,
those attributes may not be changed).
Generally, if an attribute is set on a child job, it will override (or
completely replace) attributes on the parent. This is always true for
@ -69,7 +71,8 @@ These may have different selection criteria which indicate to Zuul
that, for instance, the job should behave differently on a different
git branch. Unlike inheritance, all job variants must be defined in
the same project. Some attributes of jobs marked :attr:`job.final`
may not be overridden.
may not be overridden. Individual attributes marked as final with
with :attr:`job.attribute-control` may not be overridden.
When Zuul decides to run a job, it performs a process known as
freezing the job. Because any number of job variants may be
@ -222,6 +225,40 @@ Here is an example of two job definitions:
choosing one of the two variants, `foo` could be marked as
``intermediate``.
.. attr:: attribute-control
Individual attributes may be set to final so that any attempt to
set them by child jobs or variants will result in an error.
This is a dictionary where each key is a job attribute; the
value is another dictionary with ``final: true`` to set the
attribute final.
For example, to set the required-projects list fo final:
.. code-block:: yaml
- job:
attribute-control:
required-projects:
final: true
The following attributes are supported:
* requires
* provides
* tags
* files
* irrelevant-files
* required-projects
* vars
* extra-vars
* host-vars
* group-vars
* include-vars
* dependencies
* failure-output
.. attr:: success-message
:default: SUCCESS

6
releasenotes/notes/final-control-be382d47879f5e70.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- |
Individual attributes of jobs may now be marked as final so that
any attempt to override that attribute will cause an error. See
:attr:`job.attribute-control`.

105
tests/fixtures/layouts/final-control.yaml vendored Normal file
View File

@ -0,0 +1,105 @@
- pipeline:
name: check
manager: independent
trigger:
gerrit:
- event: patchset-created
success:
gerrit:
Verified: 1
failure:
gerrit:
Verified: -1
- job:
name: base
parent: null
run: playbooks/base.yaml
nodeset:
nodes:
- label: ubuntu-xenial
name: controller
attribute-control:
requires: {final: true}
provides: {final: true}
tags: {final: true}
files: {final: true}
irrelevant-files: {final: true}
required-projects: {final: true}
vars: {final: true}
extra-vars: {final: true}
host-vars: {final: true}
group-vars: {final: true}
include-vars: {final: true}
dependencies: {final: true}
failure-output: {final: true}
- job:
name: final-job
- job:
name: test-requires
requires: foo
- job:
name: test-provides
provides: foo
- job:
name: test-tags
tags: ['foo']
- job:
name: test-files
files: ['foo']
- job:
name: test-irrelevant-files
irrelevant-files: ['foo']
- job:
name: test-required-projects
required-projects: ['org/project1']
- job:
name: test-vars
vars: {foo: bar}
- job:
name: test-extra-vars
extra-vars: {foo: bar}
- job:
name: test-host-vars
host-vars:
controller:
foo: bar
- job:
name: test-group-vars
group-vars:
group:
foo: bar
- job:
name: test-include-vars
include-vars: ['foo.yaml']
- job:
name: test-dependencies
dependencies: ['final-job']
- job:
name: test-failure-output
failure-output: foo
- project:
name: org/project1
check:
jobs:
- final-job
- project:
name: org/project2
check:
jobs: []

257
tests/unit/test_model.py
View File

@ -580,6 +580,15 @@ class TestJob(BaseTestCase):
def test_job_override_control_provides(self):
self._test_job_override_control_set('provides')
def test_job_override_control_include_vars(self):
self._test_job_override_control_set(
'include-vars',
job_attr='include_vars',
value_factory=lambda values: tuple(
[model.JobIncludeVars(v, 'git.example.com/project', True, True)
for v in values])
)
def test_job_override_control_dependencies(self):
self._test_job_override_control_set(
'dependencies',
@ -846,6 +855,254 @@ class TestJob(BaseTestCase):
override, override_value,
errors)
def _test_job_final_control(self, attr, job_attr,
default, default_value,
final):
# Default behavior
data = configloader.safe_load_yaml(default, self.context)
parent = self.pcontext.job_parser.fromYaml(data[0]['job'])
child = self.pcontext.job_parser.fromYaml(data[1]['job'])
job = parent.copy()
job.applyVariant(child, self.layout, None)
self.assertEqual(default_value, getattr(job, job_attr))
# Verify final attr exception
data = configloader.safe_load_yaml(final, self.context)
parent = self.pcontext.job_parser.fromYaml(data[0]['job'])
child = self.pcontext.job_parser.fromYaml(data[1]['job'])
job = parent.copy()
with testtools.ExpectedException(model.JobConfigurationError,
".* final attribute"):
job.applyVariant(child, self.layout, None)
def _test_job_final_control_set(
self, attr, job_attr=None,
default_override=False,
value_factory=lambda values: {v for v in values}):
if job_attr is None:
job_attr = attr
default = textwrap.dedent(
f"""
- job:
name: parent
{attr}: parent-{attr}
- job:
name: child
{attr}: child-{attr}
""")
inherit_value = value_factory([f'parent-{attr}', f'child-{attr}'])
override_value = value_factory([f'child-{attr}'])
if default_override:
default_value = override_value
else:
default_value = inherit_value
final = textwrap.dedent(
f"""
- job:
name: parent
{attr}: parent-{attr}
attribute-control:
{attr}:
final: true
- job:
name: child
{attr}: child-{attr}
""")
self._test_job_final_control(attr, job_attr,
default, default_value,
final)
def test_job_final_control_tags(self):
self._test_job_final_control_set('tags')
def test_job_final_control_requires(self):
self._test_job_final_control_set('requires')
def test_job_final_control_provides(self):
self._test_job_final_control_set('provides')
def test_job_final_control_dependencies(self):
self._test_job_final_control_set(
'dependencies',
default_override=True,
value_factory=lambda values:
{model.JobDependency(v) for v in values})
def test_job_final_control_failure_output(self):
self._test_job_final_control_set(
'failure-output',
job_attr='failure_output',
value_factory=lambda values: tuple(v for v in sorted(values)))
def test_job_final_control_files(self):
self._test_job_final_control_set(
'files',
job_attr='file_matcher',
default_override=True,
value_factory=lambda values: change_matcher.MatchAnyFiles(
[change_matcher.FileMatcher(ZuulRegex(v))
for v in sorted(values)]))
def test_job_final_control_irrelevant_files(self):
self._test_job_final_control_set(
'irrelevant-files',
job_attr='irrelevant_file_matcher',
default_override=True,
value_factory=lambda values: change_matcher.MatchAllFiles(
[change_matcher.FileMatcher(ZuulRegex(v))
for v in sorted(values)]))
def _test_job_final_control_dict(
self, attr, job_attr=None,
default_override=False):
if job_attr is None:
job_attr = attr
default = textwrap.dedent(
f"""
- job:
name: parent
{attr}:
parent: 1
- job:
name: child
{attr}:
child: 2
""")
final = textwrap.dedent(
f"""
- job:
name: parent
{attr}:
parent: 1
attribute-control:
{attr}:
final: true
- job:
name: child
{attr}:
child: 2
""")
inherit_value = {'parent': 1, 'child': 2}
default_value = {'child': 2}
if default_override:
default_value = default_value
else:
default_value = inherit_value
self._test_job_final_control(attr, job_attr,
default, default_value,
final)
def test_job_final_control_vars(self):
self._test_job_final_control_dict(
'vars', job_attr='variables')
def test_job_final_control_extra_vars(self):
self._test_job_final_control_dict(
'extra-vars', job_attr='extra_variables')
def _test_job_final_control_host_dict(
self, attr, job_attr=None,
default_override=False):
if job_attr is None:
job_attr = attr
default = textwrap.dedent(
f"""
- job:
name: parent
{attr}:
host:
parent: 1
- job:
name: child
{attr}:
host:
child: 2
""")
final = textwrap.dedent(
f"""
- job:
name: parent
{attr}:
host:
parent: 1
attribute-control:
{attr}:
final: true
- job:
name: child
{attr}:
host:
child: 2
""")
inherit_value = {'host': {'parent': 1, 'child': 2}}
default_value = {'host': {'child': 2}}
if default_override:
default_value = default_value
else:
default_value = inherit_value
self._test_job_final_control(attr, job_attr,
default, default_value,
final)
def test_job_final_control_host_vars(self):
self._test_job_final_control_host_dict(
'host-vars', job_attr='host_variables')
def test_job_final_control_group_vars(self):
self._test_job_final_control_host_dict(
'group-vars', job_attr='group_variables')
def test_job_final_control_required_projects(self):
parent = model.Project('parent-project', self.source)
child = model.Project('child-project', self.source)
parent_tpc = model.TenantProjectConfig(parent)
child_tpc = model.TenantProjectConfig(child)
self.tenant.addTPC(parent_tpc)
self.tenant.addTPC(child_tpc)
default = textwrap.dedent(
"""
- job:
name: parent
required-projects: parent-project
- job:
name: child
required-projects: child-project
""")
final = textwrap.dedent(
"""
- job:
name: parent
required-projects: parent-project
attribute-control:
required-projects:
final: true
- job:
name: child
required-projects: child-project
""")
default_value = {
'git.example.com/parent-project': model.JobProject(
'git.example.com/parent-project'),
'git.example.com/child-project': model.JobProject(
'git.example.com/child-project'),
}
self._test_job_final_control('required-projects',
'required_projects',
default, default_value,
final)
@mock.patch("zuul.model.zkobject.ZKObject._save")
def test_image_permissions(self, save_mock):
self.pipeline.post_review = False

35
tests/unit/test_v3.py
View File

@ -10995,3 +10995,38 @@ class TestIncludeVars(ZuulTestCase):
ref='refs/tags/foo'),
dict(name='other-project', result='SUCCESS', ref='refs/tags/foo'),
], ordered=False)
class TestAttributeControl(ZuulTestCase):
@simple_layout('layouts/final-control.yaml')
def test_final_control(self):
A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
for attr in [
'requires',
'provides',
'tags',
'files',
'irrelevant-files',
'required-projects',
'vars',
'extra-vars',
'host-vars',
'group-vars',
'include-vars',
'dependencies',
'failure-output',
]:
content = "- project: {check: {jobs: [test-%s]}}" % (attr,)
file_dict = {'zuul.yaml': content}
B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B',
files=file_dict)
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertIn('Unable to modify job', B.messages[0])
self.assertHistory([
dict(name='final-job', result='SUCCESS'),
], ordered=False)

58
zuul/configloader.py
View File

@ -813,7 +813,23 @@ class JobParser(object):
'deduplicate': vs.Any(bool, 'auto'),
'failure-output': override_list(str),
'image-build-name': str,
}
'attribute-control': {
vs.Any(
'requires',
'provides',
'tags',
'files',
'irrelevant-files',
'required-projects',
'vars',
'extra-vars',
'host-vars',
'group-vars',
'include-vars',
'dependencies',
'failure-output',
): {'final': True},
}}
job_name = {vs.Required('name'): str}
@ -843,6 +859,38 @@ class JobParser(object):
'image-build-name',
]
attr_control_job_attr_map = {
'failure-message': 'failure_message',
'success-message': 'success_message',
'failure-url': 'failure_url',
'success-url': 'success_url',
'hold-following-changes': 'hold_following_changes',
'files': 'file_matcher',
'irrelevant-files': 'irrelevant_file_matcher',
'post-timeout': 'post_timeout',
'pre-run': 'pre_run',
'post-run': 'post_run',
'cleanup-run': 'cleanup_run',
'ansible-split-streams': 'ansible_split_streams',
'ansible-version': 'ansible_version',
'required-projects': 'required_projects',
'vars': 'variables',
'extra-vars': 'extra_variables',
'host-vars': 'host_variables',
'group-vars': 'group_variables',
'include-vars': 'include_vars',
'allowed-projects': 'allowed_projects',
'override-branch': 'override_branch',
'override-checkout': 'override_checkout',
'variant-description': 'variant_description',
'workspace-scheme': 'workspace_scheme',
'failure-output': 'failure_output',
'image-build-name': 'image_build_name',
}
def _getAttrControlJobAttr(self, attr):
return self.attr_control_job_attr_map.get(attr, attr)
def __init__(self, pcontext):
self.log = logging.getLogger("zuul.JobParser")
self.pcontext = pcontext
@ -1228,6 +1276,14 @@ class JobParser(object):
re2.compile(x)
job.failure_output = tuple(sorted(failure_output))
if 'attribute-control' in conf:
with self.pcontext.confAttr(conf,
'attribute-control') as conf_control:
for attr, controls in conf_control.items():
jobattr = self._getAttrControlJobAttr(attr)
for control, value in controls.items():
if control == 'final' and value is True:
job.final_control[jobattr] = True
return job
def _makeZuulRole(self, job, role):

61
zuul/model.py
View File

@ -3616,6 +3616,8 @@ class Job(ConfigObject):
override_control['required_projects'] = False
override_control['failure_output'] = False
final_control = defaultdict(lambda: False)
# These are generally internal attributes which are not
# accessible via configuration.
self.other_attributes = dict(
@ -3634,6 +3636,8 @@ class Job(ConfigObject):
waiting_status=None, # Text description of why its waiting
# Override settings for context attributes:
override_control=override_control,
# Finalize individual attributes (context or execution):
final_control=final_control,
)
self.attributes = {}
@ -4038,31 +4042,35 @@ class Job(ConfigObject):
for zuul_regex in sorted(irrelevant_files,
key=lambda x: x.pattern)])
def _handleFinalControl(self, other, attr):
if other._get(attr) is not None:
if self.final_control[attr]:
# This message says "job foo final attribute bar";
# compare to "final job foo attribute bar" elsewhere
# to distinguish final jobs from final attrs.
raise JobConfigurationError(
"Unable to modify job %s final attribute "
"%s=%s with variant %s" % (
repr(self), attr, other._get(attr),
repr(other)))
if other.final_control[attr]:
fc = self.final_control.copy()
fc[attr] = True
self.final_control = types.MappingProxyType(fc)
def _updateVariableAttribute(self, other, attr):
self._handleFinalControl(other, attr)
if other.override_control[attr]:
setattr(self, attr, getattr(other, attr))
else:
setattr(self, attr, Job._deepUpdate(
getattr(self, attr), getattr(other, attr)))
def updateVariables(self, other):
if other.variables is not None:
if other.override_control['variables']:
self.variables = other.variables
else:
self.variables = Job._deepUpdate(
self.variables, other.variables)
if other.extra_variables is not None:
if other.override_control['extra_variables']:
self.extra_variables = other.extra_variables
else:
self.extra_variables = Job._deepUpdate(
self.extra_variables, other.extra_variables)
if other.host_variables is not None:
if other.override_control['host_variables']:
self.host_variables = other.host_variables
else:
self.host_variables = Job._deepUpdate(
self.host_variables, other.host_variables)
if other.group_variables is not None:
if other.override_control['group_variables']:
self.group_variables = other.group_variables
else:
self.group_variables = Job._deepUpdate(
self.group_variables, other.group_variables)
self._updateVariableAttribute(other, 'variables')
self._updateVariableAttribute(other, 'extra_variables')
self._updateVariableAttribute(other, 'host_variables')
self._updateVariableAttribute(other, 'group_variables')
def updateProjectVariables(self, project_vars):
# Merge project/template variables directly into the job
@ -4070,6 +4078,7 @@ class Job(ConfigObject):
self.variables = Job._deepUpdate(project_vars, self.variables)
def updateProjects(self, other):
self._handleFinalControl(other, 'required_projects')
if other.override_control['required_projects']:
required_projects = {}
else:
@ -4103,6 +4112,7 @@ class Job(ConfigObject):
# If this is a config object, it's frozen, so it's
# safe to shallow copy.
setattr(job, k, v)
job.final_control = self.final_control
return job
def freezePlaybooks(self, pblist, layout, semaphore_handler):
@ -4250,6 +4260,7 @@ class Job(ConfigObject):
self.cleanup_run = other_cleanup_run + self.cleanup_run
self.updateVariables(other)
if other._get('include_vars') is not None:
self._handleFinalControl(other, 'include_vars')
if other.override_control['include_vars']:
include_vars = other.include_vars
else:
@ -4276,6 +4287,7 @@ class Job(ConfigObject):
semaphores = set(self.semaphores).union(set(other.semaphores))
self.semaphores = tuple(sorted(semaphores, key=lambda x: x.name))
if other._get('failure_output') is not None:
self._handleFinalControl(other, 'failure_output')
if other.override_control['failure_output']:
failure_output = other.failure_output
else:
@ -4297,6 +4309,7 @@ class Job(ConfigObject):
for k in self.context_attributes:
if (v := other._get(k)) is None:
continue
self._handleFinalControl(other, k)
if other.override_control[k]:
setattr(self, k, v)
else: