Refactor proxy-server conf loading to a helper function

There were two middlewares using a common pattern to load
the proxy-server app config section. The existing pattern
fails to recognise option overrides that are declared using
paste-deploy's 'set' notation, as illustrated by the change
to test_dlo.py in this patch.

This patch replaces the existing code with a helper function
that loads the proxy-server config using the paste-deploy loader.
The resulting config dict is therefore exactly the same as that
used to initialise the proxy-server app.

Change-Id: Ib58ce03e2010f41e7eb11f1a6dc78b0b7f55d466

swift/common/middleware/copy.py

@@ -114,20 +114,18 @@ greater than 5GB.
 
 """
 
-import os
-from six.moves.configparser import ConfigParser, NoSectionError, NoOptionError
 from six.moves.urllib.parse import quote, unquote
 
 from swift.common import utils
 from swift.common.utils import get_logger, \
-    config_true_value, FileLikeIter, read_conf_dir, close_if_possible
+    config_true_value, FileLikeIter, close_if_possible
 from swift.common.swob import Request, HTTPPreconditionFailed, \
     HTTPRequestEntityTooLarge, HTTPBadRequest, HTTPException
 from swift.common.http import HTTP_MULTIPLE_CHOICES, is_success, HTTP_OK
 from swift.common.constraints import check_account_format, MAX_FILE_SIZE
 from swift.common.request_helpers import copy_header_subset, remove_items, \
     is_sys_meta, is_sys_or_user_meta, is_object_transient_sysmeta
-from swift.common.wsgi import WSGIContext, make_subrequest
+from swift.common.wsgi import WSGIContext, make_subrequest, load_app_config
 
 
 def _check_path_header(req, name, length, error_msg):
@@ -263,25 +261,9 @@ class ServerSideCopyMiddleware(object):
             # This takes preference over the one set in proxy app
             return
 
-        cp = ConfigParser()
+        proxy_conf = load_app_config(conf['__file__'])
-        if os.path.isdir(conf['__file__']):
+        if 'object_post_as_copy' in proxy_conf:
-            read_conf_dir(cp, conf['__file__'])
+            conf['object_post_as_copy'] = proxy_conf['object_post_as_copy']
-        else:
-            cp.read(conf['__file__'])
-
-        try:
-            pipe = cp.get("pipeline:main", "pipeline")
-        except (NoSectionError, NoOptionError):
-            return
-
-        proxy_name = pipe.rsplit(None, 1)[-1]
-        proxy_section = "app:" + proxy_name
-
-        try:
-            conf['object_post_as_copy'] = cp.get(proxy_section,
-                                                 'object_post_as_copy')
-        except (NoSectionError, NoOptionError):
-            pass
 
     def __call__(self, env, start_response):
         req = Request(env)

swift/common/middleware/dlo.py

@@ -119,10 +119,8 @@ Here's an example using ``curl`` with tiny 1-byte segments::
 """
 
 import json
-import os
 
 import six
-from six.moves.configparser import ConfigParser, NoSectionError, NoOptionError
 from six.moves.urllib.parse import unquote
 
 from hashlib import md5
@@ -132,10 +130,9 @@ from swift.common.http import is_success
 from swift.common.swob import Request, Response, \
     HTTPRequestedRangeNotSatisfiable, HTTPBadRequest, HTTPConflict
 from swift.common.utils import get_logger, \
-    RateLimitedIterator, read_conf_dir, quote, close_if_possible, \
+    RateLimitedIterator, quote, close_if_possible, closing_if_possible
-    closing_if_possible
 from swift.common.request_helpers import SegmentedIterable
-from swift.common.wsgi import WSGIContext, make_subrequest
+from swift.common.wsgi import WSGIContext, make_subrequest, load_app_config
 
 
 class GetContext(WSGIContext):
@@ -381,26 +378,12 @@ class DynamicLargeObject(object):
                 '__file__' not in conf):
             return
 
-        cp = ConfigParser()
+        proxy_conf = load_app_config(conf['__file__'])
-        if os.path.isdir(conf['__file__']):
-            read_conf_dir(cp, conf['__file__'])
-        else:
-            cp.read(conf['__file__'])
-
-        try:
-            pipe = cp.get("pipeline:main", "pipeline")
-        except (NoSectionError, NoOptionError):
-            return
-
-        proxy_name = pipe.rsplit(None, 1)[-1]
-        proxy_section = "app:" + proxy_name
         for setting in ('rate_limit_after_segment',
                         'rate_limit_segments_per_sec',
                         'max_get_time'):
-            try:
+            if setting in proxy_conf:
-                conf[setting] = cp.get(proxy_section, setting)
+                conf[setting] = proxy_conf[setting]
-            except (NoSectionError, NoOptionError):
-                pass
 
     def __call__(self, env, start_response):
         """

swift/common/wsgi.py

@@ -397,6 +397,24 @@ def loadapp(conf_file, global_conf=None, allow_modify_pipeline=True):
     return ctx.create()
 
 
+def load_app_config(conf_file):
+    """
+    Read the app config section from a config file.
+
+    :param conf_file: path to a config file
+    :return: a dict
+    """
+    app_conf = {}
+    try:
+        ctx = loadcontext(loadwsgi.APP, conf_file)
+    except LookupError:
+        pass
+    else:
+        app_conf.update(ctx.app_context.global_conf)
+        app_conf.update(ctx.app_context.local_conf)
+    return app_conf
+
+
 def run_server(conf, logger, sock, global_conf=None):
     # Ensure TZ environment variable exists to avoid stat('/etc/localtime') on
     # some platforms. This locks in reported times to UTC.

test/unit/common/middleware/test_copy.py

@@ -1309,6 +1309,9 @@ class TestServerSideCopyConfiguration(unittest.TestCase):
         [pipeline:main]
         pipeline = catch_errors copy ye-olde-proxy-server
 
+        [filter:catch_errors]
+        use = egg:swift#catch_errors
+
         [filter:copy]
         use = egg:swift#copy
 

test/unit/common/middleware/test_dlo.py

@@ -811,6 +811,9 @@ class TestDloConfiguration(unittest.TestCase):
         [pipeline:main]
         pipeline = catch_errors dlo ye-olde-proxy-server
 
+        [filter:catch_errors]
+        use = egg:swift#catch_errors
+
         [filter:dlo]
         use = egg:swift#dlo
         max_get_time = 3600
@@ -845,13 +848,16 @@ class TestDloConfiguration(unittest.TestCase):
         [pipeline:main]
         pipeline = catch_errors dlo ye-olde-proxy-server
 
+        [filter:catch_errors]
+        use = egg:swift#catch_errors
+
         [filter:dlo]
         use = egg:swift#dlo
 
         [app:ye-olde-proxy-server]
         use = egg:swift#proxy
         rate_limit_after_segment = 13
-        max_get_time = 2900
+        set max_get_time = 2900
         """)
 
         conffile = tempfile.NamedTemporaryFile()
@@ -878,6 +884,9 @@ class TestDloConfiguration(unittest.TestCase):
         """)
 
         proxy_conf2 = dedent("""
+        [filter:catch_errors]
+        use = egg:swift#catch_errors
+
         [filter:dlo]
         use = egg:swift#dlo
 

test/unit/common/test_wsgi.py

@@ -198,6 +198,97 @@ class TestWSGI(unittest.TestCase):
         app = wsgi.loadapp(wsgi.ConfigString(conf_body))
         self.assertTrue(isinstance(app, obj_server.ObjectController))
 
+    @with_tempdir
+    def test_load_app_config(self, tempdir):
+        conf_file = os.path.join(tempdir, 'file.conf')
+
+        def _write_and_load_conf_file(conf):
+            with open(conf_file, 'wb') as fd:
+                fd.write(dedent(conf))
+            return wsgi.load_app_config(conf_file)
+
+        # typical case - DEFAULT options override same option in other sections
+        conf_str = """
+            [DEFAULT]
+            dflt_option = dflt-value
+
+            [pipeline:main]
+            pipeline = proxy-logging proxy-server
+
+            [filter:proxy-logging]
+            use = egg:swift#proxy_logging
+
+            [app:proxy-server]
+            use = egg:swift#proxy
+            proxy_option = proxy-value
+            dflt_option = proxy-dflt-value
+            """
+
+        proxy_conf = _write_and_load_conf_file(conf_str)
+        self.assertEqual('proxy-value', proxy_conf['proxy_option'])
+        self.assertEqual('dflt-value', proxy_conf['dflt_option'])
+
+        # 'set' overrides DEFAULT option
+        conf_str = """
+            [DEFAULT]
+            dflt_option = dflt-value
+
+            [pipeline:main]
+            pipeline = proxy-logging proxy-server
+
+            [filter:proxy-logging]
+            use = egg:swift#proxy_logging
+
+            [app:proxy-server]
+            use = egg:swift#proxy
+            proxy_option = proxy-value
+            set dflt_option = proxy-dflt-value
+            """
+
+        proxy_conf = _write_and_load_conf_file(conf_str)
+        self.assertEqual('proxy-value', proxy_conf['proxy_option'])
+        self.assertEqual('proxy-dflt-value', proxy_conf['dflt_option'])
+
+        # actual proxy server app name is dereferenced
+        conf_str = """
+            [pipeline:main]
+            pipeline = proxy-logging proxyserverapp
+
+            [filter:proxy-logging]
+            use = egg:swift#proxy_logging
+
+            [app:proxyserverapp]
+            use = egg:swift#proxy
+            proxy_option = proxy-value
+            dflt_option = proxy-dflt-value
+            """
+        proxy_conf = _write_and_load_conf_file(conf_str)
+        self.assertEqual('proxy-value', proxy_conf['proxy_option'])
+        self.assertEqual('proxy-dflt-value', proxy_conf['dflt_option'])
+
+        # no pipeline
+        conf_str = """
+            [filter:proxy-logging]
+            use = egg:swift#proxy_logging
+
+            [app:proxy-server]
+            use = egg:swift#proxy
+            proxy_option = proxy-value
+            """
+        proxy_conf = _write_and_load_conf_file(conf_str)
+        self.assertEqual({}, proxy_conf)
+
+        # no matching section
+        conf_str = """
+            [pipeline:main]
+            pipeline = proxy-logging proxy-server
+
+            [filter:proxy-logging]
+            use = egg:swift#proxy_logging
+            """
+        proxy_conf = _write_and_load_conf_file(conf_str)
+        self.assertEqual({}, proxy_conf)
+
     def test_init_request_processor_from_conf_dir(self):
         config_dir = {
             'proxy-server.conf.d/pipeline.conf': """