From 53aa2adffb72b6c4388afa218f42688be4f25c40 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Fri, 22 Dec 2017 09:59:45 +0100 Subject: 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 --- tests/unit/test_v3.py | 21 +++++++++++++++++++-- zuul/configloader.py | 37 +++++++++++++++++++------------------ zuul/model.py | 16 +++++++--------- 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) -- cgit v1.2.1