diff --git a/nova/api/openstack/compute/schemas/servers.py b/nova/api/openstack/compute/schemas/servers.py index 1db28bb1725f..1275230d5b92 100644 --- a/nova/api/openstack/compute/schemas/servers.py +++ b/nova/api/openstack/compute/schemas/servers.py @@ -1439,6 +1439,330 @@ create_response = { ], } +update_response = { + 'type': 'object', + 'properties': { + 'server': { + 'type': 'object', + 'properties': { + 'accessIPv4': { + 'type': 'string', + 'oneOf': [{'format': 'ipv4'}, {'const': ''}], + }, + 'accessIPv6': { + 'type': 'string', + 'oneOf': [{'format': 'ipv6'}, {'const': ''}], + }, + 'addresses': { + 'type': 'object', + 'patternProperties': { + '^.+$': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'addr': { + 'type': 'string', + 'oneOf': [ + {'format': 'ipv4'}, + {'format': 'ipv6'}, + ], + }, + 'version': { + 'type': 'number', + 'enum': [4, 6], + }, + }, + 'required': ['addr', 'version'], + 'additionalProperties': False, + }, + }, + }, + 'additionalProperties': False, + }, + 'adminPass': {'type': ['null', 'string']}, + 'created': {'type': 'string', 'format': 'date-time'}, + 'fault': { + 'type': 'object', + 'properties': { + 'code': {'type': 'integer'}, + 'created': {'type': 'string', 'format': 'date-time'}, + 'details': {'type': 'string'}, + 'message': {'type': 'string'}, + }, + 'required': ['code', 'created', 'message'], + 'additionalProperties': False, + }, + 'flavor': { + 'type': 'object', + 'properties': { + 'id': {'type': 'string'}, + 'links': response_types.links, + }, + 'additionalProperties': False, + }, + 'hostId': {'type': 'string'}, + 'id': {'type': 'string'}, + 'image': { + 'oneOf': [ + {'type': 'string', 'const': ''}, + { + 'type': 'object', + 'properties': { + 'id': {'type': 'string', 'format': 'uuid'}, + 'links': response_types.links, + }, + 'additionalProperties': False, + }, + ], + }, + 'links': response_types.links, + 'metadata': { + 'type': 'object', + 'patternProperties': { + '^.+$': { + 'type': 'string' + }, + }, + 'additionalProperties': False, + }, + 'name': {'type': ['string', 'null']}, + 'progress': {'type': ['null', 'number']}, + 'status': _server_status, + 'tenant_id': parameter_types.project_id, + 'updated': {'type': 'string', 'format': 'date-time'}, + 'user_id': parameter_types.user_id, + 'OS-DCF:diskConfig': {'type': 'string'}, + }, + 'required': [ + # fault, progress depend on server state + 'accessIPv4', + 'accessIPv6', + 'addresses', + 'created', + 'flavor', + 'hostId', + 'id', + 'image', + 'links', + 'metadata', + 'name', + 'status', + 'tenant_id', + 'updated', + 'user_id', + 'OS-DCF:diskConfig', + ], + 'additionalProperties': False, + }, + }, + 'required': [ + 'server' + ], + 'additionalProperties': False, +} + +update_response_v29 = copy.deepcopy(update_response) +update_response_v29['properties']['server']['properties']['locked'] = { + 'type': 'boolean', +} +update_response_v29['properties']['server']['required'].append('locked') + +update_response_v219 = copy.deepcopy(update_response_v29) +update_response_v219['properties']['server']['properties']['description'] = { + 'type': ['null', 'string'], +} +update_response_v219['properties']['server']['required'].append('description') + +update_response_v226 = copy.deepcopy(update_response_v219) +update_response_v226['properties']['server']['properties']['tags'] = { + 'type': 'array', + 'items': {'type': 'string'}, + 'maxItems': 50, +} +update_response_v226['properties']['server']['required'].append('tags') + +# NOTE(stephenfin): We overwrite rather than extend 'flavor', since we now +# embed the flavor in this version +update_response_v247 = copy.deepcopy(update_response_v226) +update_response_v247['properties']['server']['properties']['flavor'] = { + 'type': 'object', + 'properties': { + 'disk': {'type': 'integer'}, + 'ephemeral': {'type': 'integer'}, + 'extra_specs': { + 'type': 'object', + 'patternProperties': { + '^.+$': {'type': 'string'}, + }, + 'additionalProperties': False, + }, + 'original_name': {'type': 'string'}, + 'ram': {'type': 'integer'}, + 'swap': {'type': 'integer'}, + 'vcpus': {'type': 'integer'}, + }, + 'required': ['disk', 'ephemeral', 'original_name', 'ram', 'swap', 'vcpus'], + 'additionalProperties': False, +} + +update_response_v263 = copy.deepcopy(update_response_v247) +update_response_v263['properties']['server']['properties'].update( + { + 'trusted_image_certificates': { + 'type': ['array', 'null'], + 'items': {'type': 'string'}, + }, + }, +) +update_response_v263['properties']['server']['required'].append( + 'trusted_image_certificates' +) + +update_response_v271 = copy.deepcopy(update_response_v263) +update_response_v271['properties']['server']['properties'].update( + { + 'server_groups': { + 'type': 'array', + 'items': {'type': 'string', 'format': 'uuid'}, + 'maxLength': 1, + }, + }, +) +update_response_v271['properties']['server']['required'].append( + 'server_groups' +) + +update_response_v273 = copy.deepcopy(update_response_v271) +update_response_v273['properties']['server']['properties'].update( + { + 'locked_reason': {'type': ['null', 'string']}, + }, +) +update_response_v273['properties']['server']['required'].append( + 'locked_reason' +) + +update_response_v275 = copy.deepcopy(update_response_v273) +update_response_v275['properties']['server']['properties'].update( + { + 'config_drive': { + # TODO(stephenfin): Our tests return null but this shouldn't happen + # in practice, apparently? + 'type': ['string', 'boolean', 'null'], + }, + 'host_status': {'type': 'string'}, + 'key_name': {'type': ['null', 'string']}, + 'os-extended-volumes:volumes_attached': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'id': {'type': 'string'}, + 'delete_on_termination': { + 'type': 'boolean', + 'default': False, + }, + }, + 'required': ['id', 'delete_on_termination'], + 'additionalProperties': False, + }, + }, + 'security_groups': { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'name': {'type': 'string'}, + }, + 'required': ['name'], + 'additionalProperties': False, + }, + }, + 'OS-EXT-AZ:availability_zone': {'type': 'string'}, + 'OS-EXT-SRV-ATTR:host': {'type': ['string', 'null']}, + 'OS-EXT-SRV-ATTR:hostname': {'type': 'string'}, + 'OS-EXT-SRV-ATTR:hypervisor_hostname': {'type': ['string', 'null']}, + 'OS-EXT-SRV-ATTR:instance_name': {'type': 'string'}, + 'OS-EXT-SRV-ATTR:kernel_id': {'type': ['string', 'null']}, + 'OS-EXT-SRV-ATTR:launch_index': {'type': 'integer'}, + 'OS-EXT-SRV-ATTR:ramdisk_id': {'type': ['string', 'null']}, + 'OS-EXT-SRV-ATTR:reservation_id': {'type': ['string', 'null']}, + 'OS-EXT-SRV-ATTR:root_device_name': {'type': ['string', 'null']}, + 'OS-EXT-SRV-ATTR:user_data': { + 'type': ['string', 'null'], 'format': 'base64', 'maxLength': 65535, + }, + 'OS-EXT-STS:power_state': { + 'type': ['integer', 'null'], 'enum': [0, 1, 3, 4, 6, 7, None], + }, + 'OS-EXT-STS:task_state': {'type': ['string', 'null']}, + 'OS-EXT-STS:vm_state': {'type': ['string', 'null']}, + 'OS-SRV-USG:launched_at': { + 'type': ['string', 'null'], 'format': 'date-time', + }, + 'OS-SRV-USG:terminated_at': { + 'type': ['string', 'null'], 'format': 'date-time', + }, + }, +) + +update_response_v275['properties']['server']['required'].extend([ + 'config_drive', + 'OS-EXT-AZ:availability_zone', + 'OS-EXT-STS:power_state', + 'OS-EXT-STS:task_state', + 'OS-EXT-STS:vm_state', + 'os-extended-volumes:volumes_attached', + 'OS-SRV-USG:launched_at', + 'OS-SRV-USG:terminated_at', +]) + +update_response_v275['properties']['server']['properties']['addresses'][ + 'patternProperties' +]['^.+$']['items']['properties'].update({ + 'OS-EXT-IPS-MAC:mac_addr': {'type': 'string', 'format': 'mac-address'}, + 'OS-EXT-IPS:type': {'type': 'string', 'enum': ['fixed', 'floating']}, +}) +update_response_v275['properties']['server']['properties']['addresses'][ + 'patternProperties' +]['^.+$']['items']['required'].extend([ + 'OS-EXT-IPS-MAC:mac_addr', 'OS-EXT-IPS:type' +]) + +update_response_v296 = copy.deepcopy(update_response_v275) +update_response_v296['properties']['server']['properties'].update({ + 'pinned_availability_zone': { + 'type': ['null', 'string'], + }, +}) +update_response_v296['properties']['server']['required'].append( + 'pinned_availability_zone' +) + +update_response_v298 = copy.deepcopy(update_response_v296) +update_response_v298['properties']['server']['properties']['image']['oneOf'][ + 1 +]['properties'].update({ + 'properties': { + 'type': 'object', + 'patternProperties': { + '^[a-zA-Z0-9_:. ]{1,255}$': { + 'type': ['string', 'null'], + 'maxLength': 255, + }, + }, + 'additionalProperties': False, + }, +}) + +update_response_v2100 = copy.deepcopy(update_response_v298) +update_response_v2100['properties']['server']['properties'].update({ + 'scheduler_hints': _hints, +}) +update_response_v2100['properties']['server']['required'].append( + 'scheduler_hints' +) + resize_response = {'type': 'null'} confirm_resize_response = {'type': 'null'} @@ -1765,6 +2089,7 @@ rebuild_response_v296['properties']['server']['properties'].update({ rebuild_response_v296['properties']['server']['required'].append( 'pinned_availability_zone' ) + rebuild_response_v298 = copy.deepcopy(rebuild_response_v296) rebuild_response_v298['properties']['server']['properties']['image']['oneOf'][ 1 diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 5c69c9565dc1..fc628c48e28e 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -938,6 +938,18 @@ class ServersController(wsgi.Controller): @validation.schema(schema.update_v219, '2.19', '2.89') @validation.schema(schema.update_v290, '2.90', '2.93') @validation.schema(schema.update_v294, '2.94') + @validation.response_body_schema(schema.update_response, '2.0', '2.8') + @validation.response_body_schema(schema.update_response_v29, '2.9', '2.18') + @validation.response_body_schema(schema.update_response_v219, '2.19', '2.25') # noqa: E501 + @validation.response_body_schema(schema.update_response_v226, '2.26', '2.46') # noqa: E501 + @validation.response_body_schema(schema.update_response_v247, '2.47', '2.62') # noqa: E501 + @validation.response_body_schema(schema.update_response_v263, '2.63', '2.70') # noqa: E501 + @validation.response_body_schema(schema.update_response_v271, '2.71', '2.72') # noqa: E501 + @validation.response_body_schema(schema.update_response_v273, '2.73', '2.74') # noqa: E501 + @validation.response_body_schema(schema.update_response_v275, '2.75', '2.95') # noqa: E501 + @validation.response_body_schema(schema.update_response_v296, '2.96', '2.97') # noqa: E501 + @validation.response_body_schema(schema.update_response_v298, '2.98', '2.99') # noqa: E501 + @validation.response_body_schema(schema.update_response_v2100, '2.100') def update(self, req, id, body): """Update server then pass on to version-specific controller.""" @@ -966,42 +978,42 @@ class ServersController(wsgi.Controller): try: instance = self.compute_api.update_instance( ctxt, instance, update_dict) - - show_server_groups = api_version_request.is_supported(req, '2.71') - # NOTE(gmann): Starting from microversion 2.75, PUT and Rebuild - # API response will show all attributes like GET /servers API. - show_all_attributes = api_version_request.is_supported(req, '2.75') - extend_address = show_all_attributes - show_AZ = show_all_attributes - show_config_drive = show_all_attributes - show_keypair = show_all_attributes - show_srv_usg = show_all_attributes - show_sec_grp = show_all_attributes - show_extended_status = show_all_attributes - show_extended_volumes = show_all_attributes - # NOTE(gmann): Below attributes need to be added in response - # if respective policy allows.So setting these as None - # to perform the policy check in view builder. - show_extended_attr = None if show_all_attributes else False - show_host_status = None if show_all_attributes else False - - return self._view_builder.show( - req, instance, - extend_address=extend_address, - show_AZ=show_AZ, - show_config_drive=show_config_drive, - show_extended_attr=show_extended_attr, - show_host_status=show_host_status, - show_keypair=show_keypair, - show_srv_usg=show_srv_usg, - show_sec_grp=show_sec_grp, - show_extended_status=show_extended_status, - show_extended_volumes=show_extended_volumes, - show_server_groups=show_server_groups) except exception.InstanceNotFound: msg = _("Instance could not be found") raise exc.HTTPNotFound(explanation=msg) + show_server_groups = api_version_request.is_supported(req, '2.71') + # NOTE(gmann): Starting from microversion 2.75, PUT and Rebuild + # API response will show all attributes like GET /servers API. + show_all_attributes = api_version_request.is_supported(req, '2.75') + extend_address = show_all_attributes + show_AZ = show_all_attributes + show_config_drive = show_all_attributes + show_keypair = show_all_attributes + show_srv_usg = show_all_attributes + show_sec_grp = show_all_attributes + show_extended_status = show_all_attributes + show_extended_volumes = show_all_attributes + # NOTE(gmann): Below attributes need to be added in response + # if respective policy allows.So setting these as None + # to perform the policy check in view builder. + show_extended_attr = None if show_all_attributes else False + show_host_status = None if show_all_attributes else False + + return self._view_builder.show( + req, instance, + extend_address=extend_address, + show_AZ=show_AZ, + show_config_drive=show_config_drive, + show_extended_attr=show_extended_attr, + show_host_status=show_host_status, + show_keypair=show_keypair, + show_srv_usg=show_srv_usg, + show_sec_grp=show_sec_grp, + show_extended_status=show_extended_status, + show_extended_volumes=show_extended_volumes, + show_server_groups=show_server_groups) + # NOTE(gmann): Returns 204 for backwards compatibility but should be 202 # for representing async API as this API just accepts the request and # request hypervisor driver to complete the same in async mode. diff --git a/nova/api/validation/validators.py b/nova/api/validation/validators.py index ce0b2f57c1c4..7dd6b1e7f738 100644 --- a/nova/api/validation/validators.py +++ b/nova/api/validation/validators.py @@ -197,10 +197,7 @@ def _validate_az_name(instance): # you have multiple schemas, this method will delete properties that are not # allowed against earlier subschemas even if they're allowed (or even required) # by later subschemas. -def _soft_validate_additional_properties(validator, - additional_properties_value, - instance, - schema): +def _soft_validate_additional_properties(validator, value, instance, schema): """This validator function is used for legacy v2 compatible mode in v2.1. This will skip all the additional properties checking but keep check the 'patternProperties'. 'patternProperties' is used for metadata API. @@ -222,8 +219,7 @@ def _soft_validate_additional_properties(validator, are patternProperties specified, the extra properties will not be touched and raise validation error if pattern doesn't match. """ - if (not validator.is_type(instance, "object") or - additional_properties_value): + if not validator.is_type(instance, "object") or value is True: return properties = schema.get("properties", {}) @@ -240,6 +236,11 @@ def _soft_validate_additional_properties(validator, if not extra_properties: return + if set(extra_properties) == set(instance): + # NOTE(stephenfin): This is a bit of hack. If there are multiple + # sub-schemas (oneOf), we will expect to match on one but not the other + return + if patterns: error = "Additional properties are not allowed (%s %s unexpected)" if len(extra_properties) == 1: diff --git a/nova/tests/unit/api/openstack/compute/test_keypairs.py b/nova/tests/unit/api/openstack/compute/test_keypairs.py index 3b6db3cfc24c..ebf759c55787 100644 --- a/nova/tests/unit/api/openstack/compute/test_keypairs.py +++ b/nova/tests/unit/api/openstack/compute/test_keypairs.py @@ -303,10 +303,6 @@ class KeypairsTestV22(KeypairsTestV21): self): pass - def test_create_server_keypair_name_with_leading_trailing_compat_mode( - self): - pass - class KeypairsTestV210(KeypairsTestV22): wsgi_api_version = '2.10' @@ -315,10 +311,6 @@ class KeypairsTestV210(KeypairsTestV22): self): pass - def test_create_server_keypair_name_with_leading_trailing_compat_mode( - self): - pass - def test_keypair_list_other_user(self): req = fakes.HTTPRequest.blank( self.base_url + f'/os-keypairs?user_id={uuids.other_user_id}', diff --git a/nova/tests/unit/policies/test_servers.py b/nova/tests/unit/policies/test_servers.py index 87fae521f5dc..6f5c7a75474b 100644 --- a/nova/tests/unit/policies/test_servers.py +++ b/nova/tests/unit/policies/test_servers.py @@ -431,9 +431,13 @@ class ServersPolicyTest(base.BasePolicyTest): @mock.patch('nova.objects.BlockDeviceMappingList.bdms_by_instance_uuid') @mock.patch.object(InstanceGroup, 'get_by_instance_uuid') @mock.patch('nova.compute.api.API.update_instance') - def test_server_update_with_extra_specs_policy(self, - mock_update, mock_group, mock_bdm): + def test_server_update_with_extra_specs_policy( + self, mock_update, mock_group, mock_bdm, + ): mock_update.return_value = self.instance + mock_group.return_value = objects.InstanceGroup( + uuid=uuids.server_group) + rule = policies.SERVERS % 'update' # server 'update' policy is checked before flavor extra specs # policy so we have to allow it for everyone otherwise it will fail @@ -582,6 +586,7 @@ class ServersPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.update_instance') def test_update_server_policy(self, mock_update): + mock_update.return_value = self.instance rule_name = policies.SERVERS % 'update' body = {'server': {'name': 'test'}} @@ -607,7 +612,10 @@ class ServersPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.update_instance') def test_update_server_overridden_policy_pass_with_same_user( - self, mock_update): + self, mock_update, + ): + mock_update.return_value = self.instance + rule_name = policies.SERVERS % 'update' self.policy.set_rules({rule_name: "user_id:%(user_id)s"}, overwrite=False) @@ -977,10 +985,14 @@ class ServersPolicyTest(base.BasePolicyTest): @mock.patch.object(InstanceGroup, 'get_by_instance_uuid') @mock.patch('nova.compute.api.API.update_instance') @mock.patch('nova.compute.api.API.get_instance_host_status') - def test_server_update_with_extended_attr_policy(self, - mock_status, mock_update, mock_group, mock_bdm): - mock_update.return_value = self.instance + def test_server_update_with_extended_attr_policy( + self, mock_status, mock_update, mock_group, mock_bdm + ): mock_status.return_value = fields.HostStatus.UP + mock_update.return_value = self.instance + mock_group.return_value = objects.InstanceGroup( + uuid=uuids.server_group) + rule = policies.SERVERS % 'update' # server 'update' policy is checked before extended attributes # policy so we have to allow it for everyone otherwise it will fail @@ -1075,10 +1087,14 @@ class ServersPolicyTest(base.BasePolicyTest): @mock.patch.object(InstanceGroup, 'get_by_instance_uuid') @mock.patch('nova.compute.api.API.update_instance') @mock.patch('nova.compute.api.API.get_instance_host_status') - def test_server_update_with_host_status_policy(self, - mock_status, mock_update, mock_group, mock_bdm): - mock_update.return_value = self.instance + def test_server_update_with_host_status_policy( + self, mock_status, mock_update, mock_group, mock_bdm, + ): mock_status.return_value = fields.HostStatus.UP + mock_update.return_value = self.instance + mock_group.return_value = objects.InstanceGroup( + uuid=uuids.server_group) + rule = policies.SERVERS % 'update' # server 'update' policy is checked before host_status # policy so we have to allow it for everyone otherwise it will fail @@ -1192,10 +1208,14 @@ class ServersPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.get_instance_host_status') @mock.patch.object(InstanceGroup, 'get_by_instance_uuid') @mock.patch('nova.compute.api.API.update_instance') - def test_server_update_with_unknown_host_status_policy(self, - mock_update, mock_group, mock_status, mock_bdm): + def test_server_update_with_unknown_host_status_policy( + self, mock_update, mock_group, mock_status, mock_bdm, + ): mock_update.return_value = self.instance mock_status.return_value = fields.HostStatus.UNKNOWN + mock_group.return_value = objects.InstanceGroup( + uuid=uuids.server_group) + rule = policies.SERVERS % 'update' # server 'update' policy is checked before unknown host_status # policy so we have to allow it for everyone otherwise it will fail @@ -1221,8 +1241,7 @@ class ServersPolicyTest(base.BasePolicyTest): self.assertNotIn('host_status', resp['server']) @mock.patch('nova.compute.api.API.create') - def test_create_requested_destination_server_policy(self, - mock_create): + def test_create_requested_destination_server_policy(self, mock_create): # 'create' policy is checked before 'create:requested_destination' so # we have to allow it for everyone otherwise it will # fail for unauthorized contexts here.