Don't modify project-templates to add job name attributes
A recent change began performing full validation on project-pipeline
job variants rather than deferring to the jobparser. However,
we were modifying the config object inside the project-template parser
to add the job name, before passing it to the job parser. This meant
that the template passed validation the first time, but during a
subsequent reconfiguration, the cached value would already have the
name attribute and fail validation.
This change stops adding the name attribute, instead passing it to
the jobparser. It also disables validation by the jobparser in the
case that the project-template parser has already performed it, so that
we don't slow reconfiguration.
Change-Id: I080b4dd01ac40363a932c0853f0f9670b1e2c511
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 4b0bfc5..28e2b08 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -905,6 +905,50 @@
self.assertIn('expected str for dictionary value',
A.messages[0], "A should have a syntax error reported")
+ def test_project_template(self):
+ # Tests that a project template is not modified when used, and
+ # can therefore be used in subsequent reconfigurations.
+ in_repo_conf = textwrap.dedent(
+ """
+ - job:
+ name: project-test1
+ - project-template:
+ name: some-jobs
+ tenant-one-gate:
+ jobs:
+ - project-test1:
+ required-projects:
+ - org/project1
+ - project:
+ name: org/project
+ templates:
+ - some-jobs
+ """)
+
+ file_dict = {'.zuul.yaml': in_repo_conf}
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ files=file_dict)
+ A.addApproval('Code-Review', 2)
+ self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+ self.waitUntilSettled()
+ self.assertEqual(A.data['status'], 'MERGED')
+ self.fake_gerrit.addEvent(A.getChangeMergedEvent())
+ self.waitUntilSettled()
+ in_repo_conf = textwrap.dedent(
+ """
+ - project:
+ name: org/project1
+ templates:
+ - some-jobs
+ """)
+ file_dict = {'.zuul.yaml': in_repo_conf}
+ B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B',
+ files=file_dict)
+ B.addApproval('Code-Review', 2)
+ self.fake_gerrit.addEvent(B.addApproval('Approved', 1))
+ self.waitUntilSettled()
+ self.assertEqual(B.data['status'], 'MERGED')
+
def test_multi_repo(self):
downstream_repo_conf = textwrap.dedent(
"""
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 2b91966..6cf57f5 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -478,22 +478,27 @@
return None
@staticmethod
- def fromYaml(tenant, layout, conf, project_pipeline=False):
- with configuration_exceptions('job', conf):
- JobParser.schema(conf)
+ def fromYaml(tenant, layout, conf, project_pipeline=False,
+ name=None, validate=True):
+ if validate:
+ with configuration_exceptions('job', conf):
+ JobParser.schema(conf)
+
+ if name is None:
+ name = conf['name']
# NB: The default detection system in the Job class requires
# that we always assign values directly rather than modifying
# them (e.g., "job.run = ..." rather than
# "job.run.append(...)").
- reference = layout.jobs.get(conf['name'], [None])[0]
+ reference = layout.jobs.get(name, [None])[0]
- job = model.Job(conf['name'])
+ job = model.Job(name)
job.source_context = conf.get('_source_context')
job.source_line = conf.get('_start_mark').line + 1
- is_variant = layout.hasJob(conf['name'])
+ is_variant = layout.hasJob(name)
if 'parent' in conf:
if conf['parent'] is not None:
# Parent job is explicitly specified, so inherit from it.
@@ -759,16 +764,11 @@
def parseJobList(self, conf, source_context, start_mark, job_list):
for conf_job in conf:
if isinstance(conf_job, str):
- attrs = dict(name=conf_job)
+ jobname = conf_job
+ attrs = {}
elif isinstance(conf_job, dict):
# A dictionary in a job tree may override params
jobname, attrs = list(conf_job.items())[0]
- if attrs:
- # We are overriding params, so make a new job def
- attrs['name'] = jobname
- else:
- # Not overriding, so add a blank job
- attrs = dict(name=jobname)
else:
raise Exception("Job must be a string or dictionary")
attrs['_source_context'] = source_context
@@ -777,10 +777,11 @@
# validate that the job is existing
with configuration_exceptions('project or project-template',
attrs):
- self.layout.getJob(attrs['name'])
+ self.layout.getJob(jobname)
job_list.addJob(JobParser.fromYaml(self.tenant, self.layout,
- attrs, project_pipeline=True))
+ attrs, project_pipeline=True,
+ name=jobname, validate=False))
class ProjectParser(object):