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@linux.vnet.ibm.com> --- src/kimchi/proxy.py | 28 ++++++++++------------- src/nginx.conf.in | 2 ++ tests/test_rest.py | 64 ++++++++++++++++++++++++++++++++++++++++++----------- 3 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/kimchi/proxy.py b/src/kimchi/proxy.py index d924ac8..34bca4d 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 @@ -70,16 +66,20 @@ def _create_proxy_config(p_port, k_port, p_ssl_port, cert, key): with open(key, "w") as f: f.write(ssl_gen.key_pem()) + # Append size unit to max body size value as needed by nginx config + max_body_size = str(eval(options.max_body_size)) + 'k' + # Read template file and create a new config file # with the specified parameters. with open(os.path.join(config_dir, "nginx.conf.in")) as template: 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=max_body_size) # Write file to be used for nginx. config_file = open(os.path.join(config_dir, "nginx_kimchi.conf"), "w") @@ -89,11 +89,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 1d1a398..534171d 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; + # 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..453f304 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,63 @@ 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) - + # upload Kimchi's "COPYING" and compare its + # content to the corresponding file in the storage pool + vol_name = 'COPYING' + upload_path = os.path.join(paths.get_prefix(), vol_name) vol = open(upload_path, 'rb') - url = "https://%s:%s/storagepools/default/storagevolumes" %\ - (host, ssl_port) + rollback.prependDefer(vol.close) + url = "https://%s:%s/storagepools/default/storagevolumes" % \ + (host, ssl_port) r = requests.post(url, files={'file': vol}, - data={'name': 'new_vol2'}, + 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) + + r = requests.post(url, + files={'file': open(vol_path, 'rb')}, + 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) + + r = requests.post(url, + files={'file': open(vol_path, 'rb')}, + data={'name': vol_name}, + verify=False, + headers=fake_auth_header()) + + self.assertEquals(r.status_code, 413) class HttpsRestTests(RestTests): -- 1.9.3