From 751f5dec112898bb4903848d5efc347fe48d8750 Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Thu, 22 Sep 2016 12:24:11 +0800 Subject: [PATCH] Enable reset keypair while rebuilding instance This patch adds `key_name` param to instance rebuild API. Then the user could reset the instance keypair when rebuilding. If set key_name to None, the API will unset the keypair of the instance. APIImpact Implements blueprint: rebuild-keypair-reset Change-Id: I23886a89c25f811cfbe7e2500ce7ff52f9162966 --- api-ref/source/parameters.yaml | 20 +++ api-ref/source/servers-actions.inc | 10 +- .../v2.54/server-action-rebuild-resp.json | 60 ++++++++ .../servers/v2.54/server-action-rebuild.json | 14 ++ .../versions/v21-version-get-resp.json | 2 +- .../versions/versions-get-resp.json | 2 +- nova/api/openstack/api_version_request.py | 3 +- .../compute/rest_api_version_history.rst | 5 + nova/api/openstack/compute/schemas/servers.py | 4 + nova/api/openstack/compute/servers.py | 15 +- nova/api/validation/parameter_types.py | 8 ++ nova/compute/api.py | 19 +++ .../v2.54/server-action-rebuild-resp.json.tpl | 60 ++++++++ .../v2.54/server-action-rebuild.json.tpl | 14 ++ .../api_sample_tests/test_servers.py | 27 ++++ .../api/openstack/compute/test_serversV21.py | 135 ++++++++++++++++++ nova/tests/unit/api/openstack/fakes.py | 9 +- nova/tests/unit/compute/test_compute_api.py | 73 ++++++++++ nova/tests/unit/test_api_validation.py | 37 +++++ ...ebuild-keypair-reset-9ed45744bd85e358.yaml | 9 ++ 20 files changed, 514 insertions(+), 12 deletions(-) create mode 100644 doc/api_samples/servers/v2.54/server-action-rebuild-resp.json create mode 100644 doc/api_samples/servers/v2.54/server-action-rebuild.json create mode 100644 nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild-resp.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild.json.tpl create mode 100644 releasenotes/notes/bp-rebuild-keypair-reset-9ed45744bd85e358.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index ded7a57472e5..48e1c3563924 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -3493,7 +3493,27 @@ key_name: in: body required: false type: string +key_name_rebuild_req: + description: | + Key pair name for rebuild API. + .. note:: Users within the same project are able to rebuild other + user's instances in that project with a new keypair. Keys + are owned by users (which is the only resource that’s true + of). Servers are owned by projects. Because of this a rebuild + with a key_name is looking up the keypair by the user calling + rebuild. + in: body + required: false + type: string + min_version: 2.54 +key_name_rebuild_resp: + description: | + The name of associated key pair, if any. + in: body + required: true + type: string + min_version: 2.54 key_name_resp: description: | The name of associated key pair, if any. diff --git a/api-ref/source/servers-actions.inc b/api-ref/source/servers-actions.inc index 6254ad8d4080..31ce02293e7b 100644 --- a/api-ref/source/servers-actions.inc +++ b/api-ref/source/servers-actions.inc @@ -487,10 +487,11 @@ Request - personality.contents: contents - preserve_ephemeral: preserve_ephemeral - description: server_description + - key_name: key_name_rebuild_req -**Example Rebuild Server (rebuild Action) (v2.47)** +**Example Rebuild Server (rebuild Action) (v2.54)** -.. literalinclude:: ../../doc/api_samples/servers/v2.47/server-action-rebuild.json +.. literalinclude:: ../../doc/api_samples/servers/v2.54/server-action-rebuild.json :language: javascript Response @@ -534,10 +535,11 @@ Response - locked: locked - description: server_description_resp - tags: tags + - key_name: key_name_rebuild_resp -**Example Rebuild Server (rebuild Action) (v2.47)** +**Example Rebuild Server (rebuild Action) (v2.54)** -.. literalinclude:: ../../doc/api_samples/servers/v2.47/server-action-rebuild-resp.json +.. literalinclude:: ../../doc/api_samples/servers/v2.54/server-action-rebuild-resp.json :language: javascript Remove (Disassociate) Floating Ip (removeFloatingIp Action) (DEPRECATED) diff --git a/doc/api_samples/servers/v2.54/server-action-rebuild-resp.json b/doc/api_samples/servers/v2.54/server-action-rebuild-resp.json new file mode 100644 index 000000000000..612bd6019943 --- /dev/null +++ b/doc/api_samples/servers/v2.54/server-action-rebuild-resp.json @@ -0,0 +1,60 @@ +{ + "server": { + "accessIPv4": "1.2.3.4", + "accessIPv6": "80fe::", + "addresses": { + "private": [ + { + "addr": "192.168.0.3", + "version": 4 + } + ] + }, + "adminPass": "seekr3t", + "created": "2013-11-14T06:29:00Z", + "flavor": { + "disk": 1, + "ephemeral": 0, + "extra_specs": {}, + "original_name": "m1.tiny", + "ram": 512, + "swap": 0, + "vcpus": 1 + }, + "hostId": "28d8d56f0e3a77e20891f455721cbb68032e017045e20aa5dfc6cb66", + "id": "a0a80a94-3d81-4a10-822a-daa0cf9e870b", + "image": { + "id": "70a599e0-31e7-49b7-b260-868f441e862b", + "links": [ + { + "href": "http://openstack.example.com/6f70656e737461636b20342065766572/images/70a599e0-31e7-49b7-b260-868f441e862b", + "rel": "bookmark" + } + ] + }, + "links": [ + { + "href": "http://openstack.example.com/v2/6f70656e737461636b20342065766572/servers/a0a80a94-3d81-4a10-822a-daa0cf9e870b", + "rel": "self" + }, + { + "href": "http://openstack.example.com/6f70656e737461636b20342065766572/servers/a0a80a94-3d81-4a10-822a-daa0cf9e870b", + "rel": "bookmark" + } + ], + "locked": false, + "metadata": { + "meta_var": "meta_val" + }, + "name": "foobar", + "key_name": "new-key", + "description" : "description of foobar", + "progress": 0, + "status": "ACTIVE", + "OS-DCF:diskConfig": "AUTO", + "tenant_id": "6f70656e737461636b20342065766572", + "updated": "2013-11-14T06:29:02Z", + "user_id": "fake", + "tags": [] + } +} diff --git a/doc/api_samples/servers/v2.54/server-action-rebuild.json b/doc/api_samples/servers/v2.54/server-action-rebuild.json new file mode 100644 index 000000000000..d133952c0ee3 --- /dev/null +++ b/doc/api_samples/servers/v2.54/server-action-rebuild.json @@ -0,0 +1,14 @@ +{ + "rebuild" : { + "accessIPv4" : "1.2.3.4", + "accessIPv6" : "80fe::", + "imageRef" : "70a599e0-31e7-49b7-b260-868f441e862b", + "name" : "foobar", + "key_name": "new-key", + "description" : "description of foobar", + "adminPass" : "seekr3t", + "metadata" : { + "meta_var" : "meta_val" + } + } +} diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index 1fc439ffaedb..92d5a3e2eff7 100644 --- a/doc/api_samples/versions/v21-version-get-resp.json +++ b/doc/api_samples/versions/v21-version-get-resp.json @@ -19,7 +19,7 @@ } ], "status": "CURRENT", - "version": "2.53", + "version": "2.54", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json index f9965b4167d1..43956e0fb4b6 100644 --- a/doc/api_samples/versions/versions-get-resp.json +++ b/doc/api_samples/versions/versions-get-resp.json @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.53", + "version": "2.54", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index 0be8c905b01d..7618750e80e8 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -128,6 +128,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: The os-services and os-hypervisors APIs now return a uuid in the id field, and takes a uuid in requests. PUT and GET requests and responses are also changed. + * 2.54 - Enable reset key pair while rebuilding instance. """ # The minimum and maximum versions of the API supported @@ -136,7 +137,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: # Note(cyeoh): This only applies for the v2.1 API once microversions # support is fully merged. It does not affect the V2 API. _MIN_API_VERSION = "2.1" -_MAX_API_VERSION = "2.53" +_MAX_API_VERSION = "2.54" DEFAULT_API_VERSION = _MIN_API_VERSION # Almost all proxy APIs which related to network, images and baremetal diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst index ffbfd8295870..8990fd7e0275 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -683,3 +683,8 @@ uniqueness across cells. This microversion brings the following changes: * ``GET /os-hypervisors/detail`` * ``GET /os-hypervisors/{hypervisor_id}`` * ``GET /os-hypervisors/{hypervisor_id}/uptime`` + +2.54 +---- + + Allow the user to set the server key pair while rebuilding. diff --git a/nova/api/openstack/compute/schemas/servers.py b/nova/api/openstack/compute/schemas/servers.py index 35b3e421196d..151578429c21 100644 --- a/nova/api/openstack/compute/schemas/servers.py +++ b/nova/api/openstack/compute/schemas/servers.py @@ -199,6 +199,10 @@ base_rebuild_v219 = copy.deepcopy(base_rebuild) base_rebuild_v219['properties']['rebuild'][ 'properties']['description'] = parameter_types.description +base_rebuild_v254 = copy.deepcopy(base_rebuild_v219) +base_rebuild_v254['properties']['rebuild'][ + 'properties']['key_name'] = parameter_types.name_or_none + resize = { 'type': 'object', 'properties': { diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index a806b23b3031..9c9f87c6a9eb 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -79,6 +79,7 @@ class ServersController(wsgi.Controller): schema_server_create_v219 = schema_servers.base_create_v219 schema_server_update_v219 = schema_servers.base_update_v219 schema_server_rebuild_v219 = schema_servers.base_rebuild_v219 + schema_server_rebuild_v254 = schema_servers.base_rebuild_v254 schema_server_create_v232 = schema_servers.base_create_v232 schema_server_create_v237 = schema_servers.base_create_v237 @@ -876,7 +877,8 @@ class ServersController(wsgi.Controller): @wsgi.action('rebuild') @validation.schema(schema_server_rebuild_v20, '2.0', '2.0') @validation.schema(schema_server_rebuild, '2.1', '2.18') - @validation.schema(schema_server_rebuild_v219, '2.19') + @validation.schema(schema_server_rebuild_v219, '2.19', '2.53') + @validation.schema(schema_server_rebuild_v254, '2.54') def _action_rebuild(self, req, id, body): """Rebuild an instance with the given attributes.""" rebuild_dict = body['rebuild'] @@ -900,6 +902,10 @@ class ServersController(wsgi.Controller): helpers.translate_attributes(helpers.REBUILD, rebuild_dict, kwargs) + if (api_version_request.is_supported(req, min_version='2.54') + and 'key_name' in rebuild_dict): + kwargs['key_name'] = rebuild_dict.get('key_name') + for request_attribute, instance_attribute in attr_map.items(): try: if request_attribute == 'name': @@ -930,6 +936,9 @@ class ServersController(wsgi.Controller): except exception.ImageNotFound: msg = _("Cannot find image for rebuild") raise exc.HTTPBadRequest(explanation=msg) + except exception.KeypairNotFound: + msg = _("Invalid key_name provided.") + raise exc.HTTPBadRequest(explanation=msg) except exception.QuotaError as error: raise exc.HTTPForbidden(explanation=error.format_message()) except (exception.ImageNotActive, @@ -948,6 +957,10 @@ class ServersController(wsgi.Controller): if CONF.api.enable_instance_password: view['server']['adminPass'] = password + if api_version_request.is_supported(req, min_version='2.54'): + # NOTE(liuyulong): set the new key_name for the API response. + view['server']['key_name'] = instance.key_name + robj = wsgi.ResponseObject(view) return self._add_location(robj) diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index 462ea88af117..f36b18a667ee 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -226,6 +226,14 @@ none = { } +name_or_none = { + 'oneOf': [ + {'type': 'string', 'minLength': 1, 'maxLength': 255}, + {'type': 'null'}, + ] +} + + positive_integer = { 'type': ['integer', 'string'], 'pattern': '^[0-9]*$', 'minimum': 1, 'minLength': 1 diff --git a/nova/compute/api.py b/nova/compute/api.py index 6f9033de1833..b96fb15f825c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2866,6 +2866,25 @@ class API(base.Base): preserve_ephemeral = kwargs.get('preserve_ephemeral', False) auto_disk_config = kwargs.get('auto_disk_config') + if 'key_name' in kwargs: + key_name = kwargs.pop('key_name') + if key_name: + # NOTE(liuyulong): we are intentionally using the user_id from + # the request context rather than the instance.user_id because + # users own keys but instances are owned by projects, and + # another user in the same project can rebuild an instance + # even if they didn't create it. + key_pair = objects.KeyPair.get_by_name(context, + context.user_id, + key_name) + instance.key_name = key_pair.name + instance.key_data = key_pair.public_key + instance.keypairs = objects.KeyPairList(objects=[key_pair]) + else: + instance.key_name = None + instance.key_data = None + instance.keypairs = objects.KeyPairList(objects=[]) + image_id, image = self._get_image(context, image_href) self._check_auto_disk_config(image=image, **kwargs) diff --git a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild-resp.json.tpl new file mode 100644 index 000000000000..e77bbd952b3d --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild-resp.json.tpl @@ -0,0 +1,60 @@ +{ + "server": { + "accessIPv4": "%(access_ip_v4)s", + "accessIPv6": "%(access_ip_v6)s", + "addresses": { + "private": [ + { + "addr": "%(ip)s", + "version": 4 + } + ] + }, + "adminPass": "%(password)s", + "created": "%(isotime)s", + "flavor": { + "disk": 1, + "ephemeral": 0, + "extra_specs": {}, + "original_name": "m1.tiny", + "ram": 512, + "swap": 0, + "vcpus": 1 + }, + "hostId": "%(hostid)s", + "id": "%(uuid)s", + "image": { + "id": "%(uuid)s", + "links": [ + { + "href": "%(compute_endpoint)s/images/%(uuid)s", + "rel": "bookmark" + } + ] + }, + "links": [ + { + "href": "%(versioned_compute_endpoint)s/servers/%(uuid)s", + "rel": "self" + }, + { + "href": "%(compute_endpoint)s/servers/%(uuid)s", + "rel": "bookmark" + } + ], + "locked": false, + "metadata": { + "meta_var": "meta_val" + }, + "name": "%(name)s", + "key_name": "%(key_name)s", + "description": "%(description)s", + "progress": 0, + "OS-DCF:diskConfig": "AUTO", + "status": "ACTIVE", + "tenant_id": "6f70656e737461636b20342065766572", + "updated": "%(isotime)s", + "user_id": "fake", + "tags": [] + } +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild.json.tpl new file mode 100644 index 000000000000..4764202366bf --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.54/server-action-rebuild.json.tpl @@ -0,0 +1,14 @@ +{ + "rebuild" : { + "accessIPv4" : "%(access_ip_v4)s", + "accessIPv6" : "%(access_ip_v6)s", + "imageRef" : "%(uuid)s", + "name" : "%(name)s", + "key_name" : "%(key_name)s", + "description" : "%(description)s", + "adminPass" : "%(pass)s", + "metadata" : { + "meta_var" : "meta_val" + } + } +} diff --git a/nova/tests/functional/api_sample_tests/test_servers.py b/nova/tests/functional/api_sample_tests/test_servers.py index c859d665cf87..a384f14695d6 100644 --- a/nova/tests/functional/api_sample_tests/test_servers.py +++ b/nova/tests/functional/api_sample_tests/test_servers.py @@ -458,6 +458,33 @@ class ServersActionsJson226Test(ServersSampleBase): self._verify_response('server-action-rebuild-resp', subs, resp, 202) +class ServersActionsJson254Test(ServersSampleBase): + microversion = '2.54' + sample_dir = 'servers' + scenarios = [('v2_54', {'api_major_version': 'v2.1'})] + + def test_server_rebuild(self): + fakes.stub_out_key_pair_funcs(self) + uuid = self._post_server() + image = fake.get_valid_image_id() + params = { + 'uuid': image, + 'name': 'foobar', + 'key_name': 'new-key', + 'description': 'description of foobar', + 'pass': 'seekr3t', + 'hostid': '[a-f0-9]+', + 'access_ip_v4': '1.2.3.4', + 'access_ip_v6': '80fe::', + } + + resp = self._do_post('servers/%s/action' % uuid, + 'server-action-rebuild', params) + subs = params.copy() + del subs['uuid'] + self._verify_response('server-action-rebuild-resp', subs, resp, 202) + + class ServersCreateImageJsonTest(ServersSampleBase, _ServersActionsJsonTestMixin): """Tests the createImage server action API against 2.1.""" diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 15bee95610f7..3e4d0504f664 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -2160,6 +2160,141 @@ class ServersControllerRebuildInstanceTest(ControllerTest): self.controller._stop_server, req, 'test_inst', body) +class ServersControllerRebuildTestV254(ServersControllerRebuildInstanceTest): + + def setUp(self): + super(ServersControllerRebuildTestV254, self).setUp() + fakes.stub_out_key_pair_funcs(self) + self.req.api_version_request = \ + api_version_request.APIVersionRequest('2.54') + + def _test_set_key_name_rebuild(self, set_key_name=True): + key_name = "key" + fake_get = fakes.fake_compute_get(vm_state=vm_states.ACTIVE, + key_name=key_name, + project_id=self.req_project_id, + user_id=self.req_user_id) + with mock.patch.object(compute_api.API, 'get', + side_effect=fake_get): + if set_key_name: + self.body['rebuild']['key_name'] = key_name + self.req.body = jsonutils.dump_as_bytes(self.body) + server = self.controller._action_rebuild( + self.req, FAKE_UUID, + body=self.body).obj['server'] + self.assertEqual(server['id'], FAKE_UUID) + self.assertEqual(server['key_name'], key_name) + + def test_rebuild_accepted_with_keypair_name(self): + self._test_set_key_name_rebuild() + + def test_rebuild_key_not_changed(self): + self._test_set_key_name_rebuild(set_key_name=False) + + def test_rebuild_invalid_microversion_253(self): + self.req.api_version_request = \ + api_version_request.APIVersionRequest('2.53') + body = { + "rebuild": { + "imageRef": self.image_uuid, + "key_name": "key" + }, + } + excpt = self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) + self.assertIn('key_name', six.text_type(excpt)) + + def test_rebuild_with_not_existed_keypair_name(self): + body = { + "rebuild": { + "imageRef": self.image_uuid, + "key_name": "nonexistentkey" + }, + } + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) + + def test_rebuild_user_has_no_key_pair(self): + def no_key_pair(context, user_id, name): + raise exception.KeypairNotFound(user_id=user_id, name=name) + self.stub_out('nova.db.key_pair_get', no_key_pair) + fake_get = fakes.fake_compute_get(vm_state=vm_states.ACTIVE, + key_name=None, + project_id=self.req_project_id, + user_id=self.req_user_id) + with mock.patch.object(compute_api.API, 'get', + side_effect=fake_get): + self.body['rebuild']['key_name'] = "a-key-name" + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=self.body) + + def test_rebuild_with_non_string_keypair_name(self): + body = { + "rebuild": { + "imageRef": self.image_uuid, + "key_name": 12345 + }, + } + self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) + + def test_rebuild_with_invalid_keypair_name(self): + body = { + "rebuild": { + "imageRef": self.image_uuid, + "key_name": "123\0d456" + }, + } + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) + + def test_rebuild_with_empty_keypair_name(self): + body = { + "rebuild": { + "imageRef": self.image_uuid, + "key_name": '' + }, + } + self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) + + def test_rebuild_with_none_keypair_name(self): + key_name = None + fake_get = fakes.fake_compute_get(vm_state=vm_states.ACTIVE, + key_name=key_name, + project_id=self.req_project_id, + user_id=self.req_user_id) + with mock.patch.object(compute_api.API, 'get', + side_effect=fake_get): + with mock.patch.object(objects.KeyPair, 'get_by_name') as key_get: + self.body['rebuild']['key_name'] = key_name + self.req.body = jsonutils.dump_as_bytes(self.body) + self.controller._action_rebuild( + self.req, FAKE_UUID, + body=self.body) + # NOTE: because the api will call _get_server twice. The server + # response will always be the same one. So we just use + # objects.KeyPair.get_by_name to verify test. + key_get.assert_not_called() + + def test_rebuild_with_too_large_keypair_name(self): + body = { + "rebuild": { + "imageRef": self.image_uuid, + "key_name": 256 * "k" + }, + } + self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) + + class ServersControllerRebuildTestV219(ServersControllerRebuildInstanceTest): def setUp(self): diff --git a/nova/tests/unit/api/openstack/fakes.py b/nova/tests/unit/api/openstack/fakes.py index 8167b34e2a32..200d1335e9aa 100644 --- a/nova/tests/unit/api/openstack/fakes.py +++ b/nova/tests/unit/api/openstack/fakes.py @@ -90,9 +90,9 @@ def stub_out_key_pair_funcs(testcase, have_key_pair=True, **kwargs): name='key', public_key='public_key', **kwargs)] def one_key_pair(context, user_id, name): - if name == 'key': + if name in ['key', 'new-key']: return dict(test_keypair.fake_keypair, - name='key', public_key='public_key', **kwargs) + name=name, public_key='public_key', **kwargs) else: raise exc.KeypairNotFound(user_id=user_id, name=name) @@ -541,7 +541,8 @@ def stub_instance(id=1, user_id=None, project_id=None, host=None, "flavor": flavorinfo, }, "cleaned": cleaned, - "services": services} + "services": services, + "tags": []} instance.update(info_cache) instance['info_cache']['instance_uuid'] = instance['uuid'] @@ -552,7 +553,7 @@ def stub_instance(id=1, user_id=None, project_id=None, host=None, def stub_instance_obj(ctxt, *args, **kwargs): db_inst = stub_instance(*args, **kwargs) expected = ['metadata', 'system_metadata', 'flavor', - 'info_cache', 'security_groups'] + 'info_cache', 'security_groups', 'tags'] inst = objects.Instance._from_db_object(ctxt, objects.Instance(), db_inst, expected_attrs=expected) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index e8a66186e341..660fa64c046c 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -160,6 +160,17 @@ class _ComputeAPIUnitTestMixIn(object): instance.obj_reset_changes() return instance + def _create_keypair_obj(self, instance): + """Create a test keypair.""" + keypair = objects.KeyPair() + keypair.id = 1 + keypair.name = 'fake_key' + keypair.user_id = instance.user_id + keypair.fingerprint = 'fake' + keypair.public_key = 'fake key' + keypair.type = 'ssh' + return keypair + def _obj_to_list_obj(self, list_obj, obj): list_obj.objects = [] list_obj.objects.append(obj) @@ -3238,6 +3249,68 @@ class _ComputeAPIUnitTestMixIn(object): None, new_image, flavor, {}, [], None) self.assertEqual(fields_obj.VMMode.XEN, instance.vm_mode) + @mock.patch.object(objects.KeyPair, 'get_by_name') + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') + @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(objects.Instance, 'get_flavor') + @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') + @mock.patch.object(compute_api.API, '_get_image') + @mock.patch.object(compute_api.API, '_check_auto_disk_config') + @mock.patch.object(compute_api.API, '_checks_for_create_and_rebuild') + @mock.patch.object(compute_api.API, '_record_action_start') + def test_rebuild_change_keypair(self, _record_action_start, + _checks_for_create_and_rebuild, _check_auto_disk_config, + _get_image, bdm_get_by_instance_uuid, get_flavor, instance_save, + req_spec_get_by_inst_uuid, mock_get_keypair): + orig_system_metadata = {} + orig_key_name = 'orig_key_name' + orig_key_data = 'orig_key_data_XXX' + instance = fake_instance.fake_instance_obj(self.context, + vm_state=vm_states.ACTIVE, cell_name='fake-cell', + launched_at=timeutils.utcnow(), + system_metadata=orig_system_metadata, + image_ref='foo', + expected_attrs=['system_metadata'], + key_name=orig_key_name, + key_data=orig_key_data) + get_flavor.return_value = test_flavor.fake_flavor + flavor = instance.get_flavor() + image_href = 'foo' + image = { + "min_ram": 10, "min_disk": 1, + "properties": {'architecture': fields_obj.Architecture.X86_64, + 'vm_mode': 'hvm'}} + admin_pass = '' + files_to_inject = [] + bdms = objects.BlockDeviceMappingList() + + _get_image.return_value = (None, image) + bdm_get_by_instance_uuid.return_value = bdms + + fake_spec = objects.RequestSpec() + req_spec_get_by_inst_uuid.return_value = fake_spec + + keypair = self._create_keypair_obj(instance) + mock_get_keypair.return_value = keypair + with mock.patch.object(self.compute_api.compute_task_api, + 'rebuild_instance') as rebuild_instance: + self.compute_api.rebuild(self.context, instance, image_href, + admin_pass, files_to_inject, key_name=keypair.name) + + rebuild_instance.assert_called_once_with(self.context, + instance=instance, new_pass=admin_pass, + injected_files=files_to_inject, image_ref=image_href, + orig_image_ref=image_href, + orig_sys_metadata=orig_system_metadata, bdms=bdms, + preserve_ephemeral=False, host=instance.host, + request_spec=fake_spec, kwargs={}) + + _check_auto_disk_config.assert_called_once_with(image=image) + _checks_for_create_and_rebuild.assert_called_once_with(self.context, + None, image, flavor, {}, [], None) + self.assertNotEqual(orig_key_name, instance.key_name) + self.assertNotEqual(orig_key_data, instance.key_data) + def _test_check_injected_file_quota_onset_file_limit_exceeded(self, side_effect): injected_files = [ diff --git a/nova/tests/unit/test_api_validation.py b/nova/tests/unit/test_api_validation.py index afef4951261d..416ef62dcbad 100644 --- a/nova/tests/unit/test_api_validation.py +++ b/nova/tests/unit/test_api_validation.py @@ -988,6 +988,43 @@ class NoneTypeTestCase(APIValidationTestCase): expected_detail=detail) +class NameOrNoneTestCase(APIValidationTestCase): + + post_schema = { + 'type': 'object', + 'properties': { + 'foo': parameter_types.name_or_none + } + } + + def test_valid(self): + self.assertEqual('Validation succeeded.', + self.post(body={'foo': None}, + req=FakeRequest())) + self.assertEqual('Validation succeeded.', + self.post(body={'foo': '1'}, + req=FakeRequest())) + + def test_validate_fails(self): + detail = ("Invalid input for field/attribute foo. Value: 1234. 1234 " + "is not valid under any of the given schemas") + self.check_validation_error(self.post, body={'foo': 1234}, + expected_detail=detail) + + detail = ("Invalid input for field/attribute foo. Value: . '' " + "is not valid under any of the given schemas") + self.check_validation_error(self.post, body={'foo': ''}, + expected_detail=detail) + + too_long_name = 256 * "k" + detail = ("Invalid input for field/attribute foo. Value: %s. " + "'%s' is not valid under any of the " + "given schemas") % (too_long_name, too_long_name) + self.check_validation_error(self.post, + body={'foo': too_long_name}, + expected_detail=detail) + + class TcpUdpPortTestCase(APIValidationTestCase): post_schema = { diff --git a/releasenotes/notes/bp-rebuild-keypair-reset-9ed45744bd85e358.yaml b/releasenotes/notes/bp-rebuild-keypair-reset-9ed45744bd85e358.yaml new file mode 100644 index 000000000000..cadd81c980a7 --- /dev/null +++ b/releasenotes/notes/bp-rebuild-keypair-reset-9ed45744bd85e358.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + A new param ``key_name`` was added to the instance rebuild API (v2.54), + then it is able to reset instance key pair. It is worth noting that + users within the same project are able to rebuild other user's instances + in that project with a new keypair. + If set ``key_name`` to None in API body, nova will unset the keypair of + instance during rebuild.