[Kimchi-devel] [PATCH] bug fix: Properly set max body size to nginx proxy
Aline Manera
alinefm at linux.vnet.ibm.com
Wed Sep 10 02:44:35 UTC 2014
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 at 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
More information about the Kimchi-devel
mailing list