Merge "Protect against builds dict changing while we iterate" into feature/zuulv3
diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst
index 025ea71..f55fb4f 100644
--- a/doc/source/user/config.rst
+++ b/doc/source/user/config.rst
@@ -653,10 +653,22 @@
         configuration, Zuul reads the ``master`` branch of a given
         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
         in an untrusted-project will, if they do not have a branch
-        specifier, will have an implied branch specifier for the
-        current branch applied.
+        specifier, have an implied branch specifier for the current
+        branch applied.
 
       This allows for the very simple and expected workflow where if a
       project defines a job on the ``master`` branch with no branch
diff --git a/tests/fixtures/config/templated-project/git/untrusted-config/zuul.d/project.yaml b/tests/fixtures/config/templated-project/git/untrusted-config/zuul.d/project.yaml
new file mode 100644
index 0000000..6d1892c
--- /dev/null
+++ b/tests/fixtures/config/templated-project/git/untrusted-config/zuul.d/project.yaml
@@ -0,0 +1,4 @@
+- project:
+    name: untrusted-config
+    templates:
+      - test-one-and-two
diff --git a/tests/fixtures/config/templated-project/git/common-config/zuul.d/templates.yaml b/tests/fixtures/config/templated-project/git/untrusted-config/zuul.d/templates.yaml
similarity index 100%
rename from tests/fixtures/config/templated-project/git/common-config/zuul.d/templates.yaml
rename to tests/fixtures/config/templated-project/git/untrusted-config/zuul.d/templates.yaml
diff --git a/tests/fixtures/config/templated-project/main.yaml b/tests/fixtures/config/templated-project/main.yaml
index e59b396..bb59838 100644
--- a/tests/fixtures/config/templated-project/main.yaml
+++ b/tests/fixtures/config/templated-project/main.yaml
@@ -5,5 +5,6 @@
         config-projects:
           - common-config
         untrusted-projects:
+          - untrusted-config
           - org/templated-project
           - org/layered-project
diff --git a/tests/print_layout.py b/tests/print_layout.py
index a295886..055270f 100644
--- a/tests/print_layout.py
+++ b/tests/print_layout.py
@@ -59,6 +59,14 @@
             if fn in ['zuul.yaml', '.zuul.yaml']:
                 print_file('File: ' + os.path.join(gitrepo, 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__':
diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index 7f8c0a3..9e5e36c 100755
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -4880,6 +4880,42 @@
         self.assertEqual(self.getJobFromHistory('project-test6').result,
                          '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):
     tenant_config_file = 'config/success-url/main.yaml'
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 62cec36..c1a65be 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -456,19 +456,22 @@
         # the reference definition of this job, and this is a project
         # repo, add an implicit branch matcher for this branch
         # (assuming there are no explicit branch matchers).  But only
-        # for top-level job definitions and variants.
-        # Project-pipeline job variants should more closely attach to
-        # their branch if they appear in a project-repo.
+        # for top-level job definitions and variants.  Never for
+        # project-templates.  They, and in-project project-pipeline
+        # 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
             reference.source_context and
             reference.source_context.branch != job.source_context.branch):
-            same_context = False
+            same_branch = False
         else:
-            same_context = True
+            same_branch = True
 
         if (job.source_context 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 None
 
@@ -669,10 +672,7 @@
         if (not branches) and ('branches' in conf):
             branches = as_list(conf['branches'])
         if branches:
-            matchers = []
-            for branch in branches:
-                matchers.append(change_matcher.BranchMatcher(branch))
-            job.branch_matcher = change_matcher.MatchAny(matchers)
+            job.setBranchMatcher(branches)
         if 'files' in conf:
             matchers = []
             for fn in as_list(conf['files']):
@@ -730,8 +730,12 @@
         return vs.Schema(project_template)
 
     @staticmethod
-    def fromYaml(tenant, layout, conf):
-        with configuration_exceptions('project or project-template', conf):
+    def fromYaml(tenant, layout, conf, template):
+        if template:
+            project_or_template = 'project-template'
+        else:
+            project_or_template = 'project'
+        with configuration_exceptions(project_or_template, conf):
             ProjectTemplateParser.getSchema(layout)(conf)
         # Make a copy since we modify this later via pop
         conf = copy.deepcopy(conf)
@@ -747,12 +751,13 @@
             project_pipeline.queue_name = conf_pipeline.get('queue')
             ProjectTemplateParser._parseJobList(
                 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
 
     @staticmethod
     def _parseJobList(tenant, layout, conf, source_context,
-                      start_mark, job_list):
+                      start_mark, job_list, template):
         for conf_job in conf:
             if isinstance(conf_job, str):
                 attrs = dict(name=conf_job)
@@ -815,6 +820,7 @@
 
         configs = []
         for conf in conf_list:
+            implied_branch = None
             with configuration_exceptions('project', conf):
                 if not conf['_source_context'].trusted:
                     if project != conf['_source_context'].project:
@@ -828,13 +834,18 @@
                 # all of the templates, including the newly parsed
                 # one, in order.
                 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:
                     if name not in layout.project_templates:
                         raise TemplateNotFoundError(name)
-                configs.extend([layout.project_templates[name]
+                configs.extend([(layout.project_templates[name],
+                                 implied_branch)
                                 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
                 # find and ignore subsequent settings.
                 mode = conf.get('merge-mode')
@@ -855,12 +866,13 @@
             # For every template, iterate over the job tree and replace or
             # create the jobs in the final definition as needed.
             pipeline_defined = False
-            for template in configs:
+            for (template, implied_branch) in configs:
                 if pipeline.name in template.pipelines:
                     pipeline_defined = True
                     template_pipeline = template.pipelines[pipeline.name]
                     project_pipeline.job_list.inheritFrom(
-                        template_pipeline.job_list)
+                        template_pipeline.job_list,
+                        implied_branch)
                     if template_pipeline.queue_name:
                         queue_name = template_pipeline.queue_name
             if queue_name:
@@ -1520,7 +1532,7 @@
             if 'project-template' not in classes:
                 continue
             layout.addProjectTemplate(ProjectTemplateParser.fromYaml(
-                tenant, layout, config_template))
+                tenant, layout, config_template, template=True))
 
         for config_projects in data.projects.values():
             # Unlike other config classes, we expect multiple project
diff --git a/zuul/model.py b/zuul/model.py
index 6fe3b94..4d40b6c 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -23,6 +23,8 @@
 import urllib.parse
 import textwrap
 
+from zuul import change_matcher
+
 MERGER_MERGE = 1          # "git merge"
 MERGER_MERGE_RESOLVE = 2  # "git merge -s resolve"
 MERGER_CHERRY_PICK = 3    # "git cherry-pick"
@@ -915,6 +917,13 @@
         if changed:
             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):
         v = self.variables
         Job._deepUpdate(v, other_vars)
@@ -1042,14 +1051,14 @@
         else:
             self.jobs[job.name] = [job]
 
-    def inheritFrom(self, other):
+    def inheritFrom(self, other, implied_branch):
         for jobname, jobs in other.jobs.items():
-            if jobname in self.jobs:
-                self.jobs[jobname].extend(jobs)
-            else:
-                # Be sure to make a copy here since this list may be
-                # modified.
-                self.jobs[jobname] = jobs[:]
+            joblist = self.jobs.setdefault(jobname, [])
+            for job in jobs:
+                if not job.branch_matcher and implied_branch:
+                    job = job.copy()
+                    job.setBranchMatcher([implied_branch])
+                joblist.append(job)
 
 
 class JobGraph(object):
@@ -2117,6 +2126,7 @@
     def __init__(self, name):
         self.name = name
         self.merge_mode = None
+        # The default branch for the project (usually master).
         self.default_branch = None
         self.pipelines = {}
         self.private_key_file = None