Really fix canonical/non-canonical project merge

In change Icaf8fca3aa4577b009d691f9a67adcb43ea040f5 the merge of
canonical and non-canonical projects was fixed. However the fix was
not complete and only covered the static configuration workflow. The
dynamic configuration workflow is still broken.

Now really fix by canonicalize the project names in every case and
make the tenant parameter mandatory.

Change-Id: I5af74763fc9c4be395a341f28c6751d22bd46195

3 files changed, 45 insertions, 29 deletions

diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 44aa96665..2779e6e66 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -543,11 +543,23 @@ class TestInRepoConfig(ZuulTestCase):
                 name: project-test2
                 run: playbooks/project-test2.yaml
 
+            - job:
+                name: project-test3
+                run: playbooks/project-test2.yaml
+
+            # add a job by the short project name
             - project:
                 name: org/project
                 tenant-one-gate:
                   jobs:
                     - project-test2
+
+            # add a job by the canonical project name
+            - project:
+                name: review.example.com/org/project
+                tenant-one-gate:
+                  jobs:
+                    - project-test3
             """)
 
         in_repo_playbook = textwrap.dedent(
@@ -569,7 +581,9 @@ class TestInRepoConfig(ZuulTestCase):
         self.assertIn('tenant-one-gate', A.messages[1],
                       "A should transit tenant-one gate")
         self.assertHistory([
-            dict(name='project-test2', result='SUCCESS', changes='1,1')])
+            dict(name='project-test2', result='SUCCESS', changes='1,1'),
+            dict(name='project-test3', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
 
         self.fake_gerrit.addEvent(A.getChangeMergedEvent())
         self.waitUntilSettled()
@@ -584,7 +598,10 @@ class TestInRepoConfig(ZuulTestCase):
                          'SUCCESS')
         self.assertHistory([
             dict(name='project-test2', result='SUCCESS', changes='1,1'),
-            dict(name='project-test2', result='SUCCESS', changes='2,1')])
+            dict(name='project-test3', result='SUCCESS', changes='1,1'),
+            dict(name='project-test2', result='SUCCESS', changes='2,1'),
+            dict(name='project-test3', result='SUCCESS', changes='2,1'),
+        ], ordered=False)
 
     def test_dynamic_template(self):
         # Tests that a project can't update a template in another
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 71c4ccc83..d26bbf2fc 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -1228,8 +1228,8 @@ class TenantParser(object):
                                                   tenant.config_projects,
                                                   tenant.untrusted_projects,
                                                   cached, tenant)
-        unparsed_config.extend(tenant.config_projects_config, tenant=tenant)
-        unparsed_config.extend(tenant.untrusted_projects_config, tenant=tenant)
+        unparsed_config.extend(tenant.config_projects_config, tenant)
+        unparsed_config.extend(tenant.untrusted_projects_config, tenant)
         tenant.layout = TenantParser._parseLayout(base, tenant,
                                                   unparsed_config,
                                                   scheduler,
@@ -1484,10 +1484,10 @@ class TenantParser(object):
                     (job.project,))
                 if job.config_project:
                     config_projects_config.extend(
-                        job.project.unparsed_config)
+                        job.project.unparsed_config, tenant)
                 else:
                     untrusted_projects_config.extend(
-                        job.project.unparsed_config)
+                        job.project.unparsed_config, tenant)
                 continue
             TenantParser.log.debug("Waiting for cat job %s" % (job,))
             job.wait()
@@ -1518,17 +1518,18 @@ class TenantParser(object):
                     branch = source_context.branch
                     if source_context.trusted:
                         incdata = TenantParser._parseConfigProjectLayout(
-                            job.files[fn], source_context)
-                        config_projects_config.extend(incdata)
+                            job.files[fn], source_context, tenant)
+                        config_projects_config.extend(incdata, tenant)
                     else:
                         incdata = TenantParser._parseUntrustedProjectLayout(
-                            job.files[fn], source_context)
-                        untrusted_projects_config.extend(incdata)
-                    new_project_unparsed_config[project].extend(incdata)
+                            job.files[fn], source_context, tenant)
+                        untrusted_projects_config.extend(incdata, tenant)
+                    new_project_unparsed_config[project].extend(
+                        incdata, tenant)
                     if branch in new_project_unparsed_branch_config.get(
                             project, {}):
                         new_project_unparsed_branch_config[project][branch].\
-                            extend(incdata)
+                            extend(incdata, tenant)
         # Now that we've sucessfully loaded all of the configuration,
         # cache the unparsed data on the project objects.
         for project, data in new_project_unparsed_config.items():
@@ -1540,18 +1541,18 @@ class TenantParser(object):
         return config_projects_config, untrusted_projects_config
 
     @staticmethod
-    def _parseConfigProjectLayout(data, source_context):
+    def _parseConfigProjectLayout(data, source_context, tenant):
         # This is the top-level configuration for a tenant.
         config = model.UnparsedTenantConfig()
         with early_configuration_exceptions(source_context):
-            config.extend(safe_load_yaml(data, source_context))
+            config.extend(safe_load_yaml(data, source_context), tenant)
         return config
 
     @staticmethod
-    def _parseUntrustedProjectLayout(data, source_context):
+    def _parseUntrustedProjectLayout(data, source_context, tenant):
         config = model.UnparsedTenantConfig()
         with early_configuration_exceptions(source_context):
-            config.extend(safe_load_yaml(data, source_context))
+            config.extend(safe_load_yaml(data, source_context), tenant)
         if config.pipelines:
             with configuration_exceptions('pipeline', config.pipelines[0]):
                 raise PipelineNotPermittedError()
@@ -1753,7 +1754,7 @@ class ConfigLoader(object):
                 else:
                     incdata = project.unparsed_branch_config.get(branch)
                 if incdata:
-                    config.extend(incdata)
+                    config.extend(incdata, tenant)
                 continue
             # Otherwise, do not use the cached config (even if the
             # files are empty as that likely means they were deleted).
@@ -1782,12 +1783,12 @@ class ConfigLoader(object):
 
                     if trusted:
                         incdata = TenantParser._parseConfigProjectLayout(
-                            data, source_context)
+                            data, source_context, tenant)
                     else:
                         incdata = TenantParser._parseUntrustedProjectLayout(
-                            data, source_context)
+                            data, source_context, tenant)
 
-                    config.extend(incdata)
+                    config.extend(incdata, tenant)
 
     def createDynamicLayout(self, tenant, files,
                             include_config_projects=False,
diff --git a/zuul/model.py b/zuul/model.py
index dbae1f296..bf69fc4cc 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -2401,7 +2401,7 @@ class UnparsedTenantConfig(object):
         r.semaphores = copy.deepcopy(self.semaphores)
         return r
 
-    def extend(self, conf, tenant=None):
+    def extend(self, conf, tenant):
         if isinstance(conf, UnparsedTenantConfig):
             self.pragmas.extend(conf.pragmas)
             self.pipelines.extend(conf.pipelines)
@@ -2409,16 +2409,14 @@ class UnparsedTenantConfig(object):
             self.project_templates.extend(conf.project_templates)
             for k, v in conf.projects.items():
                 name = k
-                # If we have the tenant add the projects to
-                # the according canonical name instead of the given project
-                # name. If it is not found, it's ok to add this to the given
-                # name. We also don't need to throw the
+                # Add the projects to the according canonical name instead of
+                # the given project name. If it is not found, it's ok to add
+                # this to the given name. We also don't need to throw the
                 # ProjectNotFoundException here as semantic validation occurs
                 # later where it will fail then.
-                if tenant is not None:
-                    trusted, project = tenant.getProject(k)
-                    if project is not None:
-                        name = project.canonical_name
+                trusted, project = tenant.getProject(k)
+                if project is not None:
+                    name = project.canonical_name
                 self.projects.setdefault(name, []).extend(v)
             self.nodesets.extend(conf.nodesets)
             self.secrets.extend(conf.secrets)