Merge "Limit search scope of getChangesDependingOn to tenant"
This commit is contained in:
commit
f798ccd935
0
tests/fixtures/config/zuultrigger/parent-change-enqueued-github/git/org2_project/README
vendored
Normal file
0
tests/fixtures/config/zuultrigger/parent-change-enqueued-github/git/org2_project/README
vendored
Normal file
7
tests/fixtures/config/zuultrigger/parent-change-enqueued-github/git/org2_project/zuul.yaml
vendored
Normal file
7
tests/fixtures/config/zuultrigger/parent-change-enqueued-github/git/org2_project/zuul.yaml
vendored
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
- project:
|
||||||
|
check:
|
||||||
|
jobs:
|
||||||
|
- project-check
|
||||||
|
gate:
|
||||||
|
jobs:
|
||||||
|
- project-gate
|
|
@ -61,12 +61,3 @@
|
||||||
- job:
|
- job:
|
||||||
name: project-gate
|
name: project-gate
|
||||||
run: playbooks/project-gate.yaml
|
run: playbooks/project-gate.yaml
|
||||||
|
|
||||||
- project:
|
|
||||||
name: org/project
|
|
||||||
check:
|
|
||||||
jobs:
|
|
||||||
- project-check
|
|
||||||
gate:
|
|
||||||
jobs:
|
|
||||||
- project-gate
|
|
||||||
|
|
7
tests/fixtures/config/zuultrigger/parent-change-enqueued-github/git/org_project/zuul.yaml
vendored
Normal file
7
tests/fixtures/config/zuultrigger/parent-change-enqueued-github/git/org_project/zuul.yaml
vendored
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
- project:
|
||||||
|
check:
|
||||||
|
jobs:
|
||||||
|
- project-check
|
||||||
|
gate:
|
||||||
|
jobs:
|
||||||
|
- project-gate
|
|
@ -6,3 +6,12 @@
|
||||||
- org/common-config
|
- org/common-config
|
||||||
untrusted-projects:
|
untrusted-projects:
|
||||||
- org/project
|
- org/project
|
||||||
|
|
||||||
|
- tenant:
|
||||||
|
name: tenant-two
|
||||||
|
source:
|
||||||
|
github:
|
||||||
|
config-projects:
|
||||||
|
- org/common-config
|
||||||
|
untrusted-projects:
|
||||||
|
- org2/project
|
||||||
|
|
|
@ -115,6 +115,13 @@ class TestZuulTriggerParentChangeEnqueuedGithub(ZuulGithubAppTestCase):
|
||||||
B1.addLabel('for-check') # should go to check
|
B1.addLabel('for-check') # should go to check
|
||||||
B2.addLabel('for-gate') # should go to gate
|
B2.addLabel('for-gate') # should go to gate
|
||||||
|
|
||||||
|
# In this case we have two installations
|
||||||
|
# 1: org/common-config, org/project (used by tenant-one and tenant-two)
|
||||||
|
# 2: org2/project (only used by tenant-two)
|
||||||
|
# In order to track accesses to the installations enable client
|
||||||
|
# recording in the fake github.
|
||||||
|
self.fake_github.record_clients = True
|
||||||
|
|
||||||
self.fake_github.emitEvent(A.getReviewAddedEvent('approved'))
|
self.fake_github.emitEvent(A.getReviewAddedEvent('approved'))
|
||||||
# Jobs are being held in build to make sure that 3,1 has time
|
# Jobs are being held in build to make sure that 3,1 has time
|
||||||
# to enqueue behind 1,1 so that the test is more
|
# to enqueue behind 1,1 so that the test is more
|
||||||
|
@ -138,6 +145,7 @@ class TestZuulTriggerParentChangeEnqueuedGithub(ZuulGithubAppTestCase):
|
||||||
# Now directly enqueue a change into the check. As no pipeline reacts
|
# Now directly enqueue a change into the check. As no pipeline reacts
|
||||||
# on parent-change-enqueued from pipeline check no
|
# on parent-change-enqueued from pipeline check no
|
||||||
# parent-change-enqueued event is expected.
|
# parent-change-enqueued event is expected.
|
||||||
|
self.waitUntilSettled()
|
||||||
zuultrigger_event_count = 0
|
zuultrigger_event_count = 0
|
||||||
|
|
||||||
def counting_put(*args, **kwargs):
|
def counting_put(*args, **kwargs):
|
||||||
|
@ -163,6 +171,13 @@ class TestZuulTriggerParentChangeEnqueuedGithub(ZuulGithubAppTestCase):
|
||||||
self.assertEqual(len(self.history), 4)
|
self.assertEqual(len(self.history), 4)
|
||||||
self.assertEqual(zuultrigger_event_count, 0)
|
self.assertEqual(zuultrigger_event_count, 0)
|
||||||
|
|
||||||
|
# After starting recording installation containing org2/project
|
||||||
|
# should not be contacted
|
||||||
|
inst_id_to_check = self.fake_github.installation_map['org2/project']
|
||||||
|
inst_clients = [x for x in self.fake_github.recorded_clients
|
||||||
|
if x._inst_id == inst_id_to_check]
|
||||||
|
self.assertEqual(len(inst_clients), 0)
|
||||||
|
|
||||||
|
|
||||||
class TestZuulTriggerProjectChangeMerged(ZuulTestCase):
|
class TestZuulTriggerProjectChangeMerged(ZuulTestCase):
|
||||||
|
|
||||||
|
|
|
@ -78,7 +78,7 @@ class GerritSource(BaseSource):
|
||||||
results[0]['number'], results[0]['currentPatchSet']['number'])
|
results[0]['number'], results[0]['currentPatchSet']['number'])
|
||||||
return change
|
return change
|
||||||
|
|
||||||
def getChangesDependingOn(self, change, projects):
|
def getChangesDependingOn(self, change, projects, tenant):
|
||||||
changes = []
|
changes = []
|
||||||
if not change.uris:
|
if not change.uris:
|
||||||
return changes
|
return changes
|
||||||
|
|
|
@ -41,7 +41,7 @@ class GitSource(BaseSource):
|
||||||
def getChangeByURL(self, url):
|
def getChangeByURL(self, url):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
def getChangesDependingOn(self, change, projects):
|
def getChangesDependingOn(self, change, projects, tenant):
|
||||||
return []
|
return []
|
||||||
|
|
||||||
def getCachedChanges(self):
|
def getCachedChanges(self):
|
||||||
|
|
|
@ -751,7 +751,7 @@ class GithubConnection(BaseConnection):
|
||||||
raise
|
raise
|
||||||
return change
|
return change
|
||||||
|
|
||||||
def getChangesDependingOn(self, change, projects):
|
def getChangesDependingOn(self, change, projects, tenant):
|
||||||
changes = []
|
changes = []
|
||||||
if not change.uris:
|
if not change.uris:
|
||||||
return changes
|
return changes
|
||||||
|
@ -776,13 +776,17 @@ class GithubConnection(BaseConnection):
|
||||||
installation_projects.add(project.name)
|
installation_projects.add(project.name)
|
||||||
else:
|
else:
|
||||||
# We aren't in the context of a change queue and we just
|
# We aren't in the context of a change queue and we just
|
||||||
# need to query all installations. This currently only
|
# need to query all installations of this tenant. This currently
|
||||||
# happens if certain features of the zuul trigger are
|
# only happens if certain features of the zuul trigger are
|
||||||
# used; generally it should be avoided.
|
# used; generally it should be avoided.
|
||||||
for project, installation_id in self.installation_map.items():
|
for project_name, installation_id in self.installation_map.items():
|
||||||
|
trusted, project = tenant.getProject(project_name)
|
||||||
|
# ignore projects from different tenants
|
||||||
|
if not project:
|
||||||
|
continue
|
||||||
if installation_id not in installation_ids:
|
if installation_id not in installation_ids:
|
||||||
installation_ids.add(installation_id)
|
installation_ids.add(installation_id)
|
||||||
installation_projects.add(project)
|
installation_projects.add(project_name)
|
||||||
|
|
||||||
keys = set()
|
keys = set()
|
||||||
pattern = ' OR '.join(change.uris)
|
pattern = ' OR '.join(change.uris)
|
||||||
|
|
|
@ -91,8 +91,8 @@ class GithubSource(BaseSource):
|
||||||
patchset=pull.get('head').get('sha'))
|
patchset=pull.get('head').get('sha'))
|
||||||
return change
|
return change
|
||||||
|
|
||||||
def getChangesDependingOn(self, change, projects):
|
def getChangesDependingOn(self, change, projects, tenant):
|
||||||
return self.connection.getChangesDependingOn(change, projects)
|
return self.connection.getChangesDependingOn(change, projects, tenant)
|
||||||
|
|
||||||
def getCachedChanges(self):
|
def getCachedChanges(self):
|
||||||
return self.connection._change_cache.values()
|
return self.connection._change_cache.values()
|
||||||
|
|
|
@ -65,7 +65,8 @@ class ZuulDriver(Driver, TriggerInterface):
|
||||||
self.log.debug("onChangeEnqueued %s", tenant_events)
|
self.log.debug("onChangeEnqueued %s", tenant_events)
|
||||||
if tenant_events:
|
if tenant_events:
|
||||||
try:
|
try:
|
||||||
self._createParentChangeEnqueuedEvents(change, pipeline)
|
self._createParentChangeEnqueuedEvents(
|
||||||
|
change, pipeline, tenant)
|
||||||
except Exception:
|
except Exception:
|
||||||
self.log.exception(
|
self.log.exception(
|
||||||
"Unable to create parent-change-enqueued events for "
|
"Unable to create parent-change-enqueued events for "
|
||||||
|
@ -90,7 +91,7 @@ class ZuulDriver(Driver, TriggerInterface):
|
||||||
event.ref = change.ref
|
event.ref = change.ref
|
||||||
self.sched.addEvent(event)
|
self.sched.addEvent(event)
|
||||||
|
|
||||||
def _createParentChangeEnqueuedEvents(self, change, pipeline):
|
def _createParentChangeEnqueuedEvents(self, change, pipeline, tenant):
|
||||||
self.log.debug("Checking for changes needing %s:" % change)
|
self.log.debug("Checking for changes needing %s:" % change)
|
||||||
if not hasattr(change, 'needed_by_changes'):
|
if not hasattr(change, 'needed_by_changes'):
|
||||||
self.log.debug(" %s does not support dependencies" % type(change))
|
self.log.debug(" %s does not support dependencies" % type(change))
|
||||||
|
@ -103,7 +104,7 @@ class ZuulDriver(Driver, TriggerInterface):
|
||||||
for source in self.sched.connections.getSources():
|
for source in self.sched.connections.getSources():
|
||||||
self.log.debug(" Checking source: %s", source)
|
self.log.debug(" Checking source: %s", source)
|
||||||
needed_by_changes.update(
|
needed_by_changes.update(
|
||||||
source.getChangesDependingOn(change, None))
|
source.getChangesDependingOn(change, None, tenant))
|
||||||
self.log.debug(" Following changes: %s", needed_by_changes)
|
self.log.debug(" Following changes: %s", needed_by_changes)
|
||||||
|
|
||||||
for needs in needed_by_changes:
|
for needs in needed_by_changes:
|
||||||
|
|
|
@ -118,7 +118,8 @@ class DependentPipelineManager(PipelineManager):
|
||||||
for source in sources:
|
for source in sources:
|
||||||
self.log.debug(" Checking source: %s", source)
|
self.log.debug(" Checking source: %s", source)
|
||||||
for c in source.getChangesDependingOn(change,
|
for c in source.getChangesDependingOn(change,
|
||||||
change_queue.projects):
|
change_queue.projects,
|
||||||
|
self.pipeline.layout.tenant):
|
||||||
if c not in seen:
|
if c not in seen:
|
||||||
seen.add(c)
|
seen.add(c)
|
||||||
needed_by_changes.append(c)
|
needed_by_changes.append(c)
|
||||||
|
|
|
@ -61,7 +61,7 @@ class BaseSource(object, metaclass=abc.ABCMeta):
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@abc.abstractmethod
|
@abc.abstractmethod
|
||||||
def getChangesDependingOn(self, change, projects):
|
def getChangesDependingOn(self, change, projects, tenant):
|
||||||
"""Return changes which depend on changes at the supplied URIs.
|
"""Return changes which depend on changes at the supplied URIs.
|
||||||
|
|
||||||
Search this source for changes which depend on the supplied
|
Search this source for changes which depend on the supplied
|
||||||
|
@ -72,6 +72,9 @@ class BaseSource(object, metaclass=abc.ABCMeta):
|
||||||
If the projects argument is None, search across all known
|
If the projects argument is None, search across all known
|
||||||
projects. If it is supplied, the search may optionally be
|
projects. If it is supplied, the search may optionally be
|
||||||
restricted to only those projects.
|
restricted to only those projects.
|
||||||
|
|
||||||
|
The tenant argument can be used by the source to limit the
|
||||||
|
search scope.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@abc.abstractmethod
|
@abc.abstractmethod
|
||||||
|
|
Loading…
Reference in New Issue