From 9f7c642ae1dc5ac7de1cb0ff5c7e32d6426bd1b3 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Tue, 3 Jul 2018 15:54:54 +0200 Subject: [PATCH] Tolerate missing project Zuul fails to load if the tenant config references an inexistent or inaccessible project. This should not happen and also be added to the loading errors. This is especially important with github where users can freely rename, create and delete repos in their organizations. Change-Id: I99bc50e98c7edfd2767f950d4898ea8298d7ca94 --- tests/fixtures/config/broken/main.yaml | 1 + tests/unit/test_v3.py | 18 ++++++++------ tests/unit/test_web.py | 21 ++++++++++------ zuul/configloader.py | 34 +++++++++++++++++++++++--- zuul/model.py | 17 +++++++++++++ 5 files changed, 73 insertions(+), 18 deletions(-) diff --git a/tests/fixtures/config/broken/main.yaml b/tests/fixtures/config/broken/main.yaml index 14b382fe08..7eacee534d 100644 --- a/tests/fixtures/config/broken/main.yaml +++ b/tests/fixtures/config/broken/main.yaml @@ -7,3 +7,4 @@ untrusted-projects: - org/project - org/project2 + - org/project3 diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index d9700ba31b..c2cf685df2 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -2860,12 +2860,16 @@ class TestBrokenConfig(ZuulTestCase): def test_broken_config_on_startup(self): # verify get the errors at tenant level. tenant = self.sched.abide.tenants.get('tenant-one') + loading_errors = tenant.layout.loading_errors self.assertEquals( - len(tenant.layout.loading_errors), 1, + len(tenant.layout.loading_errors), 2, "An error should have been stored") + self.assertIn( + "Zuul encountered an error while accessing the repo org/project3", + str(loading_errors[0].error)) self.assertIn( "Zuul encountered a syntax error", - str(tenant.layout.loading_errors[0].error)) + str(loading_errors[1].error)) @simple_layout('layouts/broken-template.yaml') def test_broken_config_on_startup_template(self): @@ -2895,7 +2899,7 @@ class TestBrokenConfig(ZuulTestCase): tenant = self.sched.abide.tenants.get('tenant-one') # There is a configuration error self.assertEquals( - len(tenant.layout.loading_errors), 1, + len(tenant.layout.loading_errors), 2, "An error should have been stored") # Inside a broken tenant configuration environment, @@ -2927,7 +2931,7 @@ class TestBrokenConfig(ZuulTestCase): tenant = self.sched.abide.tenants.get('tenant-one') # There is a configuration error self.assertEquals( - len(tenant.layout.loading_errors), 1, + len(tenant.layout.loading_errors), 2, "An error should have been stored") # Inside a broken tenant configuration environment, @@ -2961,7 +2965,7 @@ class TestBrokenConfig(ZuulTestCase): tenant = self.sched.abide.tenants.get('tenant-one') # There is a configuration error self.assertEquals( - len(tenant.layout.loading_errors), 1, + len(tenant.layout.loading_errors), 2, "An error should have been stored") # Inside a broken tenant configuration environment, @@ -2995,7 +2999,7 @@ class TestBrokenConfig(ZuulTestCase): tenant = self.sched.abide.tenants.get('tenant-one') # There is a configuration error self.assertEquals( - len(tenant.layout.loading_errors), 1, + len(tenant.layout.loading_errors), 2, "An error should have been stored") # Inside a broken tenant configuration environment, @@ -3027,7 +3031,7 @@ class TestBrokenConfig(ZuulTestCase): tenant = self.sched.abide.tenants.get('tenant-one') # There is a configuration error self.assertEquals( - len(tenant.layout.loading_errors), 1, + len(tenant.layout.loading_errors), 2, "An error should have been stored") # Inside a broken tenant configuration environment, remove a diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index 58edc49f52..19ec83a07e 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -830,16 +830,23 @@ class TestTenantInfoConfigBroken(BaseTestWeb): config_errors = self.get_url( "api/tenant/tenant-one/config-errors").json() self.assertEqual( - len(config_errors), 1) + len(config_errors), 2) + self.assertEqual( - config_errors[0]['source_context']['project'], 'org/project2') - self.assertEqual( - config_errors[0]['source_context']['branch'], 'master') - self.assertEqual( - config_errors[0]['source_context']['path'], '.zuul.yaml') - self.assertIn('Zuul encountered a syntax error', + config_errors[0]['source_context']['project'], 'org/project3') + self.assertIn('Zuul encountered an error while accessing the repo ' + 'org/project3', config_errors[0]['error']) + self.assertEqual( + config_errors[1]['source_context']['project'], 'org/project2') + self.assertEqual( + config_errors[1]['source_context']['branch'], 'master') + self.assertEqual( + config_errors[1]['source_context']['path'], '.zuul.yaml') + self.assertIn('Zuul encountered a syntax error', + config_errors[1]['error']) + resp = self.get_url("api/tenant/non-tenant/config-errors") self.assertEqual(404, resp.status_code) diff --git a/zuul/configloader.py b/zuul/configloader.py index 725c9aa653..f945a4f2a1 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -195,6 +195,29 @@ def indent(s): return '\n'.join([' ' + x for x in s.split('\n')]) +@contextmanager +def project_configuration_exceptions(context, accumulator): + try: + yield + except ConfigurationSyntaxError: + raise + except Exception as e: + intro = textwrap.fill(textwrap.dedent("""\ + Zuul encountered an error while accessing the repo {repo}. The error + was:""".format( + repo=context.project.name, + ))) + + m = textwrap.dedent("""\ + {intro} + + {error}""") + + m = m.format(intro=intro, + error=indent(str(e))) + accumulator.addError(context, None, m) + + @contextmanager def early_configuration_exceptions(context): try: @@ -1396,13 +1419,16 @@ class TenantParser(object): for tpc in untrusted_tpcs: tenant.addUntrustedProject(tpc) - for tpc in config_tpcs + untrusted_tpcs: - self._getProjectBranches(tenant, tpc) - self._resolveShadowProjects(tenant, tpc) - # We prepare a stack to store config loading issues loading_errors = model.LoadingErrors() + for tpc in config_tpcs + untrusted_tpcs: + source_context = model.ProjectContext(tpc.project) + with project_configuration_exceptions(source_context, + loading_errors): + self._getProjectBranches(tenant, tpc) + self._resolveShadowProjects(tenant, tpc) + # Set default ansible version default_ansible_version = conf.get('default-ansible-version') if default_ansible_version is not None: diff --git a/zuul/model.py b/zuul/model.py index 65ea780ceb..575b50b6d1 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -866,6 +866,23 @@ class SecretUse(ConfigObject): self.pass_to_parent = False +class ProjectContext(ConfigObject): + + def __init__(self, project): + super().__init__() + self.project = project + self.branch = None + self.path = None + + def __str__(self): + return self.project.name + + def toDict(self): + return dict( + project=self.project.name, + ) + + class SourceContext(ConfigObject): """A reference to the branch of a project in configuration.