Fix implied branch matchers with regex chars

If a project had a branch with a "+" in the name (or some other
special regex chars), then implied branch matchers would not match
the branch and jobs would not run.  Instead of treating implied
branch matchers as regexes, treat them as requiring exact matches.

To facilitate creating the correct type of branch matcher, create
them in the configloader instead of in the model classes.

Change-Id: I0421c5a446d0b75194096b1d8f0e0866dae3b8f0

releasenotes/notes/implied-branch-exact-match-74cd3f227a2f6361.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    Projects and jobs on branches whose names have special characters
+    in regular expressions could fail to match changes as intended.
+    Implied branch matchers automatically generated from branch names
+    are now treated as requiring exact matches.  Any user-specified
+    branch matcher (including in :attr:`job.branches` and
+    :attr:`pragma.implied-branches`) are still treated as regular
+    expressions.

tests/unit/test_v3.py

@@ -865,6 +865,29 @@ class TestBranchMismatch(ZuulTestCase):
             dict(name='project-test2', result='SUCCESS', changes='1,1'),
         ], ordered=False)
 
+    def test_implied_branch_matcher_regex(self):
+        # Test that branch names that look like regexes aren't treated
+        # as such for implied branch matchers.
+
+        # Make sure the parent job repo is branched, so it gets
+        # implied branch matchers.
+
+        # The '+' in the branch name would cause the change not to
+        # match if it is treated as a regex.
+        self.create_branch('org/project1', 'feature/foo-0.1.12+bar')
+        self.fake_gerrit.addEvent(
+            self.fake_gerrit.getFakeBranchCreatedEvent(
+                'org/project1', 'feature/foo-0.1.12+bar'))
+
+        A = self.fake_gerrit.addFakeChange(
+            'org/project1', 'feature/foo-0.1.12+bar', 'A')
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.assertHistory([
+            dict(name='project-test1', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+
 
 class TestBranchRef(ZuulTestCase):
     tenant_config_file = 'config/branch-ref/main.yaml'

zuul/change_matcher.py

@@ -58,13 +58,13 @@ class ProjectMatcher(AbstractChangeMatcher):
 
 
 class BranchMatcher(AbstractChangeMatcher):
-    fullmatch = False
+    exactmatch = False
 
     def matches(self, change):
         if hasattr(change, 'branch'):
             # an implied branch matcher must do a fullmatch to work correctly
-            if self.fullmatch:
-                if self.regex.fullmatch(change.branch):
+            if self.exactmatch:
+                if self._regex == change.branch:
                     return True
             else:
                 if self.regex.match(change.branch):
@@ -74,13 +74,17 @@ class BranchMatcher(AbstractChangeMatcher):
             return True
         if hasattr(change, 'containing_branches'):
             for branch in change.containing_branches:
-                if self.regex.fullmatch(branch):
-                    return True
+                if self.exactmatch:
+                    if self._regex == branch:
+                        return True
+                else:
+                    if self.regex.fullmatch(branch):
+                        return True
         return False
 
 
 class ImpliedBranchMatcher(BranchMatcher):
-    fullmatch = True
+    exactmatch = True
 
 
 class FileMatcher(AbstractChangeMatcher):

zuul/configloader.py

@@ -23,6 +23,7 @@ import subprocess
 
 import voluptuous as vs
 
+from zuul import change_matcher
 from zuul import model
 from zuul.lib import yamlutil as yaml
 import zuul.manager.dependent
@@ -447,7 +448,14 @@ class PragmaParser(object):
 
         branches = conf.get('implied-branches')
         if branches is not None:
-            source_context.implied_branches = as_list(branches)
+            # This is a BranchMatcher (not an ImpliedBranchMatcher)
+            # because as user input, we allow/expect this to be
+            # regular expressions.  Only truly implicit branch names
+            # (automatically generated from source file branches) are
+            # ImpliedBranchMatchers.
+            source_context.implied_branches = [
+                change_matcher.BranchMatcher(x)
+                for x in as_list(branches)]
 
 
 class NodeSetParser(object):
@@ -910,14 +918,13 @@ class JobParser(object):
             job.allowed_projects = frozenset(allowed)
 
         branches = None
-        implied = False
         if 'branches' in conf:
-            branches = as_list(conf['branches'])
+            branches = [change_matcher.BranchMatcher(x)
+                        for x in as_list(conf['branches'])]
         elif not project_pipeline:
             branches = self.pcontext.getImpliedBranches(job.source_context)
-            implied = True
         if branches:
-            job.setBranchMatcher(branches, implied=implied)
+            job.setBranchMatcher(branches)
         if 'files' in conf:
             job.setFileMatcher(as_list(conf['files']))
         if 'irrelevant-files' in conf:
@@ -1117,7 +1124,8 @@ class ProjectParser(object):
                 project_config.setImpliedBranchMatchers([])
             else:
                 project_config.setImpliedBranchMatchers(
-                    [source_context.branch])
+                    [change_matcher.ImpliedBranchMatcher(
+                        source_context.branch)])
 
         # Add templates
         for name in conf.get('templates', []):
@@ -1464,7 +1472,7 @@ class ParseContext(object):
         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]
+            return [change_matcher.ImpliedBranchMatcher(source_context.branch)]
         elif source_context.implied_branch_matchers is False:
             return None
 
@@ -1482,7 +1490,7 @@ class ParseContext(object):
 
         if source_context.implied_branches is not None:
             return source_context.implied_branches
-        return [source_context.branch]
+        return [change_matcher.ImpliedBranchMatcher(source_context.branch)]
 
 
 class TenantParser(object):

zuul/model.py

@@ -2668,16 +2668,9 @@ class Job(ConfigObject):
         # Return the raw branch list that match this job
         return self._branches
 
-    def setBranchMatcher(self, branches, implied=False):
+    def setBranchMatcher(self, matchers):
         # Set the branch matcher to match any of the supplied branches
-        self._branches = branches
-        matchers = []
-        if implied:
-            matcher_class = change_matcher.ImpliedBranchMatcher
-        else:
-            matcher_class = change_matcher.BranchMatcher
-        for branch in branches:
-            matchers.append(matcher_class(branch))
+        self._branches = [x._regex for x in matchers]
         self.branch_matcher = change_matcher.MatchAny(matchers)
 
     def setFileMatcher(self, files):
@@ -6337,16 +6330,13 @@ class ProjectConfig(ConfigObject):
         r.queue_name = self.queue_name
         return r
 
-    def setImpliedBranchMatchers(self, branches):
-        if len(branches) == 0:
+    def setImpliedBranchMatchers(self, matchers):
+        if len(matchers) == 0:
             self.branch_matcher = None
-        elif len(branches) > 1:
-            matchers = [change_matcher.ImpliedBranchMatcher(branch)
-                        for branch in branches]
+        elif len(matchers) > 1:
             self.branch_matcher = change_matcher.MatchAny(matchers)
         else:
-            self.branch_matcher = change_matcher.ImpliedBranchMatcher(
-                branches[0])
+            self.branch_matcher = matchers[0]
 
     def changeMatches(self, change):
         if self.branch_matcher and not self.branch_matcher.matches(change):