Open Stack

On 06/15/2017 10:01 PM, Matt Riedemann wrote:
> On 6/15/2017 8:43 PM, Alex Xu wrote:
>> We added new decorator 'query_schema' to support validate the query
>> parameters by JSON-Schema.
>>
>> It provides more strict valiadation as below:
>> * set the 'additionalProperties=False' in the schema, it means that
>> reject any invalid query parameters and return HTTPBadRequest 400 to
>> the user.
>> * use the marco function 'single_param' to declare the specific query
>> parameter only support single value. For example, the 'marker'
>> parameters for the pagination actually only one value is the valid. If
>> the user specific multiple values "marker=1&marker=2", the validation
>> will return 400 to the user.
>>
>> Currently there is patch related to this:
>> https://review.openstack.org/#/c/459483/13/nova/api/openstack/compute/schemas/server_migrations.py
>>
>>
>> So my question is:
>> Are we all good with this strict validation in all the future
>> microversion?
>>
>> I didn't remember we explicit agreement this at somewhere, just want
>> to double check this is the direction everybody want to go.
>>
>> Thanks
>> Alex
>>
>>
>> __________________________________________________________________________
>>
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe:
>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
> 
> I think this is fine and makes sense for new microversions. The spec for
> consistent query parameter validation does talk about it a bit:
> 
> https://specs.openstack.org/openstack/nova-specs/specs/ocata/implemented/consistent-query-parameters-validation.html#proposed-change
> 
> 
> "The behaviour additionalProperties as below:
> 
> * When the value of additionalProperties is True means the extra query
> parameters are allowed. But those extra query parameters will be
> stripped out.
> * When the value of additionalProperties is False means the extra query
> aren’t allowed.
> 
> The value of additionalProperties will be True until we decide to
> restrict the parameters in the future, and it will be changed with new
> microversion."
> 
> I don't see a point in allowing someone to specify a query parameter
> multiple times if we only pick the first one from the list and use that.

Agreed. The point of doing strict validation and returning a 400 is to
help the user eliminate bugs in their program. If they specified marker
twice either they thought it did something, or they made a mistake. Both
are wrong. When we are silent on that front it means they may not be
getting the behavior they were expecting, which hurts their experience
with the API.

	-Sean

-- 
Sean Dague
http://dague.net