diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 1156fa6061..7fe557476b 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -18,6 +18,7 @@ from http import client as http_client import inspect import io import re +import string import jsonpatch import jsonschema @@ -932,6 +933,10 @@ _CONFIG_DRIVE_SCHEMA = { } +# Include newlines and spaces since they're common in base64 values. +_B64_ALPHABET = frozenset(string.ascii_letters + string.digits + '+/=\n\r\t ') + + def check_allow_configdrive(target, configdrive=None): if not configdrive: return @@ -969,6 +974,19 @@ def check_allow_configdrive(target, configdrive=None): 'opr': versions.MINOR_59_CONFIGDRIVE_VENDOR_DATA} raise exception.ClientSideError( msg, status_code=http_client.BAD_REQUEST) + else: + # : is not a valid base64 symbol, so we can use this simple check + if '://' in configdrive: + return + + # This is not 100% robust but it does solve the case of invalid + # JSON assumed to be a base64 string. + letters = set(configdrive) + if letters - _B64_ALPHABET: + msg = _('Invalid configdrive format: it is neither a JSON, nor ' + 'a URL, nor a base64 string') + raise exception.ClientSideError( + msg, status_code=http_client.BAD_REQUEST) def check_allow_filter_by_fault(fault): diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index eb98732393..d3bff3f8b4 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -5007,14 +5007,44 @@ class TestPut(test_api_base.BaseApiTest): deploy_steps=None) def test_provision_with_deploy_configdrive(self): + FAKE_CD = """ +w7FJYV8ywqx+wqnCpwPCoXHDisO6HMO2w4nDsBBJccOvXsKUMsO9OcOPCQLCnMKoPSFLwp +DDhj7Ck8KqwprDpcKWw6XChsOMw5lSEcKUZcO0PUJiWcK4wq0owr4ye8Ozw67ClzXDmsO7 +UxvCpjnCkFQgw73Ch8Kaw5HCicKlXMOvUnDDvg5uwoFkwqDCl8KAEWwCbUQvw7I5JcKUw7 +VbKl3Di8O4LMKuwrHChMOBw5plaVJKci04w7fCgcOgVhkwwoLCgilxwqTCpDNCGzdNw5N6 +wpgAw6jDn8ODLBBlMGcawrEZwr3DiVPDtMKTwpcxwrpBwrrDtcOEw5YTw7MMwqnCsMKqwp +PCkMK1wpTDssKfwrDCscOsEEDDo8OAw5DCqsKKGBRqwqPDqx7Cg8KkDcOkwoIuwo/CgcK0 +ZcKNf3N7wqIYQcKgQDnCq8KFw6DCvMOwWAHChMO3w5xWb8O3wq7Dn8K4eXgWw742woUqw5 +/DvcK+ScKcX8KzwprCuD3DgcOsC8Oqwp0CwqB8TsOIHsKVwozCv8O+w4LCmE9GCMORw63D +icOQw4ZFasOzw4Uvw7NSw6Qbw77DkBgkwo4COcOzOWLClRNQXcOHwojCrsOdHMKIw6nDuM +ORHMKeXMO8fcK0By7CiMKwHSXCoEQgfQhWwpMdSsO8LgHCjh87DQc= """ ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, - {'target': states.ACTIVE, 'configdrive': 'foo'}) + {'target': states.ACTIVE, + 'configdrive': FAKE_CD}) self.assertEqual(http_client.ACCEPTED, ret.status_code) self.assertEqual(b'', ret.body) self.mock_dnd.assert_called_once_with(context=mock.ANY, node_id=self.node.uuid, rebuild=False, - configdrive='foo', + configdrive=FAKE_CD, + topic='test-topic', + deploy_steps=None) + # Check location header + self.assertIsNotNone(ret.location) + expected_location = '/v1/nodes/%s/states' % self.node.uuid + self.assertEqual(urlparse.urlparse(ret.location).path, + expected_location) + + def test_provision_with_deploy_configdrive_url(self): + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.ACTIVE, + 'configdrive': 'http://example.com'}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + self.mock_dnd.assert_called_once_with(context=mock.ANY, + node_id=self.node.uuid, + rebuild=False, + configdrive='http://example.com', topic='test-topic', deploy_steps=None) # Check location header @@ -5055,10 +5085,20 @@ class TestPut(test_api_base.BaseApiTest): topic='test-topic', deploy_steps=None) - def test_provision_with_deploy_configdrive_as_dict_unsupported(self): + def test_provision_with_deploy_configdrive_invalid_type(self): ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, {'target': states.ACTIVE, - 'configdrive': {'user_data': 'foo'}}, + 'configdrive': ["aabb"]}, + headers={api_base.Version.string: '1.60'}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + + def test_provision_with_deploy_configdrive_not_base64(self): + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.ACTIVE, + # Simulate invalid JSON provided to CLI + 'configdrive': '{"meta_data": '}, + headers={api_base.Version.string: '1.60'}, expect_errors=True) self.assertEqual(http_client.BAD_REQUEST, ret.status_code) diff --git a/releasenotes/notes/configdrive-format-1b11f6068bd742cd.yaml b/releasenotes/notes/configdrive-format-1b11f6068bd742cd.yaml new file mode 100644 index 0000000000..5410831d3d --- /dev/null +++ b/releasenotes/notes/configdrive-format-1b11f6068bd742cd.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Rejects ``configdrive`` that is not a JSON, a URL or a base64 string. + Previously invalid JSON supplied to ironicclient could end up accepted + as a configdrive, which would cause a failure much later.