[Kimchi-devel] [PATCH 2/2] bug fix: Properly set max body size to nginx proxy

To support uploading file to a storage pool, the max body size must be larger enough to allow uploading a ISO or image file. It was properly set on cherrypy server but as we use nginx as a reverse proxy it also needs to have the same config. Otherwise, nginx will block request with body size larger than 1M (the default value). Also add test cases to cover those scenarios. Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; --- src/kimchi/proxy.py | 25 +++++++------------ src/nginx.conf.in | 2 ++ tests/test_rest.py | 71 ++++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 65 insertions(+), 33 deletions(-) diff --git a/src/kimchi/proxy.py b/src/kimchi/proxy.py index d924ac8..9238147 100644 --- a/src/kimchi/proxy.py +++ b/src/kimchi/proxy.py @@ -31,7 +31,7 @@ from kimchi import sslcert from kimchi.config import paths -def _create_proxy_config(p_port, k_port, p_ssl_port, cert, key): +def _create_proxy_config(options): """Create nginx configuration file based on current ports config To allow flexibility in which port kimchi runs, we need the same @@ -41,11 +41,7 @@ def _create_proxy_config(p_port, k_port, p_ssl_port, cert, key): proxy. Arguments: - p_port - proxy port - k_port - kimchid port - p_ssl_port - proxy SSL port - cert - cert file specified by user config - key - key file specified by user config + options - OptionParser object with Kimchi config options """ # User that will run the worker process of the proxy. Fedora, @@ -59,7 +55,7 @@ def _create_proxy_config(p_port, k_port, p_ssl_port, cert, key): config_dir = paths.conf_dir # No certificates specified by the user - if not cert or not key: + if not options.ssl_cert or not options.ssl_key: cert = '%s/kimchi-cert.pem' % config_dir key = '%s/kimchi-key.pem' % config_dir # create cert files if they don't exist @@ -76,10 +72,11 @@ def _create_proxy_config(p_port, k_port, p_ssl_port, cert, key): data = template.read() data = Template(data) data = data.safe_substitute(user=user_proxy, - proxy_port=p_port, - kimchid_port=k_port, - proxy_ssl_port=p_ssl_port, - cert_pem=cert, cert_key=key) + proxy_port=options.port, + kimchid_port=options.cherrypy_port, + proxy_ssl_port=options.ssl_port, + cert_pem=cert, cert_key=key, + max_body_size=eval(options.max_body_size)) # Write file to be used for nginx. config_file = open(os.path.join(config_dir, "nginx_kimchi.conf"), "w") @@ -89,11 +86,7 @@ def _create_proxy_config(p_port, k_port, p_ssl_port, cert, key): def start_proxy(options): """Start nginx reverse proxy.""" - _create_proxy_config(options.port, - options.cherrypy_port, - options.ssl_port, - options.ssl_cert, - options.ssl_key) + _create_proxy_config(options) config_dir = paths.conf_dir config_file = "%s/nginx_kimchi.conf" % config_dir cmd = ['nginx', '-c', config_file] diff --git a/src/nginx.conf.in b/src/nginx.conf.in index 9d11770..b5d207f 100644 --- a/src/nginx.conf.in +++ b/src/nginx.conf.in @@ -38,6 +38,8 @@ http { access_log /var/log/nginx/access.log main; sendfile on; + client_max_body_size ${max_body_size}k; + # Timeout set to 10 minutes to avoid the 504 Gateway Timeout # when Kimchi is processing a request. proxy_connect_timeout 600; diff --git a/tests/test_rest.py b/tests/test_rest.py index ae1c971..b63e990 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -35,6 +35,7 @@ from functools import partial import iso_gen import kimchi.mockmodel import kimchi.server +from kimchi.config import paths from kimchi.rollbackcontext import RollbackContext from utils import get_free_port, patch_auth, request from utils import run_server @@ -1907,26 +1908,62 @@ class RestTests(unittest.TestCase): return headers with RollbackContext() as rollback: - upload_path = '/tmp/vol' - f = open(upload_path, 'w') - f.write('abcd') - f.close() - rollback.prependDefer(os.remove, upload_path) - - vol = open(upload_path, 'rb') - url = "https://%s:%s/storagepools/default/storagevolumes" %\ + vol_name = 'COPYING' + vol_path = os.path.join(paths.get_prefix(), vol_name) + url = "https://%s:%s/storagepools/default/storagevolumes" % \ (host, ssl_port) - r = requests.post(url, - files={'file': vol}, - data={'name': 'new_vol2'}, - verify=False, - headers=fake_auth_header()) + with open(vol_path, 'rb') as fd: + r = requests.post(url, + files={'file': fd}, + data={'name': vol_name}, + verify=False, + headers=fake_auth_header()) + self.assertEquals(r.status_code, 202) - time.sleep(1) - vol_list = json.loads( - self.request('/storagepools/default/storagevolumes').read()) - self.assertEquals('new_vol2', vol_list[0]['name']) + self._wait_task(r.json()['id']) + resp = self.request('/storagepools/default/storagevolumes/%s' % + vol_name) + self.assertEquals(200, resp.status) + + # Create a file with 3M to upload + vol_name = '3m-file' + vol_path = os.path.join('/tmp', vol_name) + with open(vol_path, 'wb') as fd: + fd.seek(3*1024*1024-1) + fd.write("\0") + rollback.prependDefer(os.remove, vol_path) + + with open(vol_path, 'rb') as fd: + r = requests.post(url, + files={'file': fd}, + data={'name': vol_name}, + verify=False, + headers=fake_auth_header()) + + self.assertEquals(r.status_code, 202) + self._wait_task(r.json()['id']) + resp = self.request('/storagepools/default/storagevolumes/%s' % + vol_name) + self.assertEquals(200, resp.status) + + # Create a file with 5M to upload + # Max body size is set to 4M so the upload will fail with 413 + vol_name = '5m-file' + vol_path = os.path.join('/tmp', vol_name) + with open(vol_path, 'wb') as fd: + fd.seek(5*1024*1024-1) + fd.write("\0") + rollback.prependDefer(os.remove, vol_path) + + with open(vol_path, 'rb') as fd: + r = requests.post(url, + files={'file': fd}, + data={'name': vol_name}, + verify=False, + headers=fake_auth_header()) + + self.assertEquals(r.status_code, 413) class HttpsRestTests(RestTests): -- 1.9.3