Merge "api: Add response body schemas for servers APIs (5/6)"

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

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.

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,
+def _soft_validate_additional_properties(validator, value, instance, schema):
-                                         additional_properties_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
+    if not validator.is_type(instance, "object") or value is True:
-            additional_properties_value):
         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:

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}',

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,
+    def test_server_update_with_extra_specs_policy(
-            mock_update, mock_group, mock_bdm):
+        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,
+    def test_server_update_with_extended_attr_policy(
-        mock_status, mock_update, mock_group, mock_bdm):
+        self, mock_status, mock_update, mock_group, mock_bdm
-        mock_update.return_value = self.instance
+    ):
         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,
+    def test_server_update_with_host_status_policy(
-        mock_status, mock_update, mock_group, mock_bdm):
+        self, mock_status, mock_update, mock_group, mock_bdm,
-        mock_update.return_value = self.instance
+    ):
         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,
+    def test_server_update_with_unknown_host_status_policy(
-        mock_update, mock_group, mock_status, mock_bdm):
+        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,
+    def test_create_requested_destination_server_policy(self, mock_create):
-        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.