Align enqueue/dequeue events with trigger events

EnqueueEvents and DequeueEvents are duck-typed as TriggerEvents
when they are passed to source.getChange.  TriggerEvents are always
created in the context of a connection, so they have a hostname
field and a name field, and name should never have the hostname.

However, our current EnqueueEvents are created directly from
client-side user input and accept any form of project name.  This
means when we get a change from them, we may end up with a Project
that has a canonical name for it's "name" field.  This could not
only cause the enqueue event to be rejected, it could leak project
objects.

To correct this, perform some pre-processing on the input data to
get separate host/name data for the project, and then use that to
construct an Enqueue/DequeueEvent with the same field usage as
a TriggerEvent.

Change-Id: Ib596c282eba3a01c10d28593545e5d238d09de0c
This commit is contained in:
James E. Blair 2021-03-27 08:11:55 -07:00
parent 3095910660
commit abad438e53
5 changed files with 85 additions and 9 deletions

View File

@ -15,6 +15,14 @@
another_gerrit:
Verified: -1
- pipeline:
name: post
manager: independent
trigger:
review_gerrit:
- event: ref-updated
ref: ^refs/heads/.*$
- job:
name: base
parent: null
@ -28,20 +36,33 @@
label: label1
run: playbooks/project-merge.yaml
- job:
name: project-post
run: playbooks/project-merge.yaml
- project:
name: review.example.com/org/project
check:
jobs:
- project-merge
post:
jobs:
- project-post
- project:
name: another.example.com/org/project
check:
jobs:
- project-merge
post:
jobs:
- project-post
- project:
name: common-config
check:
jobs:
- project-merge
post:
jobs:
- project-post

View File

@ -6860,6 +6860,32 @@ class TestAmbiguousProjectNames(ZuulTestCase):
'SUCCESS')
self.assertEqual(r, True)
def test_client_enqueue_ref_canonical(self):
"Test that the RPC client can enqueue a ref using canonical name"
p = "review.example.com/org/project"
upstream = self.getUpstreamRepos([p])
A = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'A')
A.setMerged()
A_commit = str(upstream[p].commit('master'))
self.log.debug("A commit: %s" % A_commit)
client = zuul.rpcclient.RPCClient('127.0.0.1',
self.gearman_server.port)
self.addCleanup(client.shutdown)
r = client.enqueue_ref(
tenant='tenant-one',
pipeline='post',
project='review.example.com/org/project',
trigger=None,
ref='master',
oldrev='90f173846e3af9154517b88543ffbd1691f31366',
newrev=A_commit)
self.waitUntilSettled()
job_names = [x.name for x in self.history]
self.assertEqual(len(self.history), 1)
self.assertIn('project-post', job_names)
self.assertEqual(r, True)
class TestExecutor(ZuulTestCase):
tenant_config_file = 'config/single-tenant/main.yaml'

View File

@ -597,6 +597,7 @@ class GithubEventProcessor(object):
event = DequeueEvent(
dequeue_attrs["tenant"],
dequeue_attrs["pipeline"],
self.connection.canonical_hostname,
project,
change,
ref=None

View File

@ -3634,6 +3634,7 @@ class ChangeManagementEvent(ManagementEvent):
:arg str tenant_name: the name of the tenant
:arg str pipeline_name: the name of the pipeline
:arg str project_hostname: the hostname of the project
:arg str project_name: the name of the project
:arg str change: optional, the change
:arg str ref: optional, the ref
@ -3641,11 +3642,13 @@ class ChangeManagementEvent(ManagementEvent):
:arg str newrev: optional, the new revision
"""
def __init__(self, tenant_name, pipeline_name, project_name,
change=None, ref=None, oldrev=None, newrev=None):
def __init__(self, tenant_name, pipeline_name, project_hostname,
project_name, change=None, ref=None, oldrev=None,
newrev=None):
super().__init__()
self.tenant_name = tenant_name
self.pipeline_name = pipeline_name
self.project_hostname = project_hostname
self.project_name = project_name
self.change = change
if change is not None:
@ -3660,6 +3663,7 @@ class ChangeManagementEvent(ManagementEvent):
d = super().toDict()
d["tenant_name"] = self.tenant_name
d["pipeline_name"] = self.pipeline_name
d["project_hostname"] = self.project_hostname
d["project_name"] = self.project_name
d["change"] = self.change
d["ref"] = self.ref
@ -3672,6 +3676,7 @@ class ChangeManagementEvent(ManagementEvent):
event = cls(
data.get("tenant_name"),
data.get("pipeline_name"),
data.get("project_hostname"),
data.get("project_name"),
data.get("change"),
data.get("ref"),

View File

@ -567,18 +567,39 @@ class Scheduler(threading.Thread):
self.log.debug("Promotion complete")
def dequeue(self, tenant_name, pipeline_name, project_name, change, ref):
event = DequeueEvent(
tenant_name, pipeline_name, project_name, change, ref)
# We need to do some pre-processing here to get the correct
# form of the project hostname and name based on un-known
# inputs.
tenant = self.abide.tenants.get(tenant_name)
if tenant is None:
raise ValueError(f'Unknown tenant {tenant_name}')
(trusted, project) = tenant.getProject(project_name)
if project is None:
raise ValueError(f'Unknown project {project_name}')
event = DequeueEvent(tenant_name, pipeline_name,
project.canonical_hostname, project.name,
change, ref)
result = self.management_events.put(event)
self.log.debug("Waiting for dequeue")
result.wait()
self.log.debug("Dequeue complete")
def enqueue(self, tenant_name, pipeline_name, canonical_project_name,
def enqueue(self, tenant_name, pipeline_name, project_name,
change, ref, oldrev, newrev):
# We need to do some pre-processing here to get the correct
# form of the project hostname and name based on un-known
# inputs.
tenant = self.abide.tenants.get(tenant_name)
if tenant is None:
raise ValueError(f'Unknown tenant {tenant_name}')
(trusted, project) = tenant.getProject(project_name)
if project is None:
raise ValueError(f'Unknown project {project_name}')
event = EnqueueEvent(tenant_name, pipeline_name,
canonical_project_name, change, ref, oldrev,
newrev)
project.canonical_hostname, project.name,
change, ref, oldrev, newrev)
result = self.management_events.put(event)
self.log.debug("Waiting for enqueue")
result.wait()
@ -974,7 +995,8 @@ class Scheduler(threading.Thread):
pipeline = tenant.layout.pipelines.get(event.pipeline_name)
if pipeline is None:
raise ValueError('Unknown pipeline %s' % event.pipeline_name)
(trusted, project) = tenant.getProject(event.project_name)
canonical_name = event.project_hostname + '/' + event.project_name
(trusted, project) = tenant.getProject(canonical_name)
if project is None:
raise ValueError('Unknown project %s' % event.project_name)
change = project.source.getChange(event)
@ -1006,7 +1028,8 @@ class Scheduler(threading.Thread):
pipeline = tenant.layout.pipelines.get(event.pipeline_name)
if pipeline is None:
raise ValueError(f'Unknown pipeline {event.pipeline_name}')
(trusted, project) = tenant.getProject(event.project_name)
canonical_name = event.project_hostname + '/' + event.project_name
(trusted, project) = tenant.getProject(canonical_name)
if project is None:
raise ValueError(f'Unknown project {event.project_name}')
try: