Do not add implied branch matchers in project-templates

We parse the project-pipeline definition of a job at the location
of the project-pipeline.  This includes both 'project' stanzas and
'project-templates' which are parsed in exactly the same way.  This
normally gives us the behavior we expect in that the job variants
defined by the project or project-template appear to be defined in
the location of the project or project-template.  However, in one
case, we want a 'late-binding' rather than 'early-binding' behavior.

When it comes to calculating implied branch matchers, we want to
use the value that would be derived if there were no project-template,
and instead the job were simply defined on the project stanza itself.

What is intended to happen is that project-pipeline job variants in
a config project should never have implied branch matchers (since
config projects don't have more than one branch).  However, project-
pipeline job variants on in-repo project stazas should get an implied
branch matcher for the branch it's defined on.  This is how we end up
with behavior where the project definition in a project's master branch
controls behavior only on the master branch (unless branches are
explicitly specified), and the definition in a stable branch controls
only the stable branch.

That behavior should happen regardless of where a project-template is
defined.  Currently we are setting an implied branch matcher for job
variants in a project template at the location of definition.  Instead,
set them when the job is actually used in a project.

Change-Id: I5c8fbb3e0a2ecfac8bd95795be002e8cd15e61db
This commit is contained in:
James E. Blair 2017-09-29 15:14:31 -07:00 committed by Clark Boylan
parent 167d6cd5ed
commit e74f571085
8 changed files with 112 additions and 29 deletions

View File

@ -653,10 +653,22 @@ Here is an example of two job definitions:
configuration, Zuul reads the ``master`` branch of a given configuration, Zuul reads the ``master`` branch of a given
project first, then other branches in alphabetical order. project first, then other branches in alphabetical order.
* In the case of a job variant defined within a :ref:`project`,
if the project definition is in a :term:`config-project`, no
implied branch specifier is used. If it appears in an
:term:`untrusted-project`, with no branch specifier, the
branch containing the project definition is used as an implied
branch specifier.
* In the case of a job variant defined within a
:ref:`project-template`, if no branch specifier appears, the
implied branch specifier for the :ref:`project` definition which
uses the project-template will be used.
* Any further job variants other than the reference definition * Any further job variants other than the reference definition
in an untrusted-project will, if they do not have a branch in an untrusted-project will, if they do not have a branch
specifier, will have an implied branch specifier for the specifier, have an implied branch specifier for the current
current branch applied. branch applied.
This allows for the very simple and expected workflow where if a This allows for the very simple and expected workflow where if a
project defines a job on the ``master`` branch with no branch project defines a job on the ``master`` branch with no branch

View File

@ -0,0 +1,4 @@
- project:
name: untrusted-config
templates:
- test-one-and-two

View File

@ -5,5 +5,6 @@
config-projects: config-projects:
- common-config - common-config
untrusted-projects: untrusted-projects:
- untrusted-config
- org/templated-project - org/templated-project
- org/layered-project - org/layered-project

View File

@ -59,6 +59,14 @@ def main():
if fn in ['zuul.yaml', '.zuul.yaml']: if fn in ['zuul.yaml', '.zuul.yaml']:
print_file('File: ' + os.path.join(gitrepo, fn), print_file('File: ' + os.path.join(gitrepo, fn),
os.path.join(reporoot, fn)) os.path.join(reporoot, fn))
for subdir in ['.zuul.d', 'zuul.d']:
zuuld = os.path.join(reporoot, subdir)
if not os.path.exists(zuuld):
continue
filenames = os.listdir(zuuld)
for fn in filenames:
print_file('File: ' + os.path.join(gitrepo, subdir, fn),
os.path.join(zuuld, fn))
if __name__ == '__main__': if __name__ == '__main__':

View File

@ -4841,6 +4841,42 @@ class TestSchedulerTemplatedProject(ZuulTestCase):
self.assertEqual(self.getJobFromHistory('project-test6').result, self.assertEqual(self.getJobFromHistory('project-test6').result,
'SUCCESS') 'SUCCESS')
def test_unimplied_branch_matchers(self):
# This tests that there are no implied branch matchers added
# by project templates.
self.create_branch('org/layered-project', 'stable')
A = self.fake_gerrit.addFakeChange(
'org/layered-project', 'stable', 'A')
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(self.getJobFromHistory('project-test1').result,
'SUCCESS')
print(self.getJobFromHistory('project-test1').
parameters['zuul']['_inheritance_path'])
def test_implied_branch_matchers(self):
# This tests that there is an implied branch matcher when a
# template is used on an in-repo project pipeline definition.
self.create_branch('untrusted-config', 'stable')
self.fake_gerrit.addEvent(
self.fake_gerrit.getFakeBranchCreatedEvent(
'untrusted-config', 'stable'))
self.waitUntilSettled()
A = self.fake_gerrit.addFakeChange(
'untrusted-config', 'stable', 'A')
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(self.getJobFromHistory('project-test1').result,
'SUCCESS')
print(self.getJobFromHistory('project-test1').
parameters['zuul']['_inheritance_path'])
class TestSchedulerSuccessURL(ZuulTestCase): class TestSchedulerSuccessURL(ZuulTestCase):
tenant_config_file = 'config/success-url/main.yaml' tenant_config_file = 'config/success-url/main.yaml'

View File

@ -456,19 +456,22 @@ class JobParser(object):
# the reference definition of this job, and this is a project # the reference definition of this job, and this is a project
# repo, add an implicit branch matcher for this branch # repo, add an implicit branch matcher for this branch
# (assuming there are no explicit branch matchers). But only # (assuming there are no explicit branch matchers). But only
# for top-level job definitions and variants. # for top-level job definitions and variants. Never for
# Project-pipeline job variants should more closely attach to # project-templates. They, and in-project project-pipeline
# their branch if they appear in a project-repo. # job variants, should more closely attach to their branch if
# they appear in a project-repo. That's handled in the
# ProjectParser.
if (reference and if (reference and
reference.source_context and reference.source_context and
reference.source_context.branch != job.source_context.branch): reference.source_context.branch != job.source_context.branch):
same_context = False same_branch = False
else: else:
same_context = True same_branch = True
if (job.source_context and if (job.source_context and
(not job.source_context.trusted) and (not job.source_context.trusted) and
((not same_context) or project_pipeline)): (not project_pipeline) and
(not same_branch)):
return [job.source_context.branch] return [job.source_context.branch]
return None return None
@ -669,10 +672,7 @@ class JobParser(object):
if (not branches) and ('branches' in conf): if (not branches) and ('branches' in conf):
branches = as_list(conf['branches']) branches = as_list(conf['branches'])
if branches: if branches:
matchers = [] job.setBranchMatcher(branches)
for branch in branches:
matchers.append(change_matcher.BranchMatcher(branch))
job.branch_matcher = change_matcher.MatchAny(matchers)
if 'files' in conf: if 'files' in conf:
matchers = [] matchers = []
for fn in as_list(conf['files']): for fn in as_list(conf['files']):
@ -730,8 +730,12 @@ class ProjectTemplateParser(object):
return vs.Schema(project_template) return vs.Schema(project_template)
@staticmethod @staticmethod
def fromYaml(tenant, layout, conf): def fromYaml(tenant, layout, conf, template):
with configuration_exceptions('project or project-template', conf): if template:
project_or_template = 'project-template'
else:
project_or_template = 'project'
with configuration_exceptions(project_or_template, conf):
ProjectTemplateParser.getSchema(layout)(conf) ProjectTemplateParser.getSchema(layout)(conf)
# Make a copy since we modify this later via pop # Make a copy since we modify this later via pop
conf = copy.deepcopy(conf) conf = copy.deepcopy(conf)
@ -747,12 +751,13 @@ class ProjectTemplateParser(object):
project_pipeline.queue_name = conf_pipeline.get('queue') project_pipeline.queue_name = conf_pipeline.get('queue')
ProjectTemplateParser._parseJobList( ProjectTemplateParser._parseJobList(
tenant, layout, conf_pipeline.get('jobs', []), tenant, layout, conf_pipeline.get('jobs', []),
source_context, start_mark, project_pipeline.job_list) source_context, start_mark, project_pipeline.job_list,
template)
return project_template return project_template
@staticmethod @staticmethod
def _parseJobList(tenant, layout, conf, source_context, def _parseJobList(tenant, layout, conf, source_context,
start_mark, job_list): start_mark, job_list, template):
for conf_job in conf: for conf_job in conf:
if isinstance(conf_job, str): if isinstance(conf_job, str):
attrs = dict(name=conf_job) attrs = dict(name=conf_job)
@ -815,6 +820,7 @@ class ProjectParser(object):
configs = [] configs = []
for conf in conf_list: for conf in conf_list:
implied_branch = None
with configuration_exceptions('project', conf): with configuration_exceptions('project', conf):
if not conf['_source_context'].trusted: if not conf['_source_context'].trusted:
if project != conf['_source_context'].project: if project != conf['_source_context'].project:
@ -828,13 +834,18 @@ class ProjectParser(object):
# all of the templates, including the newly parsed # all of the templates, including the newly parsed
# one, in order. # one, in order.
project_template = ProjectTemplateParser.fromYaml( project_template = ProjectTemplateParser.fromYaml(
tenant, layout, conf) tenant, layout, conf, template=False)
# If this project definition is in a place where it
# should get implied branch matchers, set it.
if (not conf['_source_context'].trusted):
implied_branch = conf['_source_context'].branch
for name in conf_templates: for name in conf_templates:
if name not in layout.project_templates: if name not in layout.project_templates:
raise TemplateNotFoundError(name) raise TemplateNotFoundError(name)
configs.extend([layout.project_templates[name] configs.extend([(layout.project_templates[name],
implied_branch)
for name in conf_templates]) for name in conf_templates])
configs.append(project_template) configs.append((project_template, implied_branch))
# Set the following values to the first one that we # Set the following values to the first one that we
# find and ignore subsequent settings. # find and ignore subsequent settings.
mode = conf.get('merge-mode') mode = conf.get('merge-mode')
@ -855,12 +866,13 @@ class ProjectParser(object):
# For every template, iterate over the job tree and replace or # For every template, iterate over the job tree and replace or
# create the jobs in the final definition as needed. # create the jobs in the final definition as needed.
pipeline_defined = False pipeline_defined = False
for template in configs: for (template, implied_branch) in configs:
if pipeline.name in template.pipelines: if pipeline.name in template.pipelines:
pipeline_defined = True pipeline_defined = True
template_pipeline = template.pipelines[pipeline.name] template_pipeline = template.pipelines[pipeline.name]
project_pipeline.job_list.inheritFrom( project_pipeline.job_list.inheritFrom(
template_pipeline.job_list) template_pipeline.job_list,
implied_branch)
if template_pipeline.queue_name: if template_pipeline.queue_name:
queue_name = template_pipeline.queue_name queue_name = template_pipeline.queue_name
if queue_name: if queue_name:
@ -1520,7 +1532,7 @@ class TenantParser(object):
if 'project-template' not in classes: if 'project-template' not in classes:
continue continue
layout.addProjectTemplate(ProjectTemplateParser.fromYaml( layout.addProjectTemplate(ProjectTemplateParser.fromYaml(
tenant, layout, config_template)) tenant, layout, config_template, template=True))
for config_projects in data.projects.values(): for config_projects in data.projects.values():
# Unlike other config classes, we expect multiple project # Unlike other config classes, we expect multiple project

View File

@ -23,6 +23,8 @@ from uuid import uuid4
import urllib.parse import urllib.parse
import textwrap import textwrap
from zuul import change_matcher
MERGER_MERGE = 1 # "git merge" MERGER_MERGE = 1 # "git merge"
MERGER_MERGE_RESOLVE = 2 # "git merge -s resolve" MERGER_MERGE_RESOLVE = 2 # "git merge -s resolve"
MERGER_CHERRY_PICK = 3 # "git cherry-pick" MERGER_CHERRY_PICK = 3 # "git cherry-pick"
@ -901,6 +903,13 @@ class Job(object):
if changed: if changed:
self.roles = tuple(newroles) self.roles = tuple(newroles)
def setBranchMatcher(self, branches):
# Set the branch matcher to match any of the supplied branches
matchers = []
for branch in branches:
matchers.append(change_matcher.BranchMatcher(branch))
self.branch_matcher = change_matcher.MatchAny(matchers)
def updateVariables(self, other_vars): def updateVariables(self, other_vars):
v = self.variables v = self.variables
Job._deepUpdate(v, other_vars) Job._deepUpdate(v, other_vars)
@ -1028,14 +1037,14 @@ class JobList(object):
else: else:
self.jobs[job.name] = [job] self.jobs[job.name] = [job]
def inheritFrom(self, other): def inheritFrom(self, other, implied_branch):
for jobname, jobs in other.jobs.items(): for jobname, jobs in other.jobs.items():
if jobname in self.jobs: joblist = self.jobs.setdefault(jobname, [])
self.jobs[jobname].extend(jobs) for job in jobs:
else: if not job.branch_matcher and implied_branch:
# Be sure to make a copy here since this list may be job = job.copy()
# modified. job.setBranchMatcher([implied_branch])
self.jobs[jobname] = jobs[:] joblist.append(job)
class JobGraph(object): class JobGraph(object):
@ -2103,6 +2112,7 @@ class ProjectConfig(object):
def __init__(self, name): def __init__(self, name):
self.name = name self.name = name
self.merge_mode = None self.merge_mode = None
# The default branch for the project (usually master).
self.default_branch = None self.default_branch = None
self.pipelines = {} self.pipelines = {}
self.private_key_file = None self.private_key_file = None