diff --git a/ironic_discoverd/main.py b/ironic_discoverd/main.py index 73dd262ca..6e111202d 100644 --- a/ironic_discoverd/main.py +++ b/ironic_discoverd/main.py @@ -18,6 +18,7 @@ import argparse import functools import json import logging +from oslo_utils import uuidutils import sys import flask @@ -77,6 +78,9 @@ def api_continue(): def api_introspection(uuid): check_auth() + if not uuidutils.is_uuid_like(uuid): + raise utils.Error(_('Invalid UUID value'), code=400) + if flask.request.method == 'POST': new_ipmi_password = flask.request.args.get('new_ipmi_password', type=str, @@ -105,6 +109,10 @@ def api_discover(): data = flask.request.get_json(force=True) LOG.debug("/v1/discover got JSON %s", data) + for uuid in data: + if not uuidutils.is_uuid_like(uuid): + raise utils.Error(_('Invalid UUID value'), code=400) + for uuid in data: introspect.introspect(uuid) return "", 202 diff --git a/ironic_discoverd/test/base.py b/ironic_discoverd/test/base.py index 89b2c566e..bccb85816 100644 --- a/ironic_discoverd/test/base.py +++ b/ironic_discoverd/test/base.py @@ -48,7 +48,7 @@ class BaseTest(unittest.TestCase): class NodeTest(BaseTest): def setUp(self): super(NodeTest, self).setUp() - self.uuid = 'uuid' + self.uuid = '1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e' self.bmc_address = '1.2.3.4' self.macs = ['11:22:33:44:55:66', '66:55:44:33:22:11'] self.node = mock.Mock(driver='pxe_ipmitool', diff --git a/ironic_discoverd/test/test_client.py b/ironic_discoverd/test/test_client.py index afddf6168..13276a0f2 100644 --- a/ironic_discoverd/test/test_client.py +++ b/ironic_discoverd/test/test_client.py @@ -14,17 +14,22 @@ import unittest import mock +from oslo_utils import uuidutils from ironic_discoverd import client @mock.patch.object(client.requests, 'post', autospec=True) class TestIntrospect(unittest.TestCase): + def setUp(self): + super(TestIntrospect, self).setUp() + self.uuid = uuidutils.generate_uuid() + def test(self, mock_post): - client.introspect('uuid1', base_url="http://host:port", + client.introspect(self.uuid, base_url="http://host:port", auth_token="token") mock_post.assert_called_once_with( - "http://host:port/v1/introspection/uuid1", + "http://host:port/v1/introspection/%s" % self.uuid, headers={'X-Auth-Token': 'token'}, params={'new_ipmi_username': None, 'new_ipmi_password': None} ) @@ -35,28 +40,28 @@ class TestIntrospect(unittest.TestCase): new_ipmi_username='user') def test_full_url(self, mock_post): - client.introspect('uuid1', base_url="http://host:port/v1/", + client.introspect(self.uuid, base_url="http://host:port/v1/", auth_token="token") mock_post.assert_called_once_with( - "http://host:port/v1/introspection/uuid1", + "http://host:port/v1/introspection/%s" % self.uuid, headers={'X-Auth-Token': 'token'}, params={'new_ipmi_username': None, 'new_ipmi_password': None} ) def test_default_url(self, mock_post): - client.introspect('uuid1', auth_token="token") + client.introspect(self.uuid, auth_token="token") mock_post.assert_called_once_with( - "http://127.0.0.1:5050/v1/introspection/uuid1", + "http://127.0.0.1:5050/v1/introspection/%s" % self.uuid, headers={'X-Auth-Token': 'token'}, params={'new_ipmi_username': None, 'new_ipmi_password': None} ) def test_set_ipmi_credentials(self, mock_post): - client.introspect('uuid1', base_url="http://host:port", + client.introspect(self.uuid, base_url="http://host:port", auth_token="token", new_ipmi_password='p', new_ipmi_username='u') mock_post.assert_called_once_with( - "http://host:port/v1/introspection/uuid1", + "http://host:port/v1/introspection/%s" % self.uuid, headers={'X-Auth-Token': 'token'}, params={'new_ipmi_username': 'u', 'new_ipmi_password': 'p'} ) @@ -64,12 +69,18 @@ class TestIntrospect(unittest.TestCase): @mock.patch.object(client.requests, 'post', autospec=True) class TestDiscover(unittest.TestCase): + def setUp(self): + super(TestDiscover, self).setUp() + self.uuid = uuidutils.generate_uuid() + def test_old_discover(self, mock_post): - client.discover(['uuid1', 'uuid2'], base_url="http://host:port", + uuid2 = uuidutils.generate_uuid() + client.discover([self.uuid, uuid2], base_url="http://host:port", auth_token="token") mock_post.assert_called_once_with( "http://host:port/v1/discover", - data='["uuid1", "uuid2"]', + data='["%(uuid1)s", "%(uuid2)s"]' % {'uuid1': self.uuid, + 'uuid2': uuid2}, headers={'Content-Type': 'application/json', 'X-Auth-Token': 'token'} ) @@ -81,13 +92,17 @@ class TestDiscover(unittest.TestCase): @mock.patch.object(client.requests, 'get', autospec=True) class TestGetStatus(unittest.TestCase): + def setUp(self): + super(TestGetStatus, self).setUp() + self.uuid = uuidutils.generate_uuid() + def test(self, mock_get): mock_get.return_value.json.return_value = 'json' - client.get_status('uuid', auth_token='token') + client.get_status(self.uuid, auth_token='token') mock_get.assert_called_once_with( - "http://127.0.0.1:5050/v1/introspection/uuid", + "http://127.0.0.1:5050/v1/introspection/%s" % self.uuid, headers={'X-Auth-Token': 'token'} ) diff --git a/ironic_discoverd/test/test_introspect.py b/ironic_discoverd/test/test_introspect.py index 23fa99e65..31bbf7e74 100644 --- a/ironic_discoverd/test/test_introspect.py +++ b/ironic_discoverd/test/test_introspect.py @@ -248,7 +248,7 @@ class TestIntrospect(BaseTest): self.assertRaisesRegexp( utils.Error, - 'node uuid with provision state "active"', + 'node %s with provision state "active"' % self.uuid, introspect.introspect, self.uuid) self.assertEqual(0, cli.node.list_ports.call_count) @@ -264,7 +264,7 @@ class TestIntrospect(BaseTest): self.assertRaisesRegexp( utils.Error, - 'node uuid with power state "power on"', + 'node %s with power state "power on"' % self.uuid, introspect.introspect, self.uuid) self.assertEqual(0, cli.node.list_ports.call_count) diff --git a/ironic_discoverd/test/test_main.py b/ironic_discoverd/test/test_main.py index f449e7b2c..778985c4c 100644 --- a/ironic_discoverd/test/test_main.py +++ b/ironic_discoverd/test/test_main.py @@ -17,6 +17,7 @@ import unittest import eventlet from keystoneclient import exceptions as keystone_exc import mock +from oslo_utils import uuidutils from ironic_discoverd import conf from ironic_discoverd import introspect @@ -35,48 +36,50 @@ class TestApi(test_base.BaseTest): main.app.config['TESTING'] = True self.app = main.app.test_client() conf.CONF.set('discoverd', 'authenticate', 'false') + self.uuid = uuidutils.generate_uuid() @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_no_authentication(self, introspect_mock): conf.CONF.set('discoverd', 'authenticate', 'false') - res = self.app.post('/v1/introspection/uuid1') + res = self.app.post('/v1/introspection/%s' % self.uuid) self.assertEqual(202, res.status_code) - introspect_mock.assert_called_once_with("uuid1", + introspect_mock.assert_called_once_with(self.uuid, new_ipmi_credentials=None) @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_set_ipmi_credentials(self, introspect_mock): conf.CONF.set('discoverd', 'authenticate', 'false') - res = self.app.post('/v1/introspection/uuid1?new_ipmi_username=user&' - 'new_ipmi_password=password') + res = self.app.post('/v1/introspection/%s?new_ipmi_username=user&' + 'new_ipmi_password=password' % self.uuid) self.assertEqual(202, res.status_code) introspect_mock.assert_called_once_with( - "uuid1", + self.uuid, new_ipmi_credentials=('user', 'password')) @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_set_ipmi_credentials_no_user(self, introspect_mock): conf.CONF.set('discoverd', 'authenticate', 'false') - res = self.app.post('/v1/introspection/uuid1?' - 'new_ipmi_password=password') + res = self.app.post('/v1/introspection/%s?' + 'new_ipmi_password=password' % self.uuid) self.assertEqual(202, res.status_code) introspect_mock.assert_called_once_with( - "uuid1", + self.uuid, new_ipmi_credentials=(None, 'password')) @mock.patch.object(introspect, 'introspect', autospec=True) def test_intospect_failed(self, introspect_mock): introspect_mock.side_effect = utils.Error("boom") - res = self.app.post('/v1/introspection/uuid1') + res = self.app.post('/v1/introspection/%s' % self.uuid) self.assertEqual(400, res.status_code) self.assertEqual(b"boom", res.data) - introspect_mock.assert_called_once_with("uuid1", - new_ipmi_credentials=None) + introspect_mock.assert_called_once_with( + self.uuid, + new_ipmi_credentials=None) @mock.patch.object(introspect, 'introspect', autospec=True) def test_introspect_missing_authentication(self, introspect_mock): conf.CONF.set('discoverd', 'authenticate', 'true') - res = self.app.post('/v1/introspection/uuid1') + res = self.app.post('/v1/introspection/%s' % self.uuid) self.assertEqual(401, res.status_code) self.assertFalse(introspect_mock.called) @@ -86,17 +89,29 @@ class TestApi(test_base.BaseTest): keystone_mock): conf.CONF.set('discoverd', 'authenticate', 'true') keystone_mock.side_effect = keystone_exc.Unauthorized() - res = self.app.post('/v1/introspection/uuid1', + res = self.app.post('/v1/introspection/%s' % self.uuid, headers={'X-Auth-Token': 'token'}) self.assertEqual(403, res.status_code) self.assertFalse(introspect_mock.called) keystone_mock.assert_called_once_with(token='token') + @mock.patch.object(introspect, 'introspect', autospec=True) + def test_introspect_invalid_uuid(self, introspect_mock): + uuid_dummy = 'uuid1' + res = self.app.post('/v1/introspection/%s' % uuid_dummy) + self.assertEqual(400, res.status_code) + @mock.patch.object(introspect, 'introspect', autospec=True) def test_discover(self, discover_mock): - res = self.app.post('/v1/discover', data='["uuid1"]') + res = self.app.post('/v1/discover', data='["%s"]' % self.uuid) self.assertEqual(202, res.status_code) - discover_mock.assert_called_once_with("uuid1") + discover_mock.assert_called_once_with(self.uuid) + + @mock.patch.object(introspect, 'introspect', autospec=True) + def test_discover_invalid_uuid(self, discover_mock): + uuid_dummy = 'uuid1' + res = self.app.post('/v1/discover', data='["%s"]' % uuid_dummy) + self.assertEqual(400, res.status_code) @mock.patch.object(process, 'process', autospec=True) def test_continue(self, process_mock): @@ -117,20 +132,20 @@ class TestApi(test_base.BaseTest): @mock.patch.object(node_cache, 'get_node', autospec=True) def test_get_introspection_in_progress(self, get_mock): - get_mock.return_value = node_cache.NodeInfo(uuid='uuid', + get_mock.return_value = node_cache.NodeInfo(uuid=self.uuid, started_at=42.0) - res = self.app.get('/v1/introspection/uuid') + res = self.app.get('/v1/introspection/%s' % self.uuid) self.assertEqual(200, res.status_code) self.assertEqual({'finished': False, 'error': None}, json.loads(res.data.decode('utf-8'))) @mock.patch.object(node_cache, 'get_node', autospec=True) def test_get_introspection_finished(self, get_mock): - get_mock.return_value = node_cache.NodeInfo(uuid='uuid', + get_mock.return_value = node_cache.NodeInfo(uuid=self.uuid, started_at=42.0, finished_at=100.1, error='boom') - res = self.app.get('/v1/introspection/uuid') + res = self.app.get('/v1/introspection/%s' % self.uuid) self.assertEqual(200, res.status_code) self.assertEqual({'finished': True, 'error': 'boom'}, json.loads(res.data.decode('utf-8'))) diff --git a/ironic_discoverd/test/test_node_cache.py b/ironic_discoverd/test/test_node_cache.py index 3d85deddb..34fa9ad67 100644 --- a/ironic_discoverd/test/test_node_cache.py +++ b/ironic_discoverd/test/test_node_cache.py @@ -42,7 +42,8 @@ class TestNodeCache(test_base.NodeTest): res = self.db.execute("select uuid, started_at " "from nodes order by uuid").fetchall() - self.assertEqual(['uuid', 'uuid2'], [t[0] for t in res]) + self.assertEqual(['1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e', + 'uuid2'], [t[0] for t in res]) self.assertTrue(time.time() - 60 < res[0][1] < time.time() + 60) res = self.db.execute("select name, value, uuid from attributes " diff --git a/ironic_discoverd/test/test_process.py b/ironic_discoverd/test/test_process.py index 9aae17260..927b8b5a8 100644 --- a/ironic_discoverd/test/test_process.py +++ b/ironic_discoverd/test/test_process.py @@ -410,8 +410,8 @@ class TestProcessNode(BaseTest): self.patch_before) finished_mock.assert_called_once_with( mock.ANY, - error='Timeout waiting for node uuid to power off ' - 'after introspection') + error='Timeout waiting for node %s to power off ' + 'after introspection' % self.uuid) def test_port_failed(self, filters_mock, post_hook_mock): self.ports[0] = exceptions.Conflict() @@ -486,8 +486,8 @@ class TestProcessNode(BaseTest): self.assertFalse(self.cli.node.set_power_state.called) finished_mock.assert_called_once_with( mock.ANY, - error='Failed to validate updated IPMI credentials for node uuid, ' - 'node might require maintenance') + error='Failed to validate updated IPMI credentials for node %s, ' + 'node might require maintenance' % self.uuid) @mock.patch.object(node_cache.NodeInfo, 'finished', autospec=True) def test_power_off_failed(self, finished_mock, filters_mock, @@ -502,5 +502,5 @@ class TestProcessNode(BaseTest): self.patch_before) finished_mock.assert_called_once_with( mock.ANY, - error='Failed to power off node uuid, check it\'s power management' - ' configuration: boom') + error='Failed to power off node %s, check it\'s power management' + ' configuration: boom' % self.uuid) diff --git a/requirements.txt b/requirements.txt index 72aa7531b..e31124595 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,5 +7,6 @@ python-ironicclient>=0.2.1 python-keystoneclient>=1.1.0 requests>=2.2.0,!=2.4.0 oslo.i18n>=1.3.0 # Apache-2.0 +oslo.utils>=1.2.0 # Apache-2.0 six>=1.7.0 stevedore>=1.1.0