Don't add implied branch matchers to project-pipeline variants

The project pipelines themselves have branch matchers now, so we should
not add implied branch matchers to project-pipeline job variants.  This
error would cause centrally defined tag/release jobs not to run when added
to in-repo project-pipeline definitions in multi-branch repos because
these project-pipeline variants would end up with branch matchers.

There remains a similar case where if a job is defined in a multi-branch
repo with no explicit branch matchers and added to a tag/release pipeline,
it will not run because the job definition itself will not match the tag.
Currently the only solution to this is to add an explicit branch matcher
to one or more of the top-level job definitions.  A more intuitive solution
is difficult because in the case of multiple variants, it's not clear which
should apply.

Removing the implied branch matchers from project-pipeline jobs also removes
them from project-template jobs.  We previously added branch matchers to
project configs, but did not do the same for project-templates.  This change
requires that we do so.  Now all projects and project-templates are given
implied branch matchers if appropriate, and these are used to determine if
their jobs are added.  This is a further behavior change in that a project
which invokes a template defined in another project which is branched will
(absent the disabling of implicit branch matchers) no longer use that template
on branches other than the one where it is defined.

Change-Id: I55cec1897b0d64fa61d43ef5dbeb8a3c37bf7862

doc/source/user/config.rst

@@ -1034,27 +1034,6 @@ Here is an example of two job definitions:
         branch, the branch containing the job definition is used as an
         implied branch specifier.
 
-      * 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 containing the project-template definition is
-        used as an implied branch specifier.  This means that
-        definitions of the same project-template on different branches
-        may run different jobs.
-
-        When that project-template is used by a :ref:`project`
-        definition within a :term:`untrusted-project`, the branch
-        containing that project definition is combined with the branch
-        specifier of the project-template.  This means it is possible
-        for a project to use a template on one branch, but not on
-        another.
-
       This allows for the very simple and expected workflow where if a
       project defines a job on the ``master`` branch with no branch
       specifier, and then creates a new branch based on ``master``,
@@ -1098,8 +1077,13 @@ participates in a pipeline.
 Multiple project definitions may appear for the same project (for
 example, in a central :term:`config projects <config-project>` as well
 as in a repo's own ``.zuul.yaml``).  In this case, all of the project
-definitions are combined (the jobs listed in all of the definitions
-will be run).
+definitions for the relevant branch are combined (the jobs listed in
+all of the matching definitions will be run).  If a project definition
+appears in a :term:`config-project`, it will apply to all branches of
+the project.  If it appears in a branch of an
+:term:`untrusted-project` it will only apply to changes on that
+branch.  In the case of an item which does not have a branch (for
+example, a tag), all of the project definitions will be combined.
 
 Consider the following project definition::
 
@@ -1266,7 +1250,14 @@ A Project Template uses the same syntax as a :ref:`project`
 definition, however, in the case of a template, the
 :attr:`project.name` attribute does not refer to the name of a
 project, but rather names the template so that it can be referenced in
-a `Project` definition.
+a :ref:`project` definition.
+
+Because Project Templates may be used outside of the projects they are
+defined, they honor the implied branches :ref:`pragma` (unlike
+Projects).  The same heuristics described in :attr:`job.branches` that
+determine what implied branches a :ref:`job` will receive apply to
+Project Templates (with the exception that it is not possible to
+explicity set a branch matcher on a Project Template).
 
 .. _secret:
 
@@ -1333,7 +1324,7 @@ branch will not immediately produce a configuration error.
    .. attr:: name
       :required:
 
-      The name of the secret, used in a :ref:`Job` definition to
+      The name of the secret, used in a :ref:`job` definition to
       request the secret.
 
    .. attr:: data
@@ -1507,9 +1498,9 @@ pragma directives may not be set and then unset within the same file.
 
       This is a boolean, which, if set, may be used to enable
       (``True``) or disable (``False``) the addition of implied branch
-      matchers to job definitions.  Normally Zuul decides whether to
-      add these based on heuristics described in :attr:`job.branches`.
-      This attribute overrides that behavior.
+      matchers to job and project-template definitions.  Normally Zuul
+      decides whether to add these based on heuristics described in
+      :attr:`job.branches`.  This attribute overrides that behavior.
 
       This can be useful if a project has multiple branches, yet the
       jobs defined in the master branch should apply to all branches.
@@ -1521,7 +1512,8 @@ pragma directives may not be set and then unset within the same file.
 
       This is a list of regular expressions, just as
       :attr:`job.branches`, which may be used to supply the value of
-      the implied branch matcher for all jobs in a file.
+      the implied branch matcher for all jobs and project-templates in
+      a file.
 
       This may be useful if two projects share jobs but have
       dissimilar branch names.  If, for example, two projects have

releasenotes/notes/template-branch-matchers-2259b585d239b6fe.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Project Templates are now branch-aware and behave more like
+    project stanzas.  If a template is defined on a branch, it will
+    only apply to changes to that branch.

tests/fixtures/config/branch-tag/git/org_project/.zuul.yaml

@@ -6,4 +6,5 @@
     name: org/project
     tag:
       jobs:
+        - central-job
         - test-job

tests/fixtures/config/branch-tag/git/project-config/playbooks/central-job.yaml

@@ -0,0 +1,2 @@
+- hosts: all
+  tasks: []

tests/fixtures/config/branch-tag/git/project-config/zuul.yaml

@@ -10,6 +10,10 @@
     name: base
     parent: null
 
+- job:
+    name: central-job
+    run: playbooks/central-job.yaml
+
 - project:
     name: project-config
     tag:

tests/unit/test_scheduler.py

@@ -5227,7 +5227,7 @@ class TestSchedulerTemplatedProject(ZuulTestCase):
 
     def test_unimplied_branch_matchers(self):
         # This tests that there are no implied branch matchers added
-        # by project templates.
+        # to project templates in unbranched projects.
         self.create_branch('org/layered-project', 'stable')
 
         A = self.fake_gerrit.addFakeChange(

tests/unit/test_v3.py

@@ -402,13 +402,36 @@ class TestBranchDeletion(ZuulTestCase):
 class TestBranchTag(ZuulTestCase):
     tenant_config_file = 'config/branch-tag/main.yaml'
 
-    def test_negative_branch_match(self):
-        # Test that a negative branch matcher works with implied branches.
+    def test_no_branch_match(self):
+        # Test that tag jobs run with no explicit branch matchers
         event = self.fake_gerrit.addFakeTag('org/project', 'master', 'foo')
         self.fake_gerrit.addEvent(event)
         self.waitUntilSettled()
         self.assertHistory([
-            dict(name='test-job', result='SUCCESS', ref='refs/tags/foo')])
+            dict(name='central-job', result='SUCCESS', ref='refs/tags/foo'),
+            dict(name='test-job', result='SUCCESS', ref='refs/tags/foo')],
+            ordered=False)
+
+    def test_no_branch_match_multi_branch(self):
+        # Test that tag jobs run with no explicit branch matchers in a
+        # multi-branch project (where jobs generally get implied
+        # branch matchers)
+        self.create_branch('org/project', 'stable/pike')
+        self.fake_gerrit.addEvent(
+            self.fake_gerrit.getFakeBranchCreatedEvent(
+                'org/project', 'stable/pike'))
+        self.waitUntilSettled()
+
+        event = self.fake_gerrit.addFakeTag('org/project', 'master', 'foo')
+        self.fake_gerrit.addEvent(event)
+        self.waitUntilSettled()
+        # test-job does not run in this case because it is defined in
+        # a branched repo with implied branch matchers.  A release job
+        # defined in a multi-branch repo would need at least one
+        # top-level variant with no branch matcher in order to match a
+        # tag.
+        self.assertHistory([
+            dict(name='central-job', result='SUCCESS', ref='refs/tags/foo')])
 
 
 class TestBranchNegative(ZuulTestCase):

zuul/configloader.py

@@ -577,32 +577,6 @@ class JobParser(object):
         self.log = logging.getLogger("zuul.JobParser")
         self.pcontext = pcontext
 
-    def _getImpliedBranches(self, job):
-        # If the user has set a pragma directive for this, use the
-        # value (if unset, the value is None).
-        if job.source_context.implied_branch_matchers is True:
-            if job.source_context.implied_branches is not None:
-                return job.source_context.implied_branches
-            return [job.source_context.branch]
-        elif job.source_context.implied_branch_matchers is False:
-            return None
-
-        # If this is a trusted project, don't create implied branch
-        # matchers.
-        if job.source_context.trusted:
-            return None
-
-        # If this project only has one branch, don't create implied
-        # branch matchers.  This way central job repos can work.
-        branches = self.pcontext.tenant.getProjectBranches(
-            job.source_context.project)
-        if len(branches) == 1:
-            return None
-
-        if job.source_context.implied_branches is not None:
-            return job.source_context.implied_branches
-        return [job.source_context.branch]
-
     def fromYaml(self, conf, project_pipeline=False, name=None,
                  validate=True):
         if validate:
@@ -782,10 +756,10 @@ class JobParser(object):
             job.allowed_projects = frozenset(allowed)
 
         branches = None
-        if ('branches' not in conf):
-            branches = self._getImpliedBranches(job)
-        if (not branches) and ('branches' in conf):
+        if 'branches' in conf:
             branches = as_list(conf['branches'])
+        elif not project_pipeline:
+            branches = self.pcontext.getImpliedBranches(job.source_context)
         if branches:
             job.setBranchMatcher(branches)
         if 'files' in conf:
@@ -868,6 +842,13 @@ class ProjectTemplateParser(object):
             self.parseJobList(
                 conf_pipeline.get('jobs', []),
                 source_context, start_mark, project_pipeline.job_list)
+
+        # If this project definition is in a place where it
+        # should get implied branch matchers, set it.
+        branches = self.pcontext.getImpliedBranches(source_context)
+        if branches:
+            project_template.setImpliedBranchMatchers(branches)
+
         if freeze:
             project_template.freeze()
         return project_template
@@ -957,11 +938,15 @@ class ProjectParser(object):
 
             project_config.name = project.canonical_name
 
-            if not conf['_source_context'].trusted:
-                # If this project definition is in a place where it
-                # should get implied branch matchers, set it.
-                project_config.addImpliedBranchMatcher(
-                    conf['_source_context'].branch)
+            # Pragmas can cause templates to end up with implied
+            # branch matchers for arbitrary branches, but project
+            # stanzas should not.  They should either have the current
+            # branch or no branch matcher.
+            if conf['_source_context'].trusted:
+                project_config.setImpliedBranchMatchers([])
+            else:
+                project_config.setImpliedBranchMatchers(
+                    [conf['_source_context'].branch])
 
         # Add templates
         for name in conf.get('templates', []):
@@ -1184,6 +1169,32 @@ class ParseContext(object):
         self.project_template_parser = ProjectTemplateParser(self)
         self.project_parser = ProjectParser(self)
 
+    def getImpliedBranches(self, source_context):
+        # If the user has set a pragma directive for this, use the
+        # value (if unset, the value is None).
+        if source_context.implied_branch_matchers is True:
+            if source_context.implied_branches is not None:
+                return source_context.implied_branches
+            return [source_context.branch]
+        elif source_context.implied_branch_matchers is False:
+            return None
+
+        # If this is a trusted project, don't create implied branch
+        # matchers.
+        if source_context.trusted:
+            return None
+
+        # If this project only has one branch, don't create implied
+        # branch matchers.  This way central job repos can work.
+        branches = self.tenant.getProjectBranches(
+            source_context.project)
+        if len(branches) == 1:
+            return None
+
+        if source_context.implied_branches is not None:
+            return source_context.implied_branches
+        return [source_context.branch]
+
 
 class TenantParser(object):
     def __init__(self, connections, scheduler, merger):

zuul/model.py

@@ -1753,6 +1753,8 @@ class QueueItem(object):
             # Conditionally set self.ppc so that the debug method can
             # consult it as we resolve the jobs.
             self.project_pipeline_config = ppc
+            for msg in ppc.debug_messages:
+                self.debug(msg)
             job_graph = self.layout.createJobGraph(self, ppc)
             for job in job_graph.getJobs():
                 # Ensure that each jobs's dependencies are fully
@@ -2537,6 +2539,10 @@ class ProjectPipelineConfig(ConfigObject):
         self.job_list = JobList()
         self.queue_name = None
         self.debug = False
+        self.debug_messages = []
+
+    def addDebug(self, msg):
+        self.debug_messages.append(msg)
 
     def update(self, other):
         if not isinstance(other, ProjectPipelineConfig):
@@ -2564,6 +2570,10 @@ class ProjectConfig(ConfigObject):
         self.merge_mode = None
         self.default_branch = None
 
+    def __repr__(self):
+        return '<ProjectConfig %s source: %s %s>' % (
+            self.name, self.source_context, self.branch_matcher)
+
     def copy(self):
         r = self.__class__(self.name)
         r.source_context = self.source_context
@@ -2575,8 +2585,16 @@ class ProjectConfig(ConfigObject):
         r.default_branch = self.default_branch
         return r
 
-    def addImpliedBranchMatcher(self, branch):
-        self.branch_matcher = change_matcher.ImpliedBranchMatcher(branch)
+    def setImpliedBranchMatchers(self, branches):
+        if len(branches) == 0:
+            self.branch_matcher = None
+        elif len(branches) > 1:
+            matchers = [change_matcher.ImpliedBranchMatcher(branch)
+                        for branch in branches]
+            self.branch_matcher = change_matcher.MatchAny(matchers)
+        else:
+            self.branch_matcher = change_matcher.ImpliedBranchMatcher(
+                branches[0])
 
     def changeMatches(self, change):
         if self.branch_matcher and not self.branch_matcher.matches(change):
@@ -3037,12 +3055,28 @@ class Layout(object):
         project_in_pipeline = False
         for pc in self.getProjectConfigs(item.change.project.canonical_name):
             if not pc.changeMatches(item.change):
+                msg = "Project %s did not match" % (pc,)
+                ppc.addDebug(msg)
+                self.log.debug("%s item %s" % (msg, item))
                 continue
+            msg = "Project %s matched" % (pc,)
+            ppc.addDebug(msg)
+            self.log.debug("%s item %s" % (msg, item))
             for template_name in pc.templates:
                 templates = self.getProjectTemplates(template_name)
                 for template in templates:
                     template_ppc = template.pipelines.get(item.pipeline.name)
                     if template_ppc:
+                        if not template.changeMatches(item.change):
+                            msg = "Project template %s did not match" % (
+                                template,)
+                            ppc.addDebug(msg)
+                            self.log.debug("%s item %s" % (msg, item))
+                            continue
+                        msg = "Project template %s matched" % (
+                            template,)
+                        ppc.addDebug(msg)
+                        self.log.debug("%s item %s" % (msg, item))
                         project_in_pipeline = True
                         ppc.update(template_ppc)
             project_ppc = pc.pipelines.get(item.pipeline.name)