Re-enable test of returning 404 on unknown tenant
Detecting an unknown tenant got tricky when we started returning a message about tenants not being ready yet. In order to be able to return a 404 for tenants we legitimately do not know anything about, keep an UnparsedAbideConfig on the scheduler so that we can check it in case of a miss. If we know about a tenant but don't have a config, we can return the 'not ready' message. If we don't know about it at all, we can throw the 404. Also, remove the custom handler test. We have tests in other contexts (like tests of the github webhook) that test the equivilent functionality. Change-Id: Icff5d7036b6a237646ad7482103f7b487621bac0
This commit is contained in:
parent
c6ca548116
commit
9db66082d3
|
@ -5,7 +5,6 @@ pbr>=1.1.0
|
||||||
git+https://github.com/sigmavirus24/github3.py.git@develop#egg=Github3.py
|
git+https://github.com/sigmavirus24/github3.py.git@develop#egg=Github3.py
|
||||||
PyYAML>=3.1.0
|
PyYAML>=3.1.0
|
||||||
Paste
|
Paste
|
||||||
WebOb>=1.2.3
|
|
||||||
paramiko>=2.0.1
|
paramiko>=2.0.1
|
||||||
GitPython>=2.1.8
|
GitPython>=2.1.8
|
||||||
python-daemon>=2.0.4,<2.1.0
|
python-daemon>=2.0.4,<2.1.0
|
||||||
|
|
|
@ -22,9 +22,6 @@ import json
|
||||||
import urllib
|
import urllib
|
||||||
import time
|
import time
|
||||||
import socket
|
import socket
|
||||||
from unittest import skip
|
|
||||||
|
|
||||||
import webob
|
|
||||||
|
|
||||||
import zuul.web
|
import zuul.web
|
||||||
|
|
||||||
|
@ -212,21 +209,6 @@ class TestWeb(ZuulTestCase):
|
||||||
f = urllib.request.urlopen(req)
|
f = urllib.request.urlopen(req)
|
||||||
self.assertEqual(f.read(), public_pem)
|
self.assertEqual(f.read(), public_pem)
|
||||||
|
|
||||||
@skip("This may not apply to zuul-web")
|
|
||||||
def test_web_custom_handler(self):
|
|
||||||
def custom_handler(path, tenant_name, request):
|
|
||||||
return webob.Response(body='ok')
|
|
||||||
|
|
||||||
self.webapp.register_path('/custom', custom_handler)
|
|
||||||
req = urllib.request.Request(
|
|
||||||
"http://localhost:%s/custom" % self.port)
|
|
||||||
f = urllib.request.urlopen(req)
|
|
||||||
self.assertEqual(b'ok', f.read())
|
|
||||||
|
|
||||||
self.webapp.unregister_path('/custom')
|
|
||||||
self.assertRaises(urllib.error.HTTPError, urllib.request.urlopen, req)
|
|
||||||
|
|
||||||
@skip("This returns a 500")
|
|
||||||
def test_web_404_on_unknown_tenant(self):
|
def test_web_404_on_unknown_tenant(self):
|
||||||
req = urllib.request.Request(
|
req = urllib.request.Request(
|
||||||
"http://localhost:{}/non-tenant/status".format(self.port))
|
"http://localhost:{}/non-tenant/status".format(self.port))
|
||||||
|
|
|
@ -1754,20 +1754,22 @@ class ConfigLoader(object):
|
||||||
config_path)
|
config_path)
|
||||||
return config_path
|
return config_path
|
||||||
|
|
||||||
def loadConfig(self, config_path, project_key_dir):
|
def readConfig(self, config_path):
|
||||||
abide = model.Abide()
|
|
||||||
|
|
||||||
config_path = self.expandConfigPath(config_path)
|
config_path = self.expandConfigPath(config_path)
|
||||||
with open(config_path) as config_file:
|
with open(config_path) as config_file:
|
||||||
self.log.info("Loading configuration from %s" % (config_path,))
|
self.log.info("Loading configuration from %s" % (config_path,))
|
||||||
data = yaml.safe_load(config_file)
|
data = yaml.safe_load(config_file)
|
||||||
config = model.UnparsedAbideConfig()
|
|
||||||
config.extend(data)
|
|
||||||
base = os.path.dirname(os.path.realpath(config_path))
|
base = os.path.dirname(os.path.realpath(config_path))
|
||||||
|
unparsed_abide = model.UnparsedAbideConfig(base)
|
||||||
|
unparsed_abide.extend(data)
|
||||||
|
return unparsed_abide
|
||||||
|
|
||||||
for conf_tenant in config.tenants:
|
def loadConfig(self, unparsed_abide, project_key_dir):
|
||||||
|
abide = model.Abide()
|
||||||
|
for conf_tenant in unparsed_abide.tenants:
|
||||||
# When performing a full reload, do not use cached data.
|
# When performing a full reload, do not use cached data.
|
||||||
tenant = self.tenant_parser.fromYaml(base, project_key_dir,
|
tenant = self.tenant_parser.fromYaml(unparsed_abide.base,
|
||||||
|
project_key_dir,
|
||||||
conf_tenant, old_tenant=None)
|
conf_tenant, old_tenant=None)
|
||||||
abide.tenants[tenant.name] = tenant
|
abide.tenants[tenant.name] = tenant
|
||||||
return abide
|
return abide
|
||||||
|
|
|
@ -2446,8 +2446,10 @@ class UnparsedAbideConfig(object):
|
||||||
An Abide is a collection of tenants.
|
An Abide is a collection of tenants.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self):
|
def __init__(self, base=None):
|
||||||
self.tenants = []
|
self.tenants = []
|
||||||
|
self.known_tenants = set()
|
||||||
|
self.base = base
|
||||||
|
|
||||||
def extend(self, conf):
|
def extend(self, conf):
|
||||||
if isinstance(conf, UnparsedAbideConfig):
|
if isinstance(conf, UnparsedAbideConfig):
|
||||||
|
@ -2465,6 +2467,8 @@ class UnparsedAbideConfig(object):
|
||||||
key, value = list(item.items())[0]
|
key, value = list(item.items())[0]
|
||||||
if key == 'tenant':
|
if key == 'tenant':
|
||||||
self.tenants.append(value)
|
self.tenants.append(value)
|
||||||
|
if 'name' in value:
|
||||||
|
self.known_tenants.add(value['name'])
|
||||||
else:
|
else:
|
||||||
raise ConfigItemUnknownError()
|
raise ConfigItemUnknownError()
|
||||||
|
|
||||||
|
|
|
@ -246,6 +246,7 @@ class Scheduler(threading.Thread):
|
||||||
self.result_event_queue = queue.Queue()
|
self.result_event_queue = queue.Queue()
|
||||||
self.management_event_queue = zuul.lib.queue.MergedQueue()
|
self.management_event_queue = zuul.lib.queue.MergedQueue()
|
||||||
self.abide = model.Abide()
|
self.abide = model.Abide()
|
||||||
|
self.unparsed_abide = model.UnparsedAbideConfig()
|
||||||
|
|
||||||
if not testonly:
|
if not testonly:
|
||||||
time_dir = self._get_time_database_dir()
|
time_dir = self._get_time_database_dir()
|
||||||
|
@ -550,8 +551,10 @@ class Scheduler(threading.Thread):
|
||||||
self.log.info("Full reconfiguration beginning")
|
self.log.info("Full reconfiguration beginning")
|
||||||
loader = configloader.ConfigLoader(
|
loader = configloader.ConfigLoader(
|
||||||
self.connections, self, self.merger)
|
self.connections, self, self.merger)
|
||||||
|
self.unparsed_abide = loader.readConfig(
|
||||||
|
self.config.get('scheduler', 'tenant_config'))
|
||||||
abide = loader.loadConfig(
|
abide = loader.loadConfig(
|
||||||
self.config.get('scheduler', 'tenant_config'),
|
self.unparsed_abide,
|
||||||
self._get_project_key_dir())
|
self._get_project_key_dir())
|
||||||
for tenant in abide.tenants.values():
|
for tenant in abide.tenants.values():
|
||||||
self._reconfigureTenant(tenant)
|
self._reconfigureTenant(tenant)
|
||||||
|
@ -1149,6 +1152,8 @@ class Scheduler(threading.Thread):
|
||||||
data['pipelines'] = pipelines
|
data['pipelines'] = pipelines
|
||||||
tenant = self.abide.tenants.get(tenant_name)
|
tenant = self.abide.tenants.get(tenant_name)
|
||||||
if not tenant:
|
if not tenant:
|
||||||
|
if tenant_name not in self.unparsed_abide.known_tenants:
|
||||||
|
return json.dumps({"message": "Unknown tenant"})
|
||||||
self.log.warning("Tenant %s isn't loaded" % tenant_name)
|
self.log.warning("Tenant %s isn't loaded" % tenant_name)
|
||||||
return json.dumps(
|
return json.dumps(
|
||||||
{"message": "Tenant %s isn't ready" % tenant_name})
|
{"message": "Tenant %s isn't ready" % tenant_name})
|
||||||
|
|
|
@ -171,6 +171,8 @@ class GearmanHandler(object):
|
||||||
self.cache[tenant] = json.loads(job.data[0])
|
self.cache[tenant] = json.loads(job.data[0])
|
||||||
self.cache_time[tenant] = time.time()
|
self.cache_time[tenant] = time.time()
|
||||||
payload = self.cache[tenant]
|
payload = self.cache[tenant]
|
||||||
|
if payload.get('message') == 'Unknown tenant':
|
||||||
|
return web.HTTPNotFound()
|
||||||
if result_filter:
|
if result_filter:
|
||||||
payload = result_filter.filterPayload(payload)
|
payload = result_filter.filterPayload(payload)
|
||||||
resp = web.json_response(payload)
|
resp = web.json_response(payload)
|
||||||
|
|
Loading…
Reference in New Issue