Jump to: navigation, search

Difference between revisions of "NovaApiValidationFramework"

(Undefined API parameter should be invalid?)
(Combination of v2.1 and v3 APIs)
 
(147 intermediate revisions by the same user not shown)
Line 1: Line 1:
 
<big>'''Nova API Validation Framework'''</big>
 
<big>'''Nova API Validation Framework'''</big>
 +
 +
== Discussion ==
 +
https://etherpad.openstack.org/NovaApiValidationFramework
  
 
== Introduction ==
 
== Introduction ==
Line 21: Line 24:
 
Moreover, API validation is an important feature from the viewpoint of HTTP service security.<br />
 
Moreover, API validation is an important feature from the viewpoint of HTTP service security.<br />
  
=== Background ===
 
  
Figure 1 shows the current API validation overview in Nova.<br />
+
As Nova is an HTTP service, it should be able to validate API parameters as follows:
# Select API method corresponding to the received API request.
+
* Validate every API parameter.
# Execute API method.
+
* Return an error response before API operation, if API parameter is invalid.
# Possible to execute certain operations even if a parameter is invalid.
+
* Unify the error message format of the response, if the cause is the same.<br />(ex) ".. is too short.", ".. is too long.", ".. is not integer."
 +
 
 +
== What is necessary for Nova-v3-API ==
  
[[File:Current validation.png|none|Figure 1]]
+
This framework target is Nova-v3-API, because it is new and the API compatibility issues would not happen.<br />
'''Figure 1'''
+
On Havana release, Nova-v3-API is experimental and the API will become generic in Icehouse.<br />
 +
In Icehouse development cycle, the web framework of Nova-v3-API would move to Pecan/WSME.<br />
 +
WSME has original API validation mechanism already, and we'd better to find good API validation feature if we need it.<br />
 +
To find good feature, we'd better to discuss it based on current Nova-v3-API implementation.<br />
  
Not all API parameters are completely validated, so many API operations are executed without API parameter validation.<br />
+
This investigation is based on the following commit of Nova source code:
Furthermore, it is difficult to handle error messages on the API caller side because error message formats are not unified.<br />
+
commit 5f0591252dc5d6e3f49682f85dcdbd99f692c07a
 +
Merge: 0211752 a19d72b
 +
Author: Jenkins <jenkins@review.openstack.org>
 +
Date:  Mon Oct 7 03:14:43 2013 +0000
 +
 +
    Merge "Fixes rescue doesn't honor enable password conf for v3"
  
=== Objectives ===
+
=== Basic validation feature ===
  
As Nova is an HTTP service, it should be able to validate API parameters as follows:
+
The table of [[NovaApiValidationFramework#Appendix:_Nova-v3-API_parameters|Appendix: Nova-v3-API parameters]] shows all parameters of Nova-v3-API.<br />
* Validate every API parameter.
+
It seems that we need some basic validation features such as:
* Return an error response before API operation, if API parameter is invalid.
+
* Type validation
* Unify the error message format of the response, if the cause is the same.<br />(ex) ".. is too short.", ".. is too long.", ".. is not integer."
+
** str(name, ..), int(vcpus, ..), float(rxtx_factor), dict(metadata, ..), list(networks, ..), bool(conbine, ..), None(availability_zone)
 +
* String length validation
 +
** 1 - 255
 +
* Value range validation
 +
** value >= 0(rotation, ..), value > 0(vcpus, ..), value >= 1(os-multiple-create:min_count, os-multiple-create:max_count)
 +
* Data format validation
 +
** '''Pattern:''' uuid(volume_id, ..), boolean(on_shared_storage, ..), base64encoded(contents), ipv4(access_ip_v4,  fixed_ip), ipv6(access_ip_v6)
 +
** '''Allowed list:''' 'active' or 'error'(state), 'parent' or 'child'(cells.type), 'MANUAL' or 'AUTO'(os-disk-config:disk_config), ...
 +
** '''Allowed string:''' not contain '!' and '.'(cells.name), contain [a-zA-Z0-9_.- ] only(flavor.name, flavor.id)
 +
* Mandatory validation
 +
** '''Required:''' server.name, flavor.name, ..
 +
** '''Optional:''' flavor.ephemeral, flavor.swap, ..
  
== Proposal ==
+
=== Auxiliary validation feature ===
  
We would like to propose an API validation framework to implement comprehensive validation.<br />
+
Some parameters have dependency between other parameter.<br />
 +
For example, name or/and availability_zone should be specified when updating an aggregate.<br />
 +
The parameter dependencies are few cases, and the dependency validation feature would not be mandatory.<br />
 +
The cases are the following:
 +
* '''Required if not specifying other:''' (update aggregate: name or availability_zone), (host: status or maintenance_mode), (server: os-block-device-mapping:block_device_mapping or image_ref)
 +
* '''Should not specify both:''' (interface_attachment: net_id and port_id), (server: fixed_ip and port)
  
=== Validation point ===
+
== Implementation ==
  
In this framework, the API validations are separated from API methods, and the API validations are executed before each API method.<br />
+
In Icehouse summit, we have gotten a consensus API validation of v3 API will be implemented with JSONSchema.
Figure 2 shows the API validation of this framework.<br />
+
The implementation is going on https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/v3-api-schema,n,z
# Select both the API schema definition and API method corresponding to the received API request.
 
# Validate API parameters based on the API schema definition.
 
# Execute API method if the API validation is successful.
 
[[File:New validation.png|none|Figure 2]]
 
'''Figure 2'''
 
  
=== Merits ===
+
== Combination of v2.1 and v3 APIs ==
  
The OpenStack community releases a new Nova every six months, and the Nova APIs are increasing with each version.
+
[[File:Nova-v2-v3.png]]
Nova currently has hundreds of APIs.<br />
 
With multiple parameters for each API, a lot of effort is required to review and implement the API schema definition.<br />
 
However, we believe this framework is worth implementing because of the following merits.
 
  
* Clarification of the API parameter definitions.<br />The type and acceptable range and length of each parameter are clarified in the API schema definitions.<br />By defining the schemas of every API, developers will be able to create application more easily using Nova APIs.
+
== Appendix: Nova-v3-API parameters ==
* Cleaned up code<br />The amount of Nova code can be reduced by merging validations and error response methods.<br />For example, in the “create a virtual machine instance” API, the following 8 validation lines exist for the “min_count” parameter.
 
        try:
 
            min_count = int(str(min_count))
 
        except ValueError:
 
            msg = _('min_count must be an integer value')
 
            raise exc.HTTPBadRequest(explanation=msg)
 
        if min_count < 1:
 
            msg = _('min_count must be > 0')
 
            raise exc.HTTPBadRequest(explanation=msg)
 
:By using this proposed framework, we can merge these into the following single line.
 
        'min_count': {'type': 'integer', 'minimum': 1},
 
  
== Discussion ==
+
This investigation is based on the following commit of Nova source code:
 +
commit 5f0591252dc5d6e3f49682f85dcdbd99f692c07a
 +
Merge: 0211752 a19d72b
 +
Author: Jenkins <jenkins@review.openstack.org>
 +
Date:  Mon Oct 7 03:14:43 2013 +0000
 +
 +
    Merge "Fixes rescue doesn't honor enable password conf for v3"
  
=== Undefined API parameter should be invalid? ===
+
The source files of Nova-v3-API are put under ./nova/api/openstack/compute/plugins/v3/, and there are 79 API methods.<br />
If the passed API parameter is not defined in API schema, should this validation framework distinguish invalid parameter?
+
In these APIs, 49 APIs use API parameters of a request body, and they have 148 API parameters.<br />
For example, if an API schema is the following and parameter 'abc' is passed though API call, should the API call  be invalid?
+
(32% API parameters(48/148) are not validated with any ways.)<br/>
{
+
The following table shows all API parameters and each API validating implementation.<br />
    'type' : 'object',
 
    'properties' : {
 
        'server': {
 
            'properties' : {
 
                'name': {'type': 'string', 'minLength': 1,
 
                        'maxLength': 255, 'required': True},
 
          }
 
        }
 
    }
 
}
 
JSON Schema library cannot judge undefined parameter is invalid, so we need another implementation for the above.
 
  
=== API schema: define by API method or API(METHOD, URL)? ===
+
* <big>NV</big>: any elements are not validated
  
On JSON Schema prototype, API schema was defined by API method.
 
When API validation, the framework got API schema from API method and used it for validation.<br />
 
        schema = getattr(meth, 'validator_schema', None)
 
        if schema:
 
            validator = jsonschema.Draft3Validator(schema, types=va.types)
 
            try:
 
                validator.validate(action_args['body'])
 
            except jsonschema.ValidationError as ex:
 
                msg = _("Invalid API parameter: %s") % ex.args[0]
 
                return Fault(webob.exc.HTTPBadRequest(explanation=msg))
 
On the other hand, there are cases that multiple APIs(METHOD, URL) are implemented by the same API method.<br />
 
 
{| class="wikitable"
 
{| class="wikitable"
 
|-
 
|-
! METHOD !! URL !! Controller !! API method
+
! Class !! API method !! API parameter !! Required/Optional !! Type !! Length/Range !! Data format
 +
|-
 +
| AdminActionsController || _create_backup() || name || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || backup_type || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || rotation || Required || int ||  >= 0 ||
 +
|-
 +
|  ||  || metadata || Optional || dict || "{key: value} 1 < len(key) < 255" ||
 +
|-
 +
| AdminActionsController || _migrate_live() || block_migration || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || disk_over_commit || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || host || Required || <big>NV</big> ||  ||
 +
|-
 +
| AdminActionsController || _reset_state() || state || Required || str ||  || "active"or "error"
 +
|-
 +
| AdminPasswordController || change_password() || admin_password || Required || str ||  ||
 +
|-
 +
| AgentController || create() || hypervisor || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || os || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || architecture || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || version || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || url || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || md5hash || Required || <big>NV</big> ||  ||
 +
|-
 +
| AgentController || update() || url || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || md5hash || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || version || Required || <big>NV</big> ||  ||
 +
|-
 +
| AggregateController || create() || name || Required || str || 1 - 255 ||
 +
|-
 +
|  ||  || availability_zone || Required || None or str ||  ||
 +
|-
 +
| AggregateController || update() || name || Required if not specifying availability_zone || str || 1 - 255 ||
 +
|-
 +
|  ||  || availability_zone || Required if not specifying name || None or str ||  ||
 +
|-
 +
| AggregateController || _add_host() || host || Required || str ||  ||
 +
|-
 +
| AggregateController || _remove_host() || host || Required || str ||  ||
 +
|-
 +
| AggregateController || _set_metadata() || metadata || Required || dict ||  ||
 +
|-
 +
| InterfaceAttachmentController || create() || net_id || Optional, Should not specify both net_id and port_id || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || port_id || Optional, Should not specify both net_id and port_id || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || ip_address || Optional, Should specify both req_ip and net_id || <big>NV</big> ||  ||
 +
|-
 +
| CellsController || create() || name || Required || str ||  || not "!" and "."
 +
|-
 +
|  ||  || type || Optional || str ||  || "parent" or "child"
 +
|-
 +
| CellsController || update() || name || Optional || str ||  || not "!" and "."
 +
|-
 +
|  ||  || type || Optional || str ||  || "parent" or "child"
 +
|-
 +
| CellsController || sync_instances() || project_id || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || deleted || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || updated_since || Optional || <big>NV</big> ||  ||
 +
|-
 +
| ConsoleOutputController || get_console_output() || length || Optional || int ||  || Cast by int(str(length))
 +
|-
 +
| CoverageController || _start_coverage() || combine || Optional || bool ||  || boolean
 +
|-
 +
| CoverageController || _report_coverage() || file || Optional || str ||  || filename (if path != os.path.basename(path): Error)
 +
|-
 +
|  ||  || xml || Optional || bool ||  ||
 +
|-
 +
|  ||  || html || Optional || bool ||  ||
 +
|-
 +
| ServerDiskConfigController || create() || os-disk-config:disk_config || Optional || str ||  || "MANUAL" or "AUTO"
 +
|-
 +
| ServerDiskConfigController || update() || os-disk-config:disk_config || Optional || str ||  || "MANUAL" or "AUTO"
 +
|-
 +
| ServerDiskConfigController || _action_rebuild() || os-disk-config:disk_config || Required || str ||  || "MANUAL" or "AUTO"
 +
|-
 +
| ServerDiskConfigController || _action_resize() || os-disk-config:disk_config || Required || str ||  || "MANUAL" or "AUTO"
 +
|-
 +
| EvacuateController || _evacuate() || host || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || on_shared_storage || Required || str ||  || boolean
 +
|-
 +
|  ||  || admin_password || Optional || <big>NV</big> ||  ||
 +
|-
 +
| ExtendedVolumesController || swap() || old_volume_id || Required || str ||  || uuid
 +
|-
 +
|  ||  || new_volume_id || Required || str ||  || uuid
 +
|-
 +
| ExtendedVolumesController || attach() || volume_id || Required || str ||  || uuid
 +
|-
 +
|  ||  || device || Required || <big>NV</big> ||  ||
 +
|-
 +
| ExtendedVolumesController || detach() || volume_id || Required || str ||  || uuid
 +
|-
 +
| FlavorActionController || _add_tenant_access() || tenant_id || Required || <big>NV</big> ||  ||
 +
|-
 +
| FlavorActionController || _remove_tenant_access() || tenant_id || Required || <big>NV</big> ||  ||
 +
|-
 +
| FlavorManageController || _create() || name || Required || str || 1 - 255 || names can only contain [a-zA-Z0-9_.- ]
 +
|-
 +
|  ||  || id || Required || int or str || 1 - 255 || id can only contain [a-zA-Z0-9_.-]
 +
|-
 +
|  ||  || ram || Required || int ||  > 0 ||
 +
|-
 +
|  ||  || vcpus || Required || int ||  > 0 ||
 +
|-
 +
|  ||  || disk || Required || int ||  >= 0 ||
 +
|-
 +
|  ||  || ephemeral || Optional || int ||  >= 0 ||
 +
|-
 +
|  ||  || swap || Optional || int ||  >= 0 ||
 +
|-
 +
|  ||  || rxtx_factor || Optional || float ||  > 0 ||
 
|-
 
|-
| POST || /{project_id}/servers || nova.api.openstack.compute.servers.Controller || create
+
| || || os-flavor-access:is_public || Optional || str ||  || '1', 't', 'true', 'on', 'y', 'yes', '0', 'f', 'false', 'off', 'n', 'no'
 
|-
 
|-
| POST || /{project_id}/os-create-server-ext || nova.api.openstack.compute.servers.Controller || create
+
| FlavorExtraSpecsController || create() || extra_specs || Optional || dict ||  ||
 
|-
 
|-
| POST || /{project_id}/os-volumes_boot || nova.api.openstack.compute.servers.Controller || create
+
| FlavorExtraSpecsController || update() ||  || Optional || dict ||  ||
 +
|-
 +
| HostController || update() || status || Required if not specifying maintenance_mode || str ||  || "enable" or "disable"
 +
|-
 +
|  ||  || maintenance_mode || Required if not specifying status || str ||  || "enable" or "disable"
 +
|-
 +
| KeypairController || create() || name || Required || str? (not type checked) || 1 - 255 ||
 +
|-
 +
|  ||  || public_key || Optional || <big>NV</big> ||  ||
 +
|-
 +
| MultinicController || _add_fixed_ip() || network_id || Required || <big>NV</big> ||  ||
 +
|-
 +
| MultinicController || _remove_fixed_ip() || address || Required || <big>NV</big> ||  ||
 +
|-
 +
| QuotaClassSetsController || update() || instances || Optional || int ||  ||
 +
|-
 +
|  ||  || cores || Optional || int ||  ||
 +
|-
 +
|  ||  || ram || Optional || int ||  ||
 +
|-
 +
|  ||  || security_groups || Optional || int ||  ||
 +
|-
 +
|  ||  || floating_ips || Optional || int ||  ||
 +
|-
 +
|  ||  || fixed_ips || Optional || int ||  ||
 +
|-
 +
|  ||  || metadata_items || Optional || int ||  ||
 +
|-
 +
|  ||  || injected_files || Optional || int ||  ||
 +
|-
 +
|  ||  || injected_file_content_bytes || Optional || int ||  ||
 +
|-
 +
|  ||  || injected_file_path_bytes || Optional || int ||  ||
 +
|-
 +
|  ||  || security_group_rules || Optional || int ||  ||
 +
|-
 +
|  ||  || key_pairs || Optional || int ||  ||
 +
|-
 +
| QuotaSetsController || update() || instances || Optional || int ||  ||
 +
|-
 +
|  ||  || cores || Optional || int ||  ||
 +
|-
 +
|  ||  || ram || Optional || int ||  ||
 +
|-
 +
|  ||  || security_groups || Optional || int ||  ||
 +
|-
 +
|  ||  || floating_ips || Optional || int ||  ||
 +
|-
 +
|  ||  || fixed_ips || Optional || int ||  ||
 +
|-
 +
|  ||  || metadata_items || Optional || int ||  ||
 +
|-
 +
|  ||  || injected_files || Optional || int ||  ||
 +
|-
 +
|  ||  || injected_file_content_bytes || Optional || int ||  ||
 +
|-
 +
|  ||  || injected_file_path_bytes || Optional || int ||  ||
 +
|-
 +
|  ||  || security_group_rules || Optional || int ||  ||
 +
|-
 +
|  ||  || key_pairs || Optional || int ||  ||
 +
|-
 +
|  ||  || force || Optional || str ||  || boolean
 +
|-
 +
| RescueController || _rescue() || admin_pass || Optional || <big>NV</big> ||  ||
 +
|-
 +
| SchedulerHintsController || create() || os-scheduler-hints:scheduler_hints || Optional || <big>NV</big> ||  ||
 +
|-
 +
| ServerMetadataController || create() || metadata || Required || dict ||  ||
 +
|-
 +
| ServerMetadataController || update() || metadata || Required || dict || len(dict) == 1 ||
 +
|-
 +
| ServerMetadataController || update_all() || metadata || Required || dict ||  ||
 +
|-
 +
| ServersController || create() || admin_pass || Optional || str ||  ||
 +
|-
 +
|  ||  || name || Required || str || 1 - 255 ||
 +
|-
 +
|  ||  || image_ref || Required if not specifying os-block-device-mapping:block_device_mapping || str ||  || uuid or ".*/<uuid>"
 +
|-
 +
|  ||  || os-block-device-mapping:block_device_mapping || Required if not specifying image_ref || dict ||  ||
 +
|-
 +
|  ||  ||  source_type || Required || str ||  || 'volume', 'image', 'snapshot', 'blank'
 +
|-
 +
|  ||  ||  uuid || Required if source_type is not 'blank' || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || os-availability-zone:availability_zone || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || os-config-drive:config_drive || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || auto_disk_config || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || key_name || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || os-multiple-create:min_count || Optional || int ||  >= 1 ||
 +
|-
 +
|  ||  || os-multiple-create:max_count || Optional || int ||  >= 1 ||
 +
|-
 +
|  ||  || os-multiple-create:return_reservation_id || Optional || bool ||  ||
 +
|-
 +
|  ||  || personality || Optional || dict ||  ||
 +
|-
 +
|  ||  ||  path || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  ||  contents || Required || str ||  || base64encoded
 +
|-
 +
|  ||  || scheduler_hints || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || os-security-groups:security_groups || Optional || list ||  ||
 +
|-
 +
|  ||  ||  name || Optional || str ||  ||
 +
|-
 +
|  ||  || os-user-data:user_data || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || networks || Optional || list of dict ||  ||
 +
|-
 +
|  ||  ||  fixed_ip || Optional, Should not specify both fixed_ip and port ||  ||  || ipv4
 +
|-
 +
|  ||  ||  port || Optional, Should not specify both fixed_ip and port ||  ||  || uuid
 +
|-
 +
|  ||  || access_ip_v4 || Optional ||  ||  || ipv4
 +
|-
 +
|  ||  || access_ip_v6 || Optional ||  ||  || ipv6
 +
|-
 +
|  ||  || flavor_ref || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || metadata || Optional ||  || {key: value}, 1 < len(key) < 255, len(value) < 255" ||
 +
|-
 +
| ServersController || update() || name || Optional || str || 1 - 255 ||
 +
|-
 +
|  ||  || access_ip_v4 || Optional ||  ||  || ipv4
 +
|-
 +
|  ||  || access_ip_v6 || Optional ||  ||  || ipv6
 +
|-
 +
|  ||  || auto_disk_config || Optional || <big>NV</big> ||  ||
 +
|-
 +
| ServersController || _action_reboot() || type || Required || str ||  || "HARD" or "SOFT", not allowed lower case.
 +
|-
 +
| ServersController || _action_resize() || flavor_ref || Required || <big>NV</big> ||  || validated with the existing flavors.
 +
|-
 +
| ServersController || _action_rebuild() || image_ref || Required || str ||  || uuid or ".*/<uuid>"
 +
|-
 +
|  ||  || admin_pass || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || name || Optional || str || 1 - 255 ||
 +
|-
 +
|  ||  || access_ip_v4 || Optional ||  ||  || ipv4
 +
|-
 +
|  ||  || access_ip_v6 || Optional ||  ||  || ipv6
 +
|-
 +
|  ||  || auto_disk_config || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || personality || Optional || <big>NV</big> ||  ||
 +
|-
 +
|  ||  ||  path || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  ||  contents || Required || str ||  || base64encoded
 +
|-
 +
| ServersController || _action_create_image() || name || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || metadata || Optional || dict ||  ||
 +
|-
 +
| ServiceController || update() || host || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || binary || Required || <big>NV</big> ||  ||
 +
|-
 +
|  ||  || disabled_reason || Required if id is disable-log-reason || str || 1 - 255 ||
 
|}
 
|}
So if we need rigorous validation that checks API parameter existences by each API(METHOD, URL), we need to get API schema from each API(not API method).
 
 
== Prototype 1: by JSON Schema ==
 
 
Using the JSON Schema library, we have created a prototype and examined this API validation framework.<br />
 
The prototype validates the "create a virtual machine instance" API, as it is an often-used API and has many API parameters.<br />
 
For other APIs, the intent is to implement them as needed based on OpenStack community discussions.
 
 
=== API parameter type ===
 
 
The default types in the JSON Schema are basic types such as integer, float, string, and boolean.<br />
 
Nova has many APIs, and we need additional types(uuid, url, etc.) for rigorous validation.<br />
 
Current expected types are the following:
 
* integer<br />minimum, maximum
 
* float<br />minimum, maximum
 
* string<br />minLength, maxLength
 
* uuid
 
* url
 
* ipv4 address
 
* ipv6 address
 
* base64 encoded data
 
<br />
 
In the JSON Schema library, the type validation is implemented by the isinstance() method.<br />
 
isinstance() calls the meta class's __instancecheck__() method, which is an argument specification type class.<br />
 
Therefore, we can implement additional types by defining both the meta class and type class like the following:
 
class ipv4Meta(type):
 
    def __instancecheck__(self, instance):
 
        return utils.is_valid_ipv4(instance)
 
 
class ipv4:
 
    __metaclass__ = ipv4Meta
 
 
We can validate API parameters with additional types by specifying the argument "types" of a Validator instance.
 
types={
 
    'ipv4': ipv4,
 
    'ipv6': ipv6,
 
    [..]
 
}
 
 
validator = jsonschema.Draft3Validator(schema, types=types)
 
try:
 
    validator.validate(action_args['body'])
 
except jsonschema.ValidationError as ex:
 
    msg = _("Invalid API parameter: %s") % ex.args[0]
 
    return Fault(webob.exc.HTTPBadRequest(explanation=msg))
 
 
=== API schema ===
 
 
For the creation of the prototype, each API schema was defined using JSON Schema.<br />
 
Each API parameter was defined as needed using the additional types described in "3.1. API parameter type".<br />
 
The following shows the API schema for "create a virtual machine instance".<br />
 
Also, required API parameters were defined with "'required': True".
 
{
 
    'type' : 'object',
 
    'properties' : {
 
        'server': {
 
            'properties' : {
 
                'name': {'type': 'string', 'minLength': 1,
 
                          'maxLength': 255, 'required': True},
 
                'imageRef': {'type': 'intOrUuidOrUrl', 'required': True},
 
                'flavorRef': {'type': 'intOrUuidOrUrl', 'required': True},
 
                'min_count': {'type': 'integer', 'minimum': 1},
 
                'max_count': {'type': 'integer', 'minimum': 1},
 
                'accessIPv4': {'type': 'ipv4'},
 
                'accessIPv6': {'type': 'ipv6'},
 
[..]
 
            }
 
        }
 
    }
 
}
 
In this prototype, the API schema was defined just before each API method in the same source file.<br />
 
It was felt that this would make it easy to define each API schema.<br />
 
On the other hand, it may be better to define each schema in a separate file from the source code in order to keep all the API schema definitions in single file.<br />
 
This should be reconsidered when the number of API schema definitions increase.
 
 
'''Discussion on review.openstack.org'''
 
https://review.openstack.org/#/c/25358/1/nova/api/openstack/compute/servers.py
 
* Define the schema elsewhere, perhaps as a top-level constant? By including it directly here, it makes it a little more difficult to follow what's happening.
 
* Export all schemas to one module like schema.py
 
 
== Migration plan ==
 
TBD
 
 
== Future ==
 
 
I wish the following implementations after applying this framework to every Nova APIs.
 
 
* Add the existence check of API parameter definition to Jenkins<br />OpenStack utilizes a good development process through Jenkins which is used to verify the coding style and to ensure functionality, when a code change is proposed. We feel it is good to add the existence check of API parameter definition to this process. By doing so, we will be able to keep the APIs secure.
 
 
* Create API specification document<br />We think it is possible to create the elements of an API specification document from the API schema definition.<br />OpenStack community has API specification documents of all OpenStack components[1] and ones for each component[2].<br />We can extract certain elements of all the API parameters from the API schema definition.<br />[1]: http://api.openstack.org/api-ref.html<br />[2]: Nova: http://docs.openstack.org/api/openstack-compute/2/content/
 
 
* Apply API validation framework to other components<br />By applying this framework to other components, we feel that the overall safety OpenStack APIs will be improved.<br />If we can successfully apply this framework to Nova, which contains many APIs with complex API parameters, we will be able to apply it to other components as well.<br />We hope to collaborate with many developers on this framework.
 
 
== Appendix ==
 
  
* Prototype of API validation framework: https://review.openstack.org/#/c/25358/
+
== Appendix: URL ==
 +
* Discussion
 +
** https://etherpad.openstack.org/NovaApiValidationFramework
 +
** http://lists.openstack.org/pipermail/openstack-dev/2013-October/016649.html
 +
* API validation coverage improvement
 +
** https://etherpad.openstack.org/p/NovaApiValidationCoverage

Latest revision as of 08:40, 24 April 2014

Nova API Validation Framework

Discussion

https://etherpad.openstack.org/NovaApiValidationFramework

Introduction

Nova is a RESTful API based HTTP service.
These APIs allow for cloud managed server operations.
For example, we can create a virtual machine instance by sending the following POST request to Nova's URI: /servers.

{
   "server" : {
       "name" : "new-server-test",
       "imageRef" : "http://servers.api.openstack.org/1234/images/52415800-8b69-11e0-9b19-734f6f006e54",
       "flavorRef" : 1
   }
}

Nova contains many RESTful APIs to facilitate its many features.
Moreover, each RESTful API can be controlled flexibly through multiple parameters.
For example, the above "create a virtual machine instance" API has 19 parameters including "name", "imageRef", etc.

The HTTP service need to validate practically every API parameter in terms of acceptable type and minimum/maximum length and range.
Unnecessary workload can be avoided by validating API parameters before executing the API operation.
Moreover, API validation is an important feature from the viewpoint of HTTP service security.


As Nova is an HTTP service, it should be able to validate API parameters as follows:

  • Validate every API parameter.
  • Return an error response before API operation, if API parameter is invalid.
  • Unify the error message format of the response, if the cause is the same.
    (ex) ".. is too short.", ".. is too long.", ".. is not integer."

What is necessary for Nova-v3-API

This framework target is Nova-v3-API, because it is new and the API compatibility issues would not happen.
On Havana release, Nova-v3-API is experimental and the API will become generic in Icehouse.
In Icehouse development cycle, the web framework of Nova-v3-API would move to Pecan/WSME.
WSME has original API validation mechanism already, and we'd better to find good API validation feature if we need it.
To find good feature, we'd better to discuss it based on current Nova-v3-API implementation.

This investigation is based on the following commit of Nova source code:

commit 5f0591252dc5d6e3f49682f85dcdbd99f692c07a	
Merge: 0211752 a19d72b	
Author: Jenkins <jenkins@review.openstack.org>	
Date:   Mon Oct 7 03:14:43 2013 +0000	

    Merge "Fixes rescue doesn't honor enable password conf for v3"

Basic validation feature

The table of Appendix: Nova-v3-API parameters shows all parameters of Nova-v3-API.
It seems that we need some basic validation features such as:

  • Type validation
    • str(name, ..), int(vcpus, ..), float(rxtx_factor), dict(metadata, ..), list(networks, ..), bool(conbine, ..), None(availability_zone)
  • String length validation
    • 1 - 255
  • Value range validation
    • value >= 0(rotation, ..), value > 0(vcpus, ..), value >= 1(os-multiple-create:min_count, os-multiple-create:max_count)
  • Data format validation
    • Pattern: uuid(volume_id, ..), boolean(on_shared_storage, ..), base64encoded(contents), ipv4(access_ip_v4, fixed_ip), ipv6(access_ip_v6)
    • Allowed list: 'active' or 'error'(state), 'parent' or 'child'(cells.type), 'MANUAL' or 'AUTO'(os-disk-config:disk_config), ...
    • Allowed string: not contain '!' and '.'(cells.name), contain [a-zA-Z0-9_.- ] only(flavor.name, flavor.id)
  • Mandatory validation
    • Required: server.name, flavor.name, ..
    • Optional: flavor.ephemeral, flavor.swap, ..

Auxiliary validation feature

Some parameters have dependency between other parameter.
For example, name or/and availability_zone should be specified when updating an aggregate.
The parameter dependencies are few cases, and the dependency validation feature would not be mandatory.
The cases are the following:

  • Required if not specifying other: (update aggregate: name or availability_zone), (host: status or maintenance_mode), (server: os-block-device-mapping:block_device_mapping or image_ref)
  • Should not specify both: (interface_attachment: net_id and port_id), (server: fixed_ip and port)

Implementation

In Icehouse summit, we have gotten a consensus API validation of v3 API will be implemented with JSONSchema. The implementation is going on https://review.openstack.org/#/q/project:openstack/nova+branch:master+topic:bp/v3-api-schema,n,z

Combination of v2.1 and v3 APIs

Nova-v2-v3.png

Appendix: Nova-v3-API parameters

This investigation is based on the following commit of Nova source code:

commit 5f0591252dc5d6e3f49682f85dcdbd99f692c07a	
Merge: 0211752 a19d72b	
Author: Jenkins <jenkins@review.openstack.org>	
Date:   Mon Oct 7 03:14:43 2013 +0000	

    Merge "Fixes rescue doesn't honor enable password conf for v3"	

The source files of Nova-v3-API are put under ./nova/api/openstack/compute/plugins/v3/, and there are 79 API methods.
In these APIs, 49 APIs use API parameters of a request body, and they have 148 API parameters.
(32% API parameters(48/148) are not validated with any ways.)
The following table shows all API parameters and each API validating implementation.

  • NV: any elements are not validated
Class API method API parameter Required/Optional Type Length/Range Data format
AdminActionsController _create_backup() name Required NV
backup_type Required NV
rotation Required int >= 0
metadata Optional dict "{key: value} 1 < len(key) < 255"
AdminActionsController _migrate_live() block_migration Required NV
disk_over_commit Required NV
host Required NV
AdminActionsController _reset_state() state Required str "active"or "error"
AdminPasswordController change_password() admin_password Required str
AgentController create() hypervisor Required NV
os Required NV
architecture Required NV
version Required NV
url Required NV
md5hash Required NV
AgentController update() url Required NV
md5hash Required NV
version Required NV
AggregateController create() name Required str 1 - 255
availability_zone Required None or str
AggregateController update() name Required if not specifying availability_zone str 1 - 255
availability_zone Required if not specifying name None or str
AggregateController _add_host() host Required str
AggregateController _remove_host() host Required str
AggregateController _set_metadata() metadata Required dict
InterfaceAttachmentController create() net_id Optional, Should not specify both net_id and port_id NV
port_id Optional, Should not specify both net_id and port_id NV
ip_address Optional, Should specify both req_ip and net_id NV
CellsController create() name Required str not "!" and "."
type Optional str "parent" or "child"
CellsController update() name Optional str not "!" and "."
type Optional str "parent" or "child"
CellsController sync_instances() project_id Optional NV
deleted Optional NV
updated_since Optional NV
ConsoleOutputController get_console_output() length Optional int Cast by int(str(length))
CoverageController _start_coverage() combine Optional bool boolean
CoverageController _report_coverage() file Optional str filename (if path != os.path.basename(path): Error)
xml Optional bool
html Optional bool
ServerDiskConfigController create() os-disk-config:disk_config Optional str "MANUAL" or "AUTO"
ServerDiskConfigController update() os-disk-config:disk_config Optional str "MANUAL" or "AUTO"
ServerDiskConfigController _action_rebuild() os-disk-config:disk_config Required str "MANUAL" or "AUTO"
ServerDiskConfigController _action_resize() os-disk-config:disk_config Required str "MANUAL" or "AUTO"
EvacuateController _evacuate() host Required NV
on_shared_storage Required str boolean
admin_password Optional NV
ExtendedVolumesController swap() old_volume_id Required str uuid
new_volume_id Required str uuid
ExtendedVolumesController attach() volume_id Required str uuid
device Required NV
ExtendedVolumesController detach() volume_id Required str uuid
FlavorActionController _add_tenant_access() tenant_id Required NV
FlavorActionController _remove_tenant_access() tenant_id Required NV
FlavorManageController _create() name Required str 1 - 255 names can only contain [a-zA-Z0-9_.- ]
id Required int or str 1 - 255 id can only contain [a-zA-Z0-9_.-]
ram Required int > 0
vcpus Required int > 0
disk Required int >= 0
ephemeral Optional int >= 0
swap Optional int >= 0
rxtx_factor Optional float > 0
os-flavor-access:is_public Optional str '1', 't', 'true', 'on', 'y', 'yes', '0', 'f', 'false', 'off', 'n', 'no'
FlavorExtraSpecsController create() extra_specs Optional dict
FlavorExtraSpecsController update() Optional dict
HostController update() status Required if not specifying maintenance_mode str "enable" or "disable"
maintenance_mode Required if not specifying status str "enable" or "disable"
KeypairController create() name Required str? (not type checked) 1 - 255
public_key Optional NV
MultinicController _add_fixed_ip() network_id Required NV
MultinicController _remove_fixed_ip() address Required NV
QuotaClassSetsController update() instances Optional int
cores Optional int
ram Optional int
security_groups Optional int
floating_ips Optional int
fixed_ips Optional int
metadata_items Optional int
injected_files Optional int
injected_file_content_bytes Optional int
injected_file_path_bytes Optional int
security_group_rules Optional int
key_pairs Optional int
QuotaSetsController update() instances Optional int
cores Optional int
ram Optional int
security_groups Optional int
floating_ips Optional int
fixed_ips Optional int
metadata_items Optional int
injected_files Optional int
injected_file_content_bytes Optional int
injected_file_path_bytes Optional int
security_group_rules Optional int
key_pairs Optional int
force Optional str boolean
RescueController _rescue() admin_pass Optional NV
SchedulerHintsController create() os-scheduler-hints:scheduler_hints Optional NV
ServerMetadataController create() metadata Required dict
ServerMetadataController update() metadata Required dict len(dict) == 1
ServerMetadataController update_all() metadata Required dict
ServersController create() admin_pass Optional str
name Required str 1 - 255
image_ref Required if not specifying os-block-device-mapping:block_device_mapping str uuid or ".*/<uuid>"
os-block-device-mapping:block_device_mapping Required if not specifying image_ref dict
source_type Required str 'volume', 'image', 'snapshot', 'blank'
uuid Required if source_type is not 'blank' NV
os-availability-zone:availability_zone Optional NV
os-config-drive:config_drive Optional NV
auto_disk_config Optional NV
key_name Optional NV
os-multiple-create:min_count Optional int >= 1
os-multiple-create:max_count Optional int >= 1
os-multiple-create:return_reservation_id Optional bool
personality Optional dict
path Required NV
contents Required str base64encoded
scheduler_hints Optional NV
os-security-groups:security_groups Optional list
name Optional str
os-user-data:user_data Optional NV
networks Optional list of dict
fixed_ip Optional, Should not specify both fixed_ip and port ipv4
port Optional, Should not specify both fixed_ip and port uuid
access_ip_v4 Optional ipv4
access_ip_v6 Optional ipv6
flavor_ref Required NV
metadata Optional {key: value}, 1 < len(key) < 255, len(value) < 255"
ServersController update() name Optional str 1 - 255
access_ip_v4 Optional ipv4
access_ip_v6 Optional ipv6
auto_disk_config Optional NV
ServersController _action_reboot() type Required str "HARD" or "SOFT", not allowed lower case.
ServersController _action_resize() flavor_ref Required NV validated with the existing flavors.
ServersController _action_rebuild() image_ref Required str uuid or ".*/<uuid>"
admin_pass Optional NV
name Optional str 1 - 255
access_ip_v4 Optional ipv4
access_ip_v6 Optional ipv6
auto_disk_config Optional NV
personality Optional NV
path Required NV
contents Required str base64encoded
ServersController _action_create_image() name Required NV
metadata Optional dict
ServiceController update() host Required NV
binary Required NV
disabled_reason Required if id is disable-log-reason str 1 - 255

Appendix: URL