From 9654a066def93014775b3dc2d74470b03842dc6e Mon Sep 17 00:00:00 2001 From: Sam Betts Date: Wed, 24 Jun 2015 14:31:55 +0100 Subject: [PATCH] Deprecate authenticate opt in favor of auth_strategy This patch deprecates the authenticate configuration option in favor of a more ironic like auth_strategy option. Those already using the authenticate option shouldn't be affected by this change however the authenticate option should be completely removed in the future. Change-Id: If9b1f24a8a9a73c93d46d884a24658ac9556ced2 Closes-Bug: #1462365 --- example.conf | 14 ++++++++++---- ironic_inspector/conf.py | 15 ++++++++++----- ironic_inspector/main.py | 2 +- ironic_inspector/test/test_main.py | 14 ++++++-------- ironic_inspector/test/test_utils.py | 4 ++-- ironic_inspector/utils.py | 8 +++++++- 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/example.conf b/example.conf index debde18da..60436cfe1 100644 --- a/example.conf +++ b/example.conf @@ -12,11 +12,17 @@ # Deprecated group/name - [discoverd]/listen_port #listen_port = 5050 -# Whether to authenticate with Keystone on public HTTP endpoints. Note -# that introspection ramdisk postback endpoint is never authenticated. -# (boolean value) +# Authentication method used on the ironic-inspector API. Either +# "noauth" or "keystone" are currently valid options. "noauth" will +# disable all authentication. (string value) +# Allowed values: keystone, noauth +#auth_strategy = keystone + +# DEPRECATED: use auth_strategy. (boolean value) # Deprecated group/name - [discoverd]/authenticate -#authenticate = true +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +#authenticate = # SQLite3 database to store nodes under introspection, required. Do # not use :memory: here, it won't work. (string value) diff --git a/ironic_inspector/conf.py b/ironic_inspector/conf.py index a9a9e8d84..09e0425bd 100644 --- a/ironic_inspector/conf.py +++ b/ironic_inspector/conf.py @@ -152,12 +152,17 @@ SERVICE_OPTS = [ default=5050, help='Port to listen on.', deprecated_group='discoverd'), + cfg.StrOpt('auth_strategy', + default='keystone', + choices=('keystone', 'noauth'), + help='Authentication method used on the ironic-inspector ' + 'API. Either "noauth" or "keystone" are currently valid ' + 'options. "noauth" will disable all authentication.'), cfg.BoolOpt('authenticate', - default=True, - help='Whether to authenticate with Keystone on public HTTP ' - 'endpoints. Note that introspection ramdisk postback ' - 'endpoint is never authenticated.', - deprecated_group='discoverd'), + default=None, + help='DEPRECATED: use auth_strategy.', + deprecated_group='discoverd', + deprecated_for_removal=True), cfg.StrOpt('database', default='', help='SQLite3 database to store nodes under introspection, ' diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index dab4d61b5..4eaa36fab 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -121,7 +121,7 @@ def periodic_clean_up(period): # pragma: no cover def init(): - if CONF.authenticate: + if utils.get_auth_strategy() != 'noauth': utils.add_auth_middleware(app) else: LOG.warning(_LW('Starting unauthenticated, please check' diff --git a/ironic_inspector/test/test_main.py b/ironic_inspector/test/test_main.py index a65279fa6..acc44e32a 100644 --- a/ironic_inspector/test/test_main.py +++ b/ironic_inspector/test/test_main.py @@ -38,12 +38,12 @@ class TestApi(test_base.BaseTest): super(TestApi, self).setUp() main.app.config['TESTING'] = True self.app = main.app.test_client() - CONF.set_override('authenticate', False) + CONF.set_override('auth_strategy', 'noauth') self.uuid = uuidutils.generate_uuid() @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_no_authentication(self, introspect_mock): - CONF.set_override('authenticate', False) + CONF.set_override('auth_strategy', 'noauth') res = self.app.post('/v1/introspection/%s' % self.uuid) self.assertEqual(202, res.status_code) introspect_mock.assert_called_once_with(self.uuid, @@ -51,7 +51,6 @@ class TestApi(test_base.BaseTest): @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_set_ipmi_credentials(self, introspect_mock): - CONF.set_override('authenticate', False) res = self.app.post('/v1/introspection/%s?new_ipmi_username=user&' 'new_ipmi_password=password' % self.uuid) self.assertEqual(202, res.status_code) @@ -61,7 +60,6 @@ class TestApi(test_base.BaseTest): @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_set_ipmi_credentials_no_user(self, introspect_mock): - CONF.set_override('authenticate', False) res = self.app.post('/v1/introspection/%s?' 'new_ipmi_password=password' % self.uuid) self.assertEqual(202, res.status_code) @@ -85,7 +83,7 @@ class TestApi(test_base.BaseTest): @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_failed_authentication(self, introspect_mock, auth_mock): - CONF.set_override('authenticate', True) + CONF.set_override('auth_strategy', 'keystone') auth_mock.side_effect = utils.Error('Boom', code=403) res = self.app.post('/v1/introspection/%s' % self.uuid, headers={'X-Auth-Token': 'token'}) @@ -101,7 +99,7 @@ class TestApi(test_base.BaseTest): @mock.patch.object(process, 'process', autospec=True) def test_continue(self, process_mock): # should be ignored - CONF.set_override('authenticate', True) + CONF.set_override('auth_strategy', 'keystone') process_mock.return_value = [42] res = self.app.post('/v1/continue', data='"JSON"') self.assertEqual(200, res.status_code) @@ -166,7 +164,7 @@ class TestPlugins(unittest.TestCase): class TestInit(test_base.BaseTest): def test_ok(self, mock_node_cache, mock_get_client, mock_auth, mock_firewall, mock_spawn_n): - CONF.set_override('authenticate', True) + CONF.set_override('auth_strategy', 'keystone') main.init() mock_auth.assert_called_once_with(main.app) mock_node_cache.assert_called_once_with() @@ -183,7 +181,7 @@ class TestInit(test_base.BaseTest): def test_init_without_authenticate(self, mock_node_cache, mock_get_client, mock_auth, mock_firewall, mock_spawn_n): - CONF.set_override('authenticate', False) + CONF.set_override('auth_strategy', 'noauth') main.init() self.assertFalse(mock_auth.called) diff --git a/ironic_inspector/test/test_utils.py b/ironic_inspector/test/test_utils.py index fa0a54d13..263e29e77 100644 --- a/ironic_inspector/test/test_utils.py +++ b/ironic_inspector/test/test_utils.py @@ -32,7 +32,7 @@ CONF = cfg.CONF class TestCheckAuth(base.BaseTest): def setUp(self): super(TestCheckAuth, self).setUp() - CONF.set_override('authenticate', True) + CONF.set_override('auth_strategy', 'keystone') @mock.patch.object(auth_token, 'AuthProtocol') def test_middleware(self, mock_auth): @@ -103,7 +103,7 @@ class TestCheckAuth(base.BaseTest): self.assertRaises(utils.Error, utils.check_auth, request) def test_disabled(self): - CONF.set_override('authenticate', False) + CONF.set_override('auth_strategy', 'noauth') request = mock.Mock(headers={'X-Identity-Status': 'Invalid'}) utils.check_auth(request) diff --git a/ironic_inspector/utils.py b/ironic_inspector/utils.py index 2cf1eff98..ed26f65f0 100644 --- a/ironic_inspector/utils.py +++ b/ironic_inspector/utils.py @@ -113,7 +113,7 @@ def check_auth(request): :param request: Flask request :raises: utils.Error if access is denied """ - if not CONF.authenticate: + if get_auth_strategy() == 'noauth': return if request.headers.get('X-Identity-Status').lower() == 'invalid': raise Error(_('Authentication required'), code=401) @@ -130,6 +130,12 @@ def is_valid_mac(address): and re.match(m, address.lower())) +def get_auth_strategy(): + if CONF.authenticate is not None: + return 'keystone' if CONF.authenticate else 'noauth' + return CONF.auth_strategy + + def get_ipmi_address(node): # All these are kind-of-ipmi for name in ('ipmi_address', 'ilo_address', 'drac_host'):