openstack/ironic-inspector
Code Issues Proposed changes

Deprecate authenticate opt in favor of auth_strategy

Browse Source 
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
This commit is contained in:
Sam Betts 2015-06-24 14:31:55 +01:00
parent dcbe621e37
commit 9654a066de
6 changed files with 36 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
example.conf
View File

@ -12,11 +12,17 @@
# Deprecated group/name - [discoverd]/listen_port # Deprecated group/name - [discoverd]/listen_port
#listen_port = 5050 #listen_port = 5050
# Whether to authenticate with Keystone on public HTTP endpoints. Note # Authentication method used on the ironic-inspector API. Either
# that introspection ramdisk postback endpoint is never authenticated. # "noauth" or "keystone" are currently valid options. "noauth" will
# (boolean value) # disable all authentication. (string value)
# Allowed values: keystone, noauth
#auth_strategy = keystone
# DEPRECATED: use auth_strategy. (boolean value)
# Deprecated group/name - [discoverd]/authenticate # Deprecated group/name - [discoverd]/authenticate
#authenticate = true # This option is deprecated for removal.
# Its value may be silently ignored in the future.
#authenticate = <None>
# SQLite3 database to store nodes under introspection, required. Do # SQLite3 database to store nodes under introspection, required. Do
# not use :memory: here, it won't work. (string value) # not use :memory: here, it won't work. (string value)

15
ironic_inspector/conf.py
View File

@ -152,12 +152,17 @@ SERVICE_OPTS = [
default=5050, default=5050,
help='Port to listen on.', help='Port to listen on.',
deprecated_group='discoverd'), 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', cfg.BoolOpt('authenticate',
default=True, default=None,
help='Whether to authenticate with Keystone on public HTTP ' help='DEPRECATED: use auth_strategy.',
'endpoints. Note that introspection ramdisk postback ' deprecated_group='discoverd',
'endpoint is never authenticated.', deprecated_for_removal=True),
deprecated_group='discoverd'),
cfg.StrOpt('database', cfg.StrOpt('database',
default='', default='',
help='SQLite3 database to store nodes under introspection, ' help='SQLite3 database to store nodes under introspection, '

2
ironic_inspector/main.py
View File

@ -121,7 +121,7 @@ def periodic_clean_up(period): # pragma: no cover
def init(): def init():
if CONF.authenticate: if utils.get_auth_strategy() != 'noauth':
utils.add_auth_middleware(app) utils.add_auth_middleware(app)
else: else:
LOG.warning(_LW('Starting unauthenticated, please check' LOG.warning(_LW('Starting unauthenticated, please check'

14
ironic_inspector/test/test_main.py
View File

@ -38,12 +38,12 @@ class TestApi(test_base.BaseTest):
super(TestApi, self).setUp() super(TestApi, self).setUp()
main.app.config['TESTING'] = True main.app.config['TESTING'] = True
self.app = main.app.test_client() self.app = main.app.test_client()
CONF.set_override('authenticate', False) CONF.set_override('auth_strategy', 'noauth')
self.uuid = uuidutils.generate_uuid() self.uuid = uuidutils.generate_uuid()
@mock.patch.object(introspect, 'introspect', autospec=True) @mock.patch.object(introspect, 'introspect', autospec=True)
def test_introspect_no_authentication(self, introspect_mock): 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) res = self.app.post('/v1/introspection/%s' % self.uuid)
self.assertEqual(202, res.status_code) self.assertEqual(202, res.status_code)
introspect_mock.assert_called_once_with(self.uuid, introspect_mock.assert_called_once_with(self.uuid,
@ -51,7 +51,6 @@ class TestApi(test_base.BaseTest):
@mock.patch.object(introspect, 'introspect', autospec=True) @mock.patch.object(introspect, 'introspect', autospec=True)
def test_introspect_set_ipmi_credentials(self, introspect_mock): 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&' res = self.app.post('/v1/introspection/%s?new_ipmi_username=user&'
'new_ipmi_password=password' % self.uuid) 'new_ipmi_password=password' % self.uuid)
self.assertEqual(202, res.status_code) self.assertEqual(202, res.status_code)
@ -61,7 +60,6 @@ class TestApi(test_base.BaseTest):
@mock.patch.object(introspect, 'introspect', autospec=True) @mock.patch.object(introspect, 'introspect', autospec=True)
def test_introspect_set_ipmi_credentials_no_user(self, introspect_mock): def test_introspect_set_ipmi_credentials_no_user(self, introspect_mock):
CONF.set_override('authenticate', False)
res = self.app.post('/v1/introspection/%s?' res = self.app.post('/v1/introspection/%s?'
'new_ipmi_password=password' % self.uuid) 'new_ipmi_password=password' % self.uuid)
self.assertEqual(202, res.status_code) self.assertEqual(202, res.status_code)
@ -85,7 +83,7 @@ class TestApi(test_base.BaseTest):
@mock.patch.object(introspect, 'introspect', autospec=True) @mock.patch.object(introspect, 'introspect', autospec=True)
def test_introspect_failed_authentication(self, introspect_mock, def test_introspect_failed_authentication(self, introspect_mock,
auth_mock): auth_mock):
CONF.set_override('authenticate', True) CONF.set_override('auth_strategy', 'keystone')
auth_mock.side_effect = utils.Error('Boom', code=403) auth_mock.side_effect = utils.Error('Boom', code=403)
res = self.app.post('/v1/introspection/%s' % self.uuid, res = self.app.post('/v1/introspection/%s' % self.uuid,
headers={'X-Auth-Token': 'token'}) headers={'X-Auth-Token': 'token'})
@ -101,7 +99,7 @@ class TestApi(test_base.BaseTest):
@mock.patch.object(process, 'process', autospec=True) @mock.patch.object(process, 'process', autospec=True)
def test_continue(self, process_mock): def test_continue(self, process_mock):
# should be ignored # should be ignored
CONF.set_override('authenticate', True) CONF.set_override('auth_strategy', 'keystone')
process_mock.return_value = [42] process_mock.return_value = [42]
res = self.app.post('/v1/continue', data='"JSON"') res = self.app.post('/v1/continue', data='"JSON"')
self.assertEqual(200, res.status_code) self.assertEqual(200, res.status_code)
@ -166,7 +164,7 @@ class TestPlugins(unittest.TestCase):
class TestInit(test_base.BaseTest): class TestInit(test_base.BaseTest):
def test_ok(self, mock_node_cache, mock_get_client, mock_auth, def test_ok(self, mock_node_cache, mock_get_client, mock_auth,
mock_firewall, mock_spawn_n): mock_firewall, mock_spawn_n):
CONF.set_override('authenticate', True) CONF.set_override('auth_strategy', 'keystone')
main.init() main.init()
mock_auth.assert_called_once_with(main.app) mock_auth.assert_called_once_with(main.app)
mock_node_cache.assert_called_once_with() 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, def test_init_without_authenticate(self, mock_node_cache, mock_get_client,
mock_auth, mock_firewall, mock_spawn_n): mock_auth, mock_firewall, mock_spawn_n):
CONF.set_override('authenticate', False) CONF.set_override('auth_strategy', 'noauth')
main.init() main.init()
self.assertFalse(mock_auth.called) self.assertFalse(mock_auth.called)

4
ironic_inspector/test/test_utils.py
View File

@ -32,7 +32,7 @@ CONF = cfg.CONF
class TestCheckAuth(base.BaseTest): class TestCheckAuth(base.BaseTest):
def setUp(self): def setUp(self):
super(TestCheckAuth, self).setUp() super(TestCheckAuth, self).setUp()
CONF.set_override('authenticate', True) CONF.set_override('auth_strategy', 'keystone')
@mock.patch.object(auth_token, 'AuthProtocol') @mock.patch.object(auth_token, 'AuthProtocol')
def test_middleware(self, mock_auth): def test_middleware(self, mock_auth):
@ -103,7 +103,7 @@ class TestCheckAuth(base.BaseTest):
self.assertRaises(utils.Error, utils.check_auth, request) self.assertRaises(utils.Error, utils.check_auth, request)
def test_disabled(self): def test_disabled(self):
CONF.set_override('authenticate', False) CONF.set_override('auth_strategy', 'noauth')
request = mock.Mock(headers={'X-Identity-Status': 'Invalid'}) request = mock.Mock(headers={'X-Identity-Status': 'Invalid'})
utils.check_auth(request) utils.check_auth(request)

8
ironic_inspector/utils.py
View File

@ -113,7 +113,7 @@ def check_auth(request):
:param request: Flask request :param request: Flask request
:raises: utils.Error if access is denied :raises: utils.Error if access is denied
""" """
if not CONF.authenticate: if get_auth_strategy() == 'noauth':
return return
if request.headers.get('X-Identity-Status').lower() == 'invalid': if request.headers.get('X-Identity-Status').lower() == 'invalid':
raise Error(_('Authentication required'), code=401) raise Error(_('Authentication required'), code=401)
@ -130,6 +130,12 @@ def is_valid_mac(address):
and re.match(m, address.lower())) 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): def get_ipmi_address(node):
# All these are kind-of-ipmi # All these are kind-of-ipmi
for name in ('ipmi_address', 'ilo_address', 'drac_host'): for name in ('ipmi_address', 'ilo_address', 'drac_host'):