diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 9bd405e2da..f10a82d650 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -30,14 +30,15 @@ class TestJob(BaseTestCase): def setUp(self): super(TestJob, self).setUp() self.project = model.Project('project', None) - self.context = model.SourceContext(self.project, 'master', True) + self.context = model.SourceContext(self.project, 'master', + 'test', True) @property def job(self): tenant = model.Tenant('tenant') layout = model.Layout() project = model.Project('project', None) - context = model.SourceContext(project, 'master', True) + context = model.SourceContext(project, 'master', 'test', True) job = configloader.JobParser.fromYaml(tenant, layout, { '_source_context': context, 'name': 'job', @@ -142,7 +143,7 @@ class TestJob(BaseTestCase): layout.addPipeline(pipeline) queue = model.ChangeQueue(pipeline) project = model.Project('project', None) - context = model.SourceContext(project, 'master', True) + context = model.SourceContext(project, 'master', 'test', True) base = configloader.JobParser.fromYaml(tenant, layout, { '_source_context': context, @@ -296,7 +297,7 @@ class TestJob(BaseTestCase): tenant = model.Tenant('tenant') layout = model.Layout() project = model.Project('project', None) - context = model.SourceContext(project, 'master', True) + context = model.SourceContext(project, 'master', 'test', True) base = configloader.JobParser.fromYaml(tenant, layout, { '_source_context': context, @@ -381,7 +382,7 @@ class TestJob(BaseTestCase): layout.addPipeline(pipeline) queue = model.ChangeQueue(pipeline) project = model.Project('project', None) - context = model.SourceContext(project, 'master', True) + context = model.SourceContext(project, 'master', 'test', True) base = configloader.JobParser.fromYaml(tenant, layout, { '_source_context': context, @@ -454,7 +455,7 @@ class TestJob(BaseTestCase): layout.addPipeline(pipeline) queue = model.ChangeQueue(pipeline) project = model.Project('project', None) - context = model.SourceContext(project, 'master', True) + context = model.SourceContext(project, 'master', 'test', True) base = configloader.JobParser.fromYaml(tenant, layout, { '_source_context': context, @@ -498,7 +499,8 @@ class TestJob(BaseTestCase): tenant = model.Tenant('tenant') layout = model.Layout() base_project = model.Project('base_project', None) - base_context = model.SourceContext(base_project, 'master', True) + base_context = model.SourceContext(base_project, 'master', + 'test', True) base = configloader.JobParser.fromYaml(tenant, layout, { '_source_context': base_context, @@ -507,7 +509,8 @@ class TestJob(BaseTestCase): layout.addJob(base) other_project = model.Project('other_project', None) - other_context = model.SourceContext(other_project, 'master', True) + other_context = model.SourceContext(other_project, 'master', + 'test', True) base2 = configloader.JobParser.fromYaml(tenant, layout, { '_source_context': other_context, 'name': 'base', diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index ea7e85a974..e566479e5a 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -227,6 +227,26 @@ class TestInRepoConfig(ZuulTestCase): self.assertIn('syntax error', A.messages[1], "A should have a syntax error reported") + def test_untrusted_yaml_error(self): + in_repo_conf = textwrap.dedent( + """ + - job: + foo: error + """) + + file_dict = {'.zuul.yaml': in_repo_conf} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('code-review', 2) + self.fake_gerrit.addEvent(A.addApproval('approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertEqual(A.reported, 2, + "A should report start and failure") + self.assertIn('syntax error', A.messages[1], + "A should have a syntax error reported") + class TestAnsible(AnsibleZuulTestCase): # A temporary class to hold new tests while others are disabled diff --git a/zuul/configloader.py b/zuul/configloader.py index 918997ca08..3dbe2c3215 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -69,6 +69,31 @@ The offending content was a {stanza} stanza with the content: raise ConfigurationSyntaxError(m) +class ZuulSafeLoader(yaml.SafeLoader): + def __init__(self, stream, context): + super(ZuulSafeLoader, self).__init__(stream) + self.name = str(context) + + +def safe_load_yaml(stream, context): + loader = ZuulSafeLoader(stream, context) + try: + return loader.get_single_data() + except yaml.YAMLError as e: + m = """ +Zuul encountered a syntax error while parsing its configuration in the +repo {repo} on branch {branch}. The error was: + + {error} +""" + m = m.format(repo=context.project.name, + branch=context.branch, + error=str(e)) + raise ConfigurationSyntaxError(m) + finally: + loader.dispose() + + class NodeSetParser(object): @staticmethod def getSchema(): @@ -695,7 +720,8 @@ class TenantParser(object): url = source.getGitUrl(project) job = merger.getFiles(project.name, url, 'master', files=['zuul.yaml', '.zuul.yaml']) - job.source_context = model.SourceContext(project, 'master', True) + job.source_context = model.SourceContext(project, 'master', + '', True) jobs.append(job) for (source, project) in project_repos: @@ -721,8 +747,8 @@ class TenantParser(object): model.UnparsedTenantConfig() job = merger.getFiles(project.name, url, branch, files=['.zuul.yaml']) - job.source_context = model.SourceContext(project, - branch, False) + job.source_context = model.SourceContext( + project, branch, '', False) jobs.append(job) for job in jobs: @@ -732,11 +758,20 @@ class TenantParser(object): # This is important for correct inheritance. TenantParser.log.debug("Waiting for cat job %s" % (job,)) job.wait() + loaded = False for fn in ['zuul.yaml', '.zuul.yaml']: if job.files.get(fn): + # Don't load from more than one file in a repo-branch + if loaded: + TenantParser.log.warning( + "Multiple configuration files in %s" % + (job.source_context,)) + continue + loaded = True + job.source_context.path = fn TenantParser.log.info( - "Loading configuration from %s/%s" % - (job.source_context, fn)) + "Loading configuration from %s" % + (job.source_context,)) project = job.source_context.project branch = job.source_context.branch if job.source_context.trusted: @@ -756,7 +791,7 @@ class TenantParser(object): def _parseConfigRepoLayout(data, source_context): # This is the top-level configuration for a tenant. config = model.UnparsedTenantConfig() - config.extend(yaml.safe_load(data), source_context) + config.extend(safe_load_yaml(data, source_context), source_context) return config @staticmethod @@ -764,8 +799,7 @@ class TenantParser(object): # TODOv3(jeblair): this should implement some rules to protect # aspects of the config that should not be changed in-repo config = model.UnparsedTenantConfig() - config.extend(yaml.safe_load(data), source_context) - + config.extend(safe_load_yaml(data, source_context), source_context) return config @staticmethod @@ -846,12 +880,14 @@ class ConfigLoader(object): for branch in source.getProjectBranches(project): data = None if config_repo: - data = files.getFile(project.name, branch, 'zuul.yaml') + fn = 'zuul.yaml' + data = files.getFile(project.name, branch, fn) if not data: - data = files.getFile(project.name, branch, '.zuul.yaml') + fn = '.zuul.yaml' + data = files.getFile(project.name, branch, fn) if data: source_context = model.SourceContext(project, branch, - config_repo) + fn, config_repo) if config_repo: incdata = TenantParser._parseConfigRepoLayout( data, source_context) diff --git a/zuul/model.py b/zuul/model.py index 19931ea798..15529e3083 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -535,21 +535,25 @@ class SourceContext(object): Jobs and playbooks reference this to keep track of where they originate.""" - def __init__(self, project, branch, trusted): + def __init__(self, project, branch, path, trusted): self.project = project self.branch = branch + self.path = path self.trusted = trusted + def __str__(self): + return '%s/%s@%s' % (self.project, self.path, self.branch) + def __repr__(self): - return '' % (self.project, - self.branch, - self.trusted) + return '' % (str(self), + self.trusted) def __deepcopy__(self, memo): return self.copy() def copy(self): - return self.__class__(self.project, self.branch, self.trusted) + return self.__class__(self.project, self.branch, self.path, + self.trusted) def __ne__(self, other): return not self.__eq__(other) @@ -559,6 +563,7 @@ class SourceContext(object): return False return (self.project == other.project and self.branch == other.branch and + self.path == other.path and self.trusted == other.trusted)