From 5079d8429d9799e291d098e14487034c7e910a0d Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Wed, 1 Dec 2021 13:54:36 +0000 Subject: internal-client: pass global_conf to loadapp The internal client previously provided no easy way to programatically customise the configuration of the proxy-server app or other middlewares in its wsgi pipeline. This patch allows a global_conf dict to be passed via the InternalClient constructor to the wsgi loadapp function. Items in the global_conf dict will override options loaded from the config file. An example use case would be to change the log_name from the default 'swift', which would be useful to differentiate logs from different processes using an internal client. The minimum version of PasteDeploy is increased to 2.0.0 to make the global_conf behavior predictable: in older versions global_conf would not override options in the conf file DEFAULT section, but since 2.0.0 it will. Change-Id: Ida39ec7eb02a93cf4b2aa68fc07b7f0ae27b5439 --- lower-constraints.txt | 2 +- requirements.txt | 2 +- swift/common/internal_client.py | 10 ++++-- swift/common/wsgi.py | 9 +++++ test/unit/common/test_internal_client.py | 45 +++++++++++++---------- test/unit/common/test_wsgi.py | 62 +++++++++++++++++++++++++++++++- 6 files changed, 105 insertions(+), 25 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index 6437c5d7b..55336388e 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -48,7 +48,7 @@ oslo.i18n==3.20.0 oslo.log==3.22.0 oslo.serialization==2.25.0 oslo.utils==3.36.0 -PasteDeploy==1.3.3 +PasteDeploy==2.0.0 pbr==3.1.1 prettytable==0.7.2 pycparser==2.18 diff --git a/requirements.txt b/requirements.txt index 223fc617b..3b74c3a87 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,7 @@ eventlet>=0.25.0 # MIT greenlet>=0.3.2 netifaces>=0.8,!=0.10.0,!=0.10.1 -PasteDeploy>=1.3.3 +PasteDeploy>=2.0.0 lxml>=3.4.1 requests>=2.14.2 # Apache-2.0 six>=1.10.0 diff --git a/swift/common/internal_client.py b/swift/common/internal_client.py index bd9452ec5..c24693f48 100644 --- a/swift/common/internal_client.py +++ b/swift/common/internal_client.py @@ -145,14 +145,18 @@ class InternalClient(object): :param user_agent: User agent to be sent to requests to Swift. :param request_tries: Number of tries before InternalClient.make_request() gives up. + :param global_conf: a dict of options to update the loaded proxy config. + Options in ``global_conf`` will override those in ``conf_path`` except + where the ``conf_path`` option is preceded by ``set``. """ def __init__(self, conf_path, user_agent, request_tries, - allow_modify_pipeline=False, use_replication_network=False): + allow_modify_pipeline=False, use_replication_network=False, + global_conf=None): if request_tries < 1: raise ValueError('request_tries must be positive') - self.app = loadapp(conf_path, - allow_modify_pipeline=allow_modify_pipeline) + self.app = loadapp(conf_path, global_conf=global_conf, + allow_modify_pipeline=allow_modify_pipeline,) self.user_agent = user_agent self.request_tries = request_tries self.use_replication_network = use_replication_network diff --git a/swift/common/wsgi.py b/swift/common/wsgi.py index 0f2528146..8e3a7a4af 100644 --- a/swift/common/wsgi.py +++ b/swift/common/wsgi.py @@ -386,6 +386,15 @@ def loadapp(conf_file, global_conf=None, allow_modify_pipeline=True): """ Loads a context from a config file, and if the context is a pipeline then presents the app with the opportunity to modify the pipeline. + + :param conf_file: path to a config file + :param global_conf: a dict of options to update the loaded config. Options + in ``global_conf`` will override those in ``conf_file`` except where + the ``conf_file`` option is preceded by ``set``. + :param allow_modify_pipeline: if True, and the context is a pipeline, and + the loaded app has a ``modify_wsgi_pipeline`` property, then that + property will be called before the pipeline is loaded. + :return: the loaded app """ global_conf = global_conf or {} ctx = loadcontext(loadwsgi.APP, conf_file, global_conf=global_conf) diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index a3bc6c7a7..c25b1e7dd 100644 --- a/test/unit/common/test_internal_client.py +++ b/test/unit/common/test_internal_client.py @@ -282,47 +282,54 @@ class TestInternalClient(unittest.TestCase): self.assertEqual(client.auto_create_account_prefix, '-') def test_init(self): - class App(object): - def __init__(self, test, conf_path): - self.test = test - self.conf_path = conf_path - self.load_called = 0 - - def load(self, uri, allow_modify_pipeline=True): - self.load_called += 1 - self.test.assertEqual(conf_path, uri) - self.test.assertFalse(allow_modify_pipeline) - return self - conf_path = 'some_path' - app = App(self, conf_path) + app = object() user_agent = 'some_user_agent' request_tries = 123 - with mock.patch.object(internal_client, 'loadapp', app.load), \ + with mock.patch.object( + internal_client, 'loadapp', return_value=app) as mock_loadapp,\ self.assertRaises(ValueError): # First try with a bad arg internal_client.InternalClient( conf_path, user_agent, request_tries=0) - self.assertEqual(0, app.load_called) + mock_loadapp.assert_not_called() - with mock.patch.object(internal_client, 'loadapp', app.load): + with mock.patch.object( + internal_client, 'loadapp', return_value=app) as mock_loadapp: client = internal_client.InternalClient( conf_path, user_agent, request_tries) - self.assertEqual(1, app.load_called) + mock_loadapp.assert_called_once_with( + conf_path, global_conf=None, allow_modify_pipeline=False) self.assertEqual(app, client.app) self.assertEqual(user_agent, client.user_agent) self.assertEqual(request_tries, client.request_tries) self.assertFalse(client.use_replication_network) - with mock.patch.object(internal_client, 'loadapp', app.load): + with mock.patch.object( + internal_client, 'loadapp', return_value=app) as mock_loadapp: client = internal_client.InternalClient( conf_path, user_agent, request_tries, use_replication_network=True) - self.assertEqual(2, app.load_called) + mock_loadapp.assert_called_once_with( + conf_path, global_conf=None, allow_modify_pipeline=False) + self.assertEqual(app, client.app) + self.assertEqual(user_agent, client.user_agent) + self.assertEqual(request_tries, client.request_tries) + self.assertTrue(client.use_replication_network) + + global_conf = {'log_name': 'custom'} + with mock.patch.object( + internal_client, 'loadapp', return_value=app) as mock_loadapp: + client = internal_client.InternalClient( + conf_path, user_agent, request_tries, + use_replication_network=True, global_conf=global_conf) + + mock_loadapp.assert_called_once_with( + conf_path, global_conf=global_conf, allow_modify_pipeline=False) self.assertEqual(app, client.app) self.assertEqual(user_agent, client.user_agent) self.assertEqual(request_tries, client.request_tries) diff --git a/test/unit/common/test_wsgi.py b/test/unit/common/test_wsgi.py index 35be5b03c..bf601570e 100644 --- a/test/unit/common/test_wsgi.py +++ b/test/unit/common/test_wsgi.py @@ -144,7 +144,67 @@ class TestWSGI(unittest.TestCase): with open(conf_path, 'w') as f: f.write(contents) app = wsgi.loadapp(conf_path) - self.assertTrue(isinstance(app, obj_server.ObjectController)) + self.assertIsInstance(app, obj_server.ObjectController) + + @with_tempdir + def test_loadapp_from_file_with_global_conf(self, tempdir): + # verify that global_conf items override conf file DEFAULTS... + conf_path = os.path.join(tempdir, 'object-server.conf') + conf_body = """ + [DEFAULT] + log_name = swift + [app:main] + use = egg:swift#object + log_name = swift-main + """ + contents = dedent(conf_body) + with open(conf_path, 'w') as f: + f.write(contents) + app = wsgi.loadapp(conf_path) + self.assertIsInstance(app, obj_server.ObjectController) + self.assertEqual('swift', app.logger.server) + + app = wsgi.loadapp(conf_path, global_conf={'log_name': 'custom'}) + self.assertIsInstance(app, obj_server.ObjectController) + self.assertEqual('custom', app.logger.server) + + # and regular section options... + conf_path = os.path.join(tempdir, 'object-server.conf') + conf_body = """ + [DEFAULT] + [app:main] + use = egg:swift#object + log_name = swift-main + """ + contents = dedent(conf_body) + with open(conf_path, 'w') as f: + f.write(contents) + app = wsgi.loadapp(conf_path) + self.assertIsInstance(app, obj_server.ObjectController) + self.assertEqual('swift-main', app.logger.server) + + app = wsgi.loadapp(conf_path, global_conf={'log_name': 'custom'}) + self.assertIsInstance(app, obj_server.ObjectController) + self.assertEqual('custom', app.logger.server) + + # ...but global_conf items do not override conf file 'set' options + conf_body = """ + [DEFAULT] + log_name = swift + [app:main] + use = egg:swift#object + set log_name = swift-main + """ + contents = dedent(conf_body) + with open(conf_path, 'w') as f: + f.write(contents) + app = wsgi.loadapp(conf_path) + self.assertIsInstance(app, obj_server.ObjectController) + self.assertEqual('swift-main', app.logger.server) + + app = wsgi.loadapp(conf_path, global_conf={'log_name': 'custom'}) + self.assertIsInstance(app, obj_server.ObjectController) + self.assertEqual('swift-main', app.logger.server) def test_loadapp_from_string(self): conf_body = """ -- cgit v1.2.1