zuul
/
zuul
Code Issues Proposed changes

Flatten SourceContext data structure 

This stores necessary attributes from a Project directly on the
SourceContext instead of the whole Project.

This will help us to serialize the SourceContext without serialising the
Project as well as the Project also contains references to the
Connection and Source.

This is part of a bigger change to serialize the JobGraph (including all
frozen jobs).

Change-Id: Ib4037da2f7a0f803aca24ce880dbc262375db6a4
This commit is contained in:
Felix Edel 2021-09-29 15:02:45 +02:00 committed by James E. Blair
parent 41e00215ba
commit 08dde23ab5
6 changed files with 120 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
tests/unit/test_configloader.py
View File

@ -101,7 +101,9 @@ class TestTenantSimple(TenantParserTestCase):
""")
tenant = self.scheds.first.sched.abide.tenants.get('tenant-one')
project = tenant.config_projects[0]
source_context = SourceContext(project, 'master', 'zuul.yaml', True)
source_context = SourceContext(
project.canonical_name, project.name, project.connection_name,
'master', 'zuul.yaml', True)
data = safe_load_yaml(to_parse, source_context)
self.assertEqual(len(data), 3)
@ -479,12 +481,12 @@ class TestSplitConfig(ZuulTestCase):
self.assertIn('project-test1', tenant.layout.jobs)
self.assertIn('project-test2', tenant.layout.jobs)
test1 = tenant.layout.getJob('project-test1')
self.assertEqual(test1.source_context.project.name, 'common-config')
self.assertEqual(test1.source_context.project_name, 'common-config')
self.assertEqual(test1.source_context.branch, 'master')
self.assertEqual(test1.source_context.path, 'zuul.d/jobs.yaml')
self.assertEqual(test1.source_context.trusted, True)
test2 = tenant.layout.getJob('project-test2')
self.assertEqual(test2.source_context.project.name, 'common-config')
self.assertEqual(test2.source_context.project_name, 'common-config')
self.assertEqual(test2.source_context.branch, 'master')
self.assertEqual(test2.source_context.path, 'zuul.d/more-jobs.yaml')
self.assertEqual(test2.source_context.trusted, True)

20
tests/unit/test_model.py
View File

@ -53,10 +53,12 @@ class TestJob(BaseTestCase):
self.layout = model.Layout(self.tenant)
self.tenant.layout = self.layout
self.project = model.Project('project', self.source)
self.context = model.SourceContext(self.project, 'master',
'test', True)
self.untrusted_context = model.SourceContext(self.project, 'master',
'test', False)
self.context = model.SourceContext(
self.project.canonical_name, self.project.name,
self.project.connection_name, 'master', 'test', True)
self.untrusted_context = model.SourceContext(
self.project.canonical_name, self.project.name,
self.project.connection_name, 'master', 'test', False)
self.tpc = model.TenantProjectConfig(self.project)
self.tenant.addUntrustedProject(self.tpc)
self.pipeline = model.Pipeline('gate', self.tenant)
@ -275,8 +277,9 @@ class TestJob(BaseTestCase):
def test_job_source_project(self):
base_project = model.Project('base_project', self.source)
base_context = model.SourceContext(base_project, 'master',
'test', True)
base_context = model.SourceContext(
base_project.canonical_name, base_project.name,
base_project.connection_name, 'master', 'test', True)
tpc = model.TenantProjectConfig(base_project)
self.tenant.addUntrustedProject(tpc)
@ -289,8 +292,9 @@ class TestJob(BaseTestCase):
self.layout.addJob(base)
other_project = model.Project('other_project', self.source)
other_context = model.SourceContext(other_project, 'master',
'test', True)
other_context = model.SourceContext(
other_project.canonical_name, other_project.name,
other_project.connection_name, 'master', 'test', True)
tpc = model.TenantProjectConfig(other_project)
self.tenant.addUntrustedProject(tpc)
base2 = self.pcontext.job_parser.fromYaml({

71
zuul/configloader.py
View File

@ -172,7 +172,7 @@ class YAMLDuplicateKeyError(ConfigurationSyntaxError):
intro = textwrap.fill(textwrap.dedent("""\
Zuul encountered a syntax error while parsing its configuration in the
repo {repo} on branch {branch}. The error was:""".format(
repo=context.project.name,
repo=context.project_name,
branch=context.branch,
)))
@ -215,7 +215,7 @@ def project_configuration_exceptions(context, accumulator):
intro = textwrap.fill(textwrap.dedent("""\
Zuul encountered an error while accessing the repo {repo}. The error
was:""".format(
repo=context.project.name,
repo=context.project_name,
)))
m = textwrap.dedent("""\
@ -238,7 +238,7 @@ def early_configuration_exceptions(context):
intro = textwrap.fill(textwrap.dedent("""\
Zuul encountered a syntax error while parsing its configuration in the
repo {repo} on branch {branch}. The error was:""".format(
repo=context.project.name,
repo=context.project_name,
branch=context.branch,
)))
@ -265,7 +265,7 @@ def configuration_exceptions(stanza, conf, accumulator):
intro = textwrap.fill(textwrap.dedent("""\
Zuul encountered a syntax error while parsing its configuration in the
repo {repo} on branch {branch}. The error was:""".format(
repo=context.project.name,
repo=context.project_name,
branch=context.branch,
)))
@ -301,7 +301,7 @@ def reference_exceptions(stanza, obj, accumulator):
intro = textwrap.fill(textwrap.dedent("""\
Zuul encountered a syntax error while parsing its configuration in the
repo {repo} on branch {branch}. The error was:""".format(
repo=context.project.name,
repo=context.project_name,
branch=context.branch,
)))
@ -400,7 +400,7 @@ repo {repo} on branch {branch}. The error was:
{error}
"""
m = m.format(repo=context.project.name,
m = m.format(repo=context.project_name,
branch=context.branch,
error=str(e))
raise ConfigurationSyntaxError(m)
@ -722,7 +722,7 @@ class JobParser(object):
if secrets and not conf['_source_context'].trusted:
job.post_review = True
job.allowed_projects = frozenset((
conf['_source_context'].project.name,))
conf['_source_context'].project_name,))
if (conf.get('timeout') and
self.pcontext.tenant.max_job_timeout != -1 and
@ -934,11 +934,11 @@ class JobParser(object):
project.canonical_name)
def _makeImplicitRole(self, job):
project = job.source_context.project
name = project.name.split('/')[-1]
project_name = job.source_context.project_name
name = project_name.split('/')[-1]
name = JobParser.ANSIBLE_ROLE_RE.sub('', name) or name
return model.ZuulRole(name,
project.canonical_name,
job.source_context.project_canonical_name,
implicit=True)
@ -1072,15 +1072,16 @@ class ProjectParser(object):
self.schema(conf)
project_name = conf.get('name')
source_context = conf['_source_context']
if not project_name:
# There is no name defined so implicitly add the name
# of the project where it is defined.
project_name = (conf['_source_context'].project.canonical_name)
project_name = (source_context.project_canonical_name)
if project_name.startswith('^'):
# regex matching is designed to match other projects so disallow
# in untrusted contexts
if not conf['_source_context'].trusted:
if not source_context.trusted:
raise ProjectNotPermittedError()
# Parse the project as a template since they're mostly the
@ -1094,8 +1095,9 @@ class ProjectParser(object):
if project is None:
raise ProjectNotFoundError(project_name)
if not conf['_source_context'].trusted:
if project != conf['_source_context'].project:
if not source_context.trusted:
if project.canonical_name != \
source_context.project_canonical_name:
raise ProjectNotPermittedError()
# Parse the project as a template since they're mostly the
@ -1109,11 +1111,11 @@ class ProjectParser(object):
# branch matchers for arbitrary branches, but project
# stanzas should not. They should either have the current
# branch or no branch matcher.
if conf['_source_context'].trusted:
if source_context.trusted:
project_config.setImpliedBranchMatchers([])
else:
project_config.setImpliedBranchMatchers(
[conf['_source_context'].branch])
[source_context.branch])
# Add templates
for name in conf.get('templates', []):
@ -1460,7 +1462,7 @@ class ParseContext(object):
# If this project only has one branch, don't create implied
# branch matchers. This way central job repos can work.
branches = self.tenant.getProjectBranches(
source_context.project)
source_context.project_canonical_name)
if len(branches) == 1:
return None
@ -1575,7 +1577,8 @@ class TenantParser(object):
loading_errors = model.LoadingErrors()
for tpc in config_tpcs + untrusted_tpcs:
source_context = model.ProjectContext(tpc.project)
source_context = model.ProjectContext(
tpc.project.canonical_name, tpc.project.name)
with project_configuration_exceptions(source_context,
loading_errors):
self._getProjectBranches(tenant, tpc)
@ -1633,7 +1636,7 @@ class TenantParser(object):
_, project = tenant.getProject(sp)
if project is None:
raise ProjectNotFoundError(sp)
shadow_projects.append(project)
shadow_projects.append(project.canonical_name)
tpc.shadow_projects = frozenset(shadow_projects)
def _getProjectBranches(self, tenant, tpc):
@ -1770,7 +1773,7 @@ class TenantParser(object):
# branch. Remember the branch and then implicitly add a
# branch selector to each job there. This makes the
# in-repo configuration apply only to that branch.
branches = tenant.getProjectBranches(project)
branches = tenant.getProjectBranches(project.canonical_name)
for branch in branches:
if not tpc.load_classes:
# If all config classes are excluded then do not
@ -1778,7 +1781,8 @@ class TenantParser(object):
continue
source_context = model.SourceContext(
project, branch, '', False)
project.canonical_name, project.name,
project.connection_name, branch, '', False)
if min_ltimes is not None:
files_cache = self.unparsed_config_cache.getFilesCache(
project.canonical_name, branch)
@ -1848,10 +1852,10 @@ class TenantParser(object):
# Save all config files in Zookeeper (not just for the current tpc)
files_cache = self.unparsed_config_cache.getFilesCache(
job.source_context.project.canonical_name,
job.source_context.project_canonical_name,
job.source_context.branch)
with self.unparsed_config_cache.writeLock(
job.source_context.project.canonical_name):
job.source_context.project_canonical_name):
# Since the cat job returns all required config files
# for ALL tenants the project is a part of, we can
# clear the whole cache and then populate it with the
@ -1868,12 +1872,12 @@ class TenantParser(object):
def _updateUnparsedBranchCache(self, abide, tenant, source_context, files,
loading_errors, ltime):
loaded = False
tpc = tenant.project_configs[source_context.project.canonical_name]
tpc = tenant.project_configs[source_context.project_canonical_name]
# Make sure we are clearing the local cache before updating it.
abide.clearUnparsedBranchCache(source_context.project.canonical_name,
abide.clearUnparsedBranchCache(source_context.project_canonical_name,
source_context.branch)
branch_cache = abide.getUnparsedBranchCache(
source_context.project.canonical_name,
source_context.project_canonical_name,
source_context.branch)
for conf_root in (
('zuul.yaml', 'zuul.d', '.zuul.yaml', '.zuul.d') +
@ -1921,7 +1925,7 @@ class TenantParser(object):
config_projects_config.extend(unparsed_branch_config)
for project in tenant.untrusted_projects:
branches = tenant.getProjectBranches(project)
branches = tenant.getProjectBranches(project.canonical_name)
for branch in branches:
branch_cache = abide.getUnparsedBranchCache(
project.canonical_name, branch)
@ -1956,8 +1960,8 @@ class TenantParser(object):
return data.copy(trusted=False)
def _getLoadClasses(self, tenant, conf_object):
project = conf_object.get('_source_context').project
tpc = tenant.project_configs[project.canonical_name]
project = conf_object.get('_source_context').project_canonical_name
tpc = tenant.project_configs[project]
return tpc.load_classes
def parseConfig(self, tenant, unparsed_config, loading_errors, pcontext):
@ -2059,7 +2063,7 @@ class TenantParser(object):
def cacheConfig(self, tenant, parsed_config):
def _cache(attr, obj):
tpc = tenant.project_configs[
obj.source_context.project.canonical_name]
obj.source_context.project_canonical_name]
branch_cache = tpc.parsed_branch_config.get(
obj.source_context.branch)
if branch_cache is None:
@ -2398,7 +2402,7 @@ class ConfigLoader(object):
else:
# Use the cached branch list; since this is a dynamic
# reconfiguration there should not be any branch changes.
branches = tenant.getProjectBranches(project)
branches = tenant.getProjectBranches(project.canonical_name)
for branch in branches:
fns1 = []
@ -2437,8 +2441,9 @@ class ConfigLoader(object):
data = files.getFile(project.source.connection.connection_name,
project.name, branch, fn)
if data:
source_context = model.SourceContext(project, branch,
fn, trusted)
source_context = model.SourceContext(
project.canonical_name, project.name,
project.connection_name, branch, fn, trusted)
# Prevent mixing configuration source
conf_root = fn.split('/')[0]

5
zuul/manager/__init__.py
View File

@ -840,7 +840,7 @@ class PipelineManager(metaclass=ABCMeta):
for err in layout.loading_errors.errors:
econtext = err.key.context
if ((err.key not in parent_error_keys) or
(econtext.project.name == item.change.project.name and
(econtext.project_name == item.change.project.name and
econtext.branch == item.change.branch)):
relevant_errors.append(err)
return relevant_errors
@ -1035,7 +1035,8 @@ class PipelineManager(metaclass=ABCMeta):
# Add all protected branches of all involved projects
for project in projects:
branches.update(tenant.getProjectBranches(project))
branches.update(
tenant.getProjectBranches(project.canonical_name))
# Additionally add all target branches of all involved items.
branches.update(item.change.branch for item in items

105
zuul/model.py
View File

@ -114,7 +114,7 @@ class ConfigurationErrorKey(object):
elements = []
if context:
elements.extend([
context.project.canonical_name,
context.project_canonical_name,
context.branch,
context.path,
])
@ -1029,18 +1029,19 @@ class FrozenSecret(ConfigObject):
class ProjectContext(ConfigObject):
def __init__(self, project):
def __init__(self, project_canonical_name, project_name):
super().__init__()
self.project = project
self.project_canonical_name = project_canonical_name
self.project_name = project_name
self.branch = None
self.path = None
def __str__(self):
return self.project.name
return self.project_name
def toDict(self):
return dict(
project=self.project.name,
project=self.project_name,
)
@ -1050,9 +1051,14 @@ class SourceContext(ConfigObject):
Jobs and playbooks reference this to keep track of where they
originate."""
def __init__(self, project, branch, path, trusted):
def __init__(self, project_canonical_name, project_name,
project_connection_name, branch, path, trusted):
super(SourceContext, self).__init__()
self.project = project
# TODO (felix): Would it be enough to only store the project's
# canonical name?
self.project_canonical_name = project_canonical_name
self.project_name = project_name
self.project_connection_name = project_connection_name
self.branch = branch
self.path = path
self.trusted = trusted
@ -1060,7 +1066,8 @@ class SourceContext(ConfigObject):
self.implied_branches = None
def __str__(self):
return '%s/%s@%s' % (self.project, self.path, self.branch)
return '%s/%s@%s' % (
self.project_name, self.path, self.branch)
def __repr__(self):
return '<SourceContext %s trusted:%s>' % (str(self),
@ -1070,13 +1077,14 @@ class SourceContext(ConfigObject):
return self.copy()
def copy(self):
return self.__class__(self.project, self.branch, self.path,
self.trusted)
return self.__class__(
self.project_canonical_name, self.project_name,
self.project_connection_name, self.branch, self.path, self.trusted)
def isSameProject(self, other):
if not isinstance(other, SourceContext):
return False
return (self.project == other.project and
return (self.project_canonical_name == other.project_canonical_name and
self.trusted == other.trusted)
def __ne__(self, other):
@ -1085,14 +1093,14 @@ class SourceContext(ConfigObject):
def __eq__(self, other):
if not isinstance(other, SourceContext):
return False
return (self.project == other.project and
return (self.project_canonical_name == other.project_canonical_name and
self.branch == other.branch and
self.path == other.path and
self.trusted == other.trusted)
def toDict(self):
return dict(
project=self.project.name,
project=self.project_name,
branch=self.branch,
path=self.path,
)
@ -1160,8 +1168,10 @@ class PlaybookContext(ConfigObject):
"defined in the same project in which they "
"are used".format(
name=secret_use.name))
project = layout.tenant.getProject(
self.source_context.project_canonical_name)[1]
# Decrypt a copy of the secret to verify it can be done
secret.decrypt(self.source_context.project.private_secrets_key)
secret.decrypt(project.private_secrets_key)
def freezeSecrets(self, layout):
secrets = []
@ -1171,10 +1181,10 @@ class PlaybookContext(ConfigObject):
encrypted_secret_data = secret.serialize()
# Use *our* project, not the secret's, because we want to decrypt
# with *our* key.
connection_name = self.source_context.project.connection_name
project_name = self.source_context.project.name
project = layout.tenant.getProject(
self.source_context.project_canonical_name)[1]
secrets.append(FrozenSecret.construct_cached(
connection_name, project_name, secret_name,
project.connection_name, project.name, secret_name,
encrypted_secret_data))
self.frozen_secrets = tuple(secrets)
@ -1193,8 +1203,8 @@ class PlaybookContext(ConfigObject):
else:
secrets[secret.name] = secret.toDict()
return dict(
connection=self.source_context.project.connection_name,
project=self.source_context.project.name,
connection=self.source_context.project_connection_name,
project=self.source_context.project_name,
branch=self.source_context.branch,
trusted=self.source_context.trusted,
roles=[r.toDict() for r in self.roles],
@ -1726,7 +1736,7 @@ class Job(ConfigObject):
if self.protected_origin:
# this is a protected job, check origin of job definition
this_origin = self.protected_origin
other_origin = other.source_context.project.canonical_name
other_origin = other.source_context.project_canonical_name
if this_origin != other_origin:
raise Exception("Job %s which is defined in %s is "
"protected and cannot be inherited "
@ -1773,7 +1783,7 @@ class Job(ConfigObject):
repr(self), repr(other)))
if not self.protected_origin:
self.protected_origin = \
other.source_context.project.canonical_name
other.source_context.project_canonical_name
# We must update roles before any playbook contexts
if other._get('roles') is not None:
@ -1794,8 +1804,8 @@ class Job(ConfigObject):
encrypted_secret_data = secret.serialize()
# Use the other project, not the secret's, because we
# want to decrypt with the other project's key key.
connection_name = other.source_context.project.connection_name
project_name = other.source_context.project.name
connection_name = other.source_context.project_connection_name
project_name = other.source_context.project_name
frozen_secrets.append(FrozenSecret.construct_cached(
connection_name, project_name,
secret_name, encrypted_secret_data))
@ -1886,7 +1896,7 @@ class Job(ConfigObject):
for playbook in playbooks:
# noop job does not have source_context
if playbook.source_context:
yield playbook.source_context.project.canonical_name
yield playbook.source_context.project_canonical_name
for role in playbook.roles:
if role.implicit and not with_implicit:
continue
@ -4915,7 +4925,9 @@ class UnparsedConfig(object):
setattr(r, attr, new_objlist)
for i, new_obj in enumerate(new_objlist):
old_obj = old_objlist[i]
key = (old_obj['_source_context'].project,
key = (old_obj['_source_context'].project_canonical_name,
old_obj['_source_context'].project_name,
old_obj['_source_context'].project_connection_name,
old_obj['_source_context'].branch,
old_obj['_source_context'].path)
new_sc = source_contexts.get(key)
@ -5070,16 +5082,16 @@ class Layout(object):
# We can have multiple variants of a job all with the same
# name, but these variants must all be defined in the same repo.
prior_jobs = [j for j in self.getJobs(job.name) if
j.source_context.project !=
job.source_context.project]
j.source_context.project_canonical_name !=
job.source_context.project_canonical_name]
# Unless the repo is permitted to shadow another. If so, and
# the job we are adding is from a repo that is permitted to
# shadow the one with the older jobs, skip adding this job.
job_project = job.source_context.project
job_tpc = self.tenant.project_configs[job_project.canonical_name]
job_project = job.source_context.project_canonical_name
job_tpc = self.tenant.project_configs[job_project]
skip_add = False
for prior_job in prior_jobs[:]:
prior_project = prior_job.source_context.project
prior_project = prior_job.source_context.project_canonical_name
if prior_project in job_tpc.shadow_projects:
prior_jobs.remove(prior_job)
skip_add = True
@ -5088,9 +5100,9 @@ class Layout(object):
raise Exception("Job %s in %s is not permitted to shadow "
"job %s in %s" % (
job,
job.source_context.project,
job.source_context.project_name,
prior_jobs[0],
prior_jobs[0].source_context.project))
prior_jobs[0].source_context.project_name))
if skip_add:
return False
if job.name in self.jobs:
@ -5106,8 +5118,9 @@ class Layout(object):
other = self.nodesets.get(nodeset.name)
if other is not None:
if not nodeset.source_context.isSameProject(other.source_context):
raise Exception("Nodeset %s already defined in project %s" %
(nodeset.name, other.source_context.project))
raise Exception(
"Nodeset %s already defined in project %s" %
(nodeset.name, other.source_context.project_name))
if nodeset.source_context.branch == other.source_context.branch:
raise Exception("Nodeset %s already defined" % (nodeset.name,))
if nodeset != other:
@ -5126,8 +5139,9 @@ class Layout(object):
other = self.secrets.get(secret.name)
if other is not None:
if not secret.source_context.isSameProject(other.source_context):
raise Exception("Secret %s already defined in project %s" %
(secret.name, other.source_context.project))
raise Exception(
"Secret %s already defined in project %s" %
(secret.name, other.source_context.project_name))
if secret.source_context.branch == other.source_context.branch:
raise Exception("Secret %s already defined" % (secret.name,))
if not secret.areDataEqual(other):
@ -5147,8 +5161,9 @@ class Layout(object):
if other is not None:
if not semaphore.source_context.isSameProject(
other.source_context):
raise Exception("Semaphore %s already defined in project %s" %
(semaphore.name, other.source_context.project))
raise Exception(
"Semaphore %s already defined in project %s" %
(semaphore.name, other.source_context.project_name))
if semaphore.source_context.branch == other.source_context.branch:
raise Exception("Semaphore %s already defined" %
(semaphore.name,))
@ -5185,8 +5200,8 @@ class Layout(object):
template_list = self.project_templates.get(project_template.name)
if template_list is not None:
reference = template_list[0]
if (reference.source_context.project !=
project_template.source_context.project):
if (reference.source_context.project_canonical_name !=
project_template.source_context.project_canonical_name):
raise Exception("Project template %s is already defined" %
(project_template.name,))
else:
@ -5313,11 +5328,11 @@ class Layout(object):
project = None
for variant in self.getJobs(jobname):
if project is None and variant.source_context:
project = variant.source_context.project
project = variant.source_context.project_canonical_name
if override_checkouts.get(None) is not None:
override_branch = override_checkouts.get(None)
override_branch = override_checkouts.get(
project.canonical_name, override_branch)
project, override_branch)
branches = self.tenant.getProjectBranches(project)
if override_branch not in branches:
override_branch = None
@ -5703,16 +5718,16 @@ class Tenant(object):
(project,))
return result
def getProjectBranches(self, project):
def getProjectBranches(self, project_canonical_name):
"""Return a project's branches (filtered by this tenant config)
:arg Project project: The project object.
:arg str project_canonical: The project's canonical name.
:returns: A list of branch names.
:rtype: [str]
"""
tpc = self.project_configs[project.canonical_name]
tpc = self.project_configs[project_canonical_name]
return tpc.branches
def getExcludeUnprotectedBranches(self, project):

3
zuul/reporter/__init__.py
View File

@ -65,7 +65,8 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
mark = err.key.mark
if not (context and mark and err.short_error):
continue
if context.project != item.change.project:
if context.project_canonical_name != \
item.change.project.canonical_name:
continue
if not hasattr(item.change, 'branch'):
continue