From edd6810c3ddfa9b23febda7d6198bd1089ab3f2f Mon Sep 17 00:00:00 2001 From: Kaifeng Wang Date: Tue, 17 Apr 2018 14:39:48 +0800 Subject: [PATCH] Wrap Flask into oslo.service This patch is part of inspector HA work, which wraps inspector api into oslo service. oslo.service has also provided support to signal processing like SIGHUP or SIGTERM, so these code were removed in this patch. Deprecated current SSL cert/key options used by ironic-inspector, code manually creates ssl context were removed. These options will be fed from [ssl] section. Change-Id: Ia5e16fcb9104556d62c90f5507f17b41f73a5208 Story: #2001842 Task: #12609 --- ironic_inspector/cmd/all.py | 12 +- ironic_inspector/common/service_utils.py | 6 + ironic_inspector/conf/default.py | 8 +- .../test/unit/test_wsgi_service.py | 187 ++++-------------- ironic_inspector/wsgi_service.py | 105 +++------- lower-constraints.txt | 2 +- .../deprecate-ssl-opts-40ce8f4618c786ef.yaml | 7 + requirements.txt | 1 + tools/config-generator.conf | 5 +- 9 files changed, 96 insertions(+), 237 deletions(-) create mode 100644 releasenotes/notes/deprecate-ssl-opts-40ce8f4618c786ef.yaml diff --git a/ironic_inspector/cmd/all.py b/ironic_inspector/cmd/all.py index c300cca3d..c88459b4c 100644 --- a/ironic_inspector/cmd/all.py +++ b/ironic_inspector/cmd/all.py @@ -14,16 +14,24 @@ import sys +from oslo_config import cfg +from oslo_service import service + +from ironic_inspector.common.rpc_service import RPCService from ironic_inspector.common import service_utils from ironic_inspector import wsgi_service +CONF = cfg.CONF + def main(args=sys.argv[1:]): # Parse config file and command line options, then start logging service_utils.prepare_service(args) - server = wsgi_service.WSGIService() - server.run() + launcher = service.ServiceLauncher(CONF, restart_method='mutate') + launcher.launch_service(wsgi_service.WSGIService()) + launcher.launch_service(RPCService(CONF.host)) + launcher.wait() if __name__ == '__main__': diff --git a/ironic_inspector/common/service_utils.py b/ironic_inspector/common/service_utils.py index ba09428b9..2f37cb984 100644 --- a/ironic_inspector/common/service_utils.py +++ b/ironic_inspector/common/service_utils.py @@ -12,6 +12,7 @@ from oslo_config import cfg from oslo_log import log +from oslo_service import sslutils from ironic_inspector.conf import opts @@ -26,5 +27,10 @@ def prepare_service(args=None): opts.parse_args(args) log.setup(CONF, 'ironic_inspector') + # TODO(kaifeng) Remove deprecated options at T* cycle. + sslutils.register_opts(CONF) + CONF.set_default('cert_file', CONF.ssl_cert_path, group='ssl') + CONF.set_default('key_file', CONF.ssl_key_path, group='ssl') + LOG.debug("Configuration:") CONF.log_opt_values(LOG, log.DEBUG) diff --git a/ironic_inspector/conf/default.py b/ironic_inspector/conf/default.py index ef2b9c2d5..5567fb5d5 100644 --- a/ironic_inspector/conf/default.py +++ b/ironic_inspector/conf/default.py @@ -52,9 +52,15 @@ _OPTS = [ help=_('SSL Enabled/Disabled')), cfg.StrOpt('ssl_cert_path', default='', + deprecated_for_removal=True, + deprecated_reason=_('This option will be superseded by ' + '[ssl]cert_file.'), help=_('Path to SSL certificate')), cfg.StrOpt('ssl_key_path', default='', + deprecated_for_removal=True, + deprecated_reason=_('This option will be superseded by ' + '[ssl]key_file.'), help=_('Path to SSL key')), cfg.IntOpt('max_concurrency', default=1000, min=2, @@ -78,7 +84,7 @@ _OPTS = [ help=_('Whether the current installation of ironic-inspector ' 'can manage PXE booting of nodes. If set to False, ' 'the API will reject introspection requests with ' - 'manage_boot missing or set to True.')) + 'manage_boot missing or set to True.')), ] diff --git a/ironic_inspector/test/unit/test_wsgi_service.py b/ironic_inspector/test/unit/test_wsgi_service.py index 3cb83dbaa..7f39c55bf 100644 --- a/ironic_inspector/test/unit/test_wsgi_service.py +++ b/ironic_inspector/test/unit/test_wsgi_service.py @@ -11,15 +11,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import ssl -import sys -import unittest - import eventlet # noqa import fixtures import mock from oslo_config import cfg +from ironic_inspector.common import service_utils from ironic_inspector.test import base as test_base from ironic_inspector import wsgi_service @@ -32,9 +29,12 @@ class BaseWSGITest(test_base.BaseTest): super(BaseWSGITest, self).setUp() self.app = self.useFixture(fixtures.MockPatchObject( wsgi_service.app, 'app', autospec=True)).mock + self.server = self.useFixture(fixtures.MockPatchObject( + wsgi_service.wsgi, 'Server', autospec=True)).mock self.mock_log = self.useFixture(fixtures.MockPatchObject( wsgi_service, 'LOG')).mock self.service = wsgi_service.WSGIService() + self.service.server = self.server class TestWSGIServiceInitMiddleware(BaseWSGITest): @@ -66,174 +66,55 @@ class TestWSGIServiceInitMiddleware(BaseWSGITest): self.mock_add_cors_middleware.assert_called_once_with(self.app) -class TestWSGIServiceRun(BaseWSGITest): +class TestWSGIService(BaseWSGITest): def setUp(self): - super(TestWSGIServiceRun, self).setUp() + super(TestWSGIService, self).setUp() self.mock__init_middleware = self.useFixture(fixtures.MockPatchObject( self.service, '_init_middleware')).mock - self.mock__create_ssl_context = self.useFixture( - fixtures.MockPatchObject(self.service, '_create_ssl_context')).mock - self.mock_shutdown = self.useFixture(fixtures.MockPatchObject( - self.service, 'shutdown')).mock # 'positive' settings CONF.set_override('listen_address', '42.42.42.42') CONF.set_override('listen_port', 42) - def test_run(self): - self.service.run() + def test_start(self): + self.service.start() - self.mock__create_ssl_context.assert_called_once_with() self.mock__init_middleware.assert_called_once_with() - self.app.run.assert_called_once_with( - host=CONF.listen_address, port=CONF.listen_port, - ssl_context=self.mock__create_ssl_context.return_value) - self.mock_shutdown.assert_called_once_with() + self.server.start.assert_called_once_with() - def test_run_no_ssl_context(self): - self.mock__create_ssl_context.return_value = None + def test_stop(self): + self.service.stop() + self.server.stop.assert_called_once_with() - self.service.run() - self.mock__create_ssl_context.assert_called_once_with() - self.mock__init_middleware.assert_called_once_with() - self.app.run.assert_called_once_with( - host=CONF.listen_address, port=CONF.listen_port) - self.mock_shutdown.assert_called_once_with() + def test_wait(self): + self.service.wait() + self.server.wait.assert_called_once_with() - def test_run_app_error(self): - class MyError(Exception): - pass - - error = MyError('Oops!') - self.app.run.side_effect = error - self.service.run() - - self.mock__create_ssl_context.assert_called_once_with() - self.mock__init_middleware.assert_called_once_with() - self.app.run.assert_called_once_with( - host=CONF.listen_address, port=CONF.listen_port, - ssl_context=self.mock__create_ssl_context.return_value) - self.mock_shutdown.assert_called_once_with(error=str(error)) + def test_reset(self): + self.service.reset() + self.server.reset.assert_called_once_with() -class TestWSGIServiceShutdown(BaseWSGITest): - def setUp(self): - super(TestWSGIServiceShutdown, self).setUp() - self.service = wsgi_service.WSGIService() - self.mock_rpc_service = mock.MagicMock() - self.service.rpc_service = self.mock_rpc_service - self.mock_exit = self.useFixture(fixtures.MockPatchObject( - wsgi_service.sys, 'exit')).mock +@mock.patch.object(service_utils.log, 'register_options', autospec=True) +class TestSSLOptions(test_base.BaseTest): - def test_shutdown(self): - class MyError(Exception): - pass - error = MyError('Oops!') + def test_use_deprecated_options(self, mock_log): + CONF.set_override('ssl_cert_path', 'fake_cert_file') + CONF.set_override('ssl_key_path', 'fake_key_file') - self.service.shutdown(error=error) - self.mock_rpc_service.stop.assert_called_once_with() - self.mock_exit.assert_called_once_with(error) + service_utils.prepare_service() + self.assertEqual(CONF.ssl.cert_file, 'fake_cert_file') + self.assertEqual(CONF.ssl.key_file, 'fake_key_file') -class TestCreateSSLContext(test_base.BaseTest): - def setUp(self): - super(TestCreateSSLContext, self).setUp() - self.app = mock.Mock() - self.service = wsgi_service.WSGIService() + def test_use_ssl_options(self, mock_log): + CONF.set_override('ssl_cert_path', 'fake_cert_file') + CONF.set_override('ssl_key_path', 'fake_key_file') - def test_use_ssl_false(self): - CONF.set_override('use_ssl', False) - con = self.service._create_ssl_context() - self.assertIsNone(con) + service_utils.prepare_service() - @mock.patch.object(sys, 'version_info') - def test_old_python_returns_none(self, mock_version_info): - mock_version_info.__lt__.return_value = True - CONF.set_override('use_ssl', True) - con = self.service._create_ssl_context() - self.assertIsNone(con) + CONF.set_override('cert_file', 'fake_new_cert', 'ssl') + CONF.set_override('key_file', 'fake_new_key', 'ssl') - @unittest.skipIf(sys.version_info[:3] < (2, 7, 9), - 'This feature is unsupported in this version of python ' - 'so the tests will be skipped') - @mock.patch.object(ssl, 'create_default_context', autospec=True) - def test_use_ssl_true(self, mock_cdc): - CONF.set_override('use_ssl', True) - m_con = mock_cdc() - con = self.service._create_ssl_context() - self.assertEqual(m_con, con) - - @unittest.skipIf(sys.version_info[:3] < (2, 7, 9), - 'This feature is unsupported in this version of python ' - 'so the tests will be skipped') - @mock.patch.object(ssl, 'create_default_context', autospec=True) - def test_only_key_path_provided(self, mock_cdc): - CONF.set_override('use_ssl', True) - CONF.set_override('ssl_key_path', '/some/fake/path') - mock_context = mock_cdc() - con = self.service._create_ssl_context() - self.assertEqual(mock_context, con) - self.assertFalse(mock_context.load_cert_chain.called) - - @unittest.skipIf(sys.version_info[:3] < (2, 7, 9), - 'This feature is unsupported in this version of python ' - 'so the tests will be skipped') - @mock.patch.object(ssl, 'create_default_context', autospec=True) - def test_only_cert_path_provided(self, mock_cdc): - CONF.set_override('use_ssl', True) - CONF.set_override('ssl_cert_path', '/some/fake/path') - mock_context = mock_cdc() - con = self.service._create_ssl_context() - self.assertEqual(mock_context, con) - self.assertFalse(mock_context.load_cert_chain.called) - - @unittest.skipIf(sys.version_info[:3] < (2, 7, 9), - 'This feature is unsupported in this version of python ' - 'so the tests will be skipped') - @mock.patch.object(ssl, 'create_default_context', autospec=True) - def test_both_paths_provided(self, mock_cdc): - key_path = '/some/fake/path/key' - cert_path = '/some/fake/path/cert' - CONF.set_override('use_ssl', True) - CONF.set_override('ssl_key_path', key_path) - CONF.set_override('ssl_cert_path', cert_path) - mock_context = mock_cdc() - con = self.service._create_ssl_context() - self.assertEqual(mock_context, con) - mock_context.load_cert_chain.assert_called_once_with(cert_path, - key_path) - - @unittest.skipIf(sys.version_info[:3] < (2, 7, 9), - 'This feature is unsupported in this version of python ' - 'so the tests will be skipped') - @mock.patch.object(ssl, 'create_default_context', autospec=True) - def test_load_cert_chain_fails(self, mock_cdc): - CONF.set_override('use_ssl', True) - key_path = '/some/fake/path/key' - cert_path = '/some/fake/path/cert' - CONF.set_override('use_ssl', True) - CONF.set_override('ssl_key_path', key_path) - CONF.set_override('ssl_cert_path', cert_path) - mock_context = mock_cdc() - mock_context.load_cert_chain.side_effect = IOError('Boom!') - con = self.service._create_ssl_context() - self.assertEqual(mock_context, con) - mock_context.load_cert_chain.assert_called_once_with(cert_path, - key_path) - - -class TestWSGIServiceOnSigHup(BaseWSGITest): - def setUp(self): - super(TestWSGIServiceOnSigHup, self).setUp() - self.mock_spawn = self.useFixture(fixtures.MockPatchObject( - wsgi_service.eventlet, 'spawn')).mock - self.mock_mutate_conf = self.useFixture(fixtures.MockPatchObject( - wsgi_service.CONF, 'mutate_config_files')).mock - - def test_on_sighup(self): - self.service._handle_sighup() - self.mock_spawn.assert_called_once_with(self.service._handle_sighup_bg) - - def test_on_sighup_bg(self): - self.service._handle_sighup_bg() - self.mock_mutate_conf.assert_called_once_with() + self.assertEqual(CONF.ssl.cert_file, 'fake_new_cert') + self.assertEqual(CONF.ssl.key_file, 'fake_new_key') diff --git a/ironic_inspector/wsgi_service.py b/ironic_inspector/wsgi_service.py index 8513e16f6..b42e9c9a8 100644 --- a/ironic_inspector/wsgi_service.py +++ b/ironic_inspector/wsgi_service.py @@ -10,16 +10,11 @@ # License for the specific language governing permissions and limitations # under the License. -import signal -import ssl -import sys - -import eventlet from oslo_config import cfg from oslo_log import log from oslo_service import service +from oslo_service import wsgi -from ironic_inspector.common.rpc_service import RPCService from ironic_inspector import main as app from ironic_inspector import utils @@ -27,21 +22,22 @@ LOG = log.getLogger(__name__) CONF = cfg.CONF -class WSGIService(object): +class WSGIService(service.Service): """Provides ability to launch API from wsgi app.""" def __init__(self): self.app = app.app - signal.signal(signal.SIGHUP, self._handle_sighup) - signal.signal(signal.SIGTERM, self._handle_sigterm) - self.rpc_service = RPCService(CONF.host) + self.server = wsgi.Server(CONF, 'ironic_inspector', + self.app, + host=CONF.listen_address, + port=CONF.listen_port, + use_ssl=CONF.use_ssl) def _init_middleware(self): """Initialize WSGI middleware. :returns: None """ - if CONF.auth_strategy != 'noauth': utils.add_auth_middleware(self.app) else: @@ -49,80 +45,31 @@ class WSGIService(object): ' configuration') utils.add_cors_middleware(self.app) - def _create_ssl_context(self): - if not CONF.use_ssl: - return - - MIN_VERSION = (2, 7, 9) - - if sys.version_info < MIN_VERSION: - LOG.warning(('Unable to use SSL in this version of Python: ' - '%(current)s, please ensure your version of Python ' - 'is greater than %(min)s to enable this feature.'), - {'current': '.'.join(map(str, sys.version_info[:3])), - 'min': '.'.join(map(str, MIN_VERSION))}) - return - - context = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH) - if CONF.ssl_cert_path and CONF.ssl_key_path: - try: - context.load_cert_chain(CONF.ssl_cert_path, CONF.ssl_key_path) - except IOError as exc: - LOG.warning('Failed to load certificate or key from defined ' - 'locations: %(cert)s and %(key)s, will continue ' - 'to run with the default settings: %(exc)s', - {'cert': CONF.ssl_cert_path, - 'key': CONF.ssl_key_path, - 'exc': exc}) - except ssl.SSLError as exc: - LOG.warning('There was a problem with the loaded certificate ' - 'and key, will continue to run with the default ' - 'settings: %s', exc) - return context - - def shutdown(self, error=None): - """Stop serving API. + def start(self): + """Start serving this service using loaded configuration. :returns: None """ - LOG.debug('Shutting down') - self.rpc_service.stop() - sys.exit(error) - - def run(self): - """Start serving this service using loaded application. - - :returns: None - """ - app_kwargs = {'host': CONF.listen_address, - 'port': CONF.listen_port} - - context = self._create_ssl_context() - if context: - app_kwargs['ssl_context'] = context - self._init_middleware() + self.server.start() - LOG.info('Spawning RPC service') - service.launch(CONF, self.rpc_service, - restart_method='mutate') + def stop(self): + """Stop serving this API. - try: - self.app.run(**app_kwargs) - except Exception as e: - self.shutdown(error=str(e)) - else: - self.shutdown() + :returns: None + """ + self.server.stop() - def _handle_sighup_bg(self, *args): - """Reload config on SIGHUP.""" - CONF.mutate_config_files() + def wait(self): + """Wait for the service to stop serving this API. - def _handle_sighup(self, *args): - eventlet.spawn(self._handle_sighup_bg, *args) + :returns: None + """ + self.server.wait() - def _handle_sigterm(self, *args): - # This is a workaround to ensure that shutdown() is done when recieving - # SIGTERM. Raising KeyboardIntrerrupt which won't be caught by any - # 'except Exception' clauses. - raise KeyboardInterrupt + def reset(self): + """Reset server greenpool size to default. + + :returns: None + """ + self.server.reset() diff --git a/lower-constraints.txt b/lower-constraints.txt index dc3c40fff..64a46e881 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -73,7 +73,7 @@ oslo.middleware==3.31.0 oslo.policy==1.30.0 oslo.rootwrap==5.8.0 oslo.serialization==2.18.0 -oslo.service==1.30.0 +oslo.service==1.24.0 oslo.utils==3.33.0 oslotest==3.2.0 packaging==17.1 diff --git a/releasenotes/notes/deprecate-ssl-opts-40ce8f4618c786ef.yaml b/releasenotes/notes/deprecate-ssl-opts-40ce8f4618c786ef.yaml new file mode 100644 index 000000000..9c456f76a --- /dev/null +++ b/releasenotes/notes/deprecate-ssl-opts-40ce8f4618c786ef.yaml @@ -0,0 +1,7 @@ +--- +deprecations: + - | + Configuration options ``[DEFAULT]ssl_cert_path`` and + ``[DEFAULT]ssl_key_path`` are deprecated for ironic-inspector now uses + oslo.service as underlying HTTP service instead of Werkzeug. Please use + ``[ssl]cert_file`` and ``[ssl]key_file``. \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index ef6c0557e..c0f4edc8a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,6 +29,7 @@ oslo.middleware>=3.31.0 # Apache-2.0 oslo.policy>=1.30.0 # Apache-2.0 oslo.rootwrap>=5.8.0 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 +oslo.service!=1.28.1,>=1.24.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 retrying!=1.3.0,>=1.2.3 # Apache-2.0 six>=1.10.0 # MIT diff --git a/tools/config-generator.conf b/tools/config-generator.conf index 47fd222cf..b2663b9b1 100644 --- a/tools/config-generator.conf +++ b/tools/config-generator.conf @@ -4,6 +4,9 @@ namespace = ironic_inspector namespace = keystonemiddleware.auth_token namespace = oslo.db namespace = oslo.log +namespace = oslo.messaging namespace = oslo.middleware.cors namespace = oslo.policy -namespace = oslo.messaging +namespace = oslo.service.service +namespace = oslo.service.sslutils +namespace = oslo.service.wsgi \ No newline at end of file