Open Stack

2015-09-08 19:45 GMT+09:00 Sean Dague <sean at dague.net>:
> On 09/06/2015 11:15 PM, GHANSHYAM MANN wrote:
>> Hi All,
>>
>> As we all knows, api-paste.ini default setting for /v2 was changed to
>> run those on v2.1 (v2.0 on v2.1) which is really great think for easy
>> code maintenance in future (removal of v2 code).
>>
>> To keep "v2.0 on v2.1" fully compatible with "v2.0 on v2.0", some bugs
>> were found[1] and fixed. But I think we should fix those only for v2
>> compatible mode not for v2.1.
>>
>> For example bug#1491325, 'device' on volume attachment Request is
>> optional param[2] (which does not mean 'null-able' is allowed) and
>> v2.1 used to detect and error on usage of 'device' as "None". But as
>> it was used as 'None' by many /v2 users and not to break those, we
>> should allow 'None' on v2 compatible mode also. But we should not
>> allow the same for v2.1.
>>
>> IMO v2.1 strong input validation feature (which helps to make API
>> usage in correct manner) should not be changed, and for v2 compatible
>> mode we should have another solution without affecting v2.1 behavior
>> may be having different schema for v2 compatible mode and do the
>> necessary fixes there.
>>
>> Trying to know other's opinion on this or something I missed during
>> any discussion.
>>
>> [1]: https://bugs.launchpad.net/python-novaclient/+bug/1491325
>>       https://bugs.launchpad.net/nova/+bug/1491511
>>
>> [2]: http://developer.openstack.org/api-ref-compute-v2.1.html#attachVolume
>
> A lot of these issue need to be a case by case determination.
>
> In this particular case, we had the Documetation, the nova code, the
> clients, and the future.
>
> The documentation: device is optional. That means it should be a string
> or not there at all. The schema was set to enforce this on v2.1
>
> The nova code: device = None was accepted previously, because device is
> a mandatory parameter all the way down the call stack. 2 layers in we
> default it to None if it wasn't specified.
>
> The clients: both python-novaclient and ruby fog sent device=None in the
> common case. While only 2 data points, this does demonstrate this is
> more wide spread than just our buggy code.
>
> The future: it turns out we really can't honor this parameter in most
> cases anyway, and passing it just means causing bugs. This is an
> artifact of the EC2 API that only works on specific (and possibly
> forked) versions of Xen that Amazon runs. Most hypervisor / guest
> relationships don't allow this to be set. The long term direction is
> going to be removing it from our API.
>
> Given that it seemed fine to relax this across all API. We screwed up
> and didn't test this case correctly, and long term we're going to dump
> it. So we don't want to honor 3 different versions of this API,
> especially as no one seems written to work against the documentation,
> but were written against the code in question. If they write to the
> docs, they'll be fine. But the clients that are out in the wild will be
> fine as well.

I think the case by case determination is fine, but current change
progress of relaxing validation seems wrong.
In Kilo, we required nova-specs for relaxing v2.1 API validation like
https://review.openstack.org/#/c/126696/
and we had much enough discussion and we built a consensus about that.
But we merged the above patch in just 2 working days without any
nova-spec even if we didn't have a consensus about that v2.1
validation change requires microversion bump or not.

If we really need to relax validation thing for v2.0 compatible API,
please consider separating v2.0 API schema from v2.1 API schema.
I have one idea about that like https://review.openstack.org/#/c/221129/

We worked for strict and consistent validation way on v2.1 API over 2
years, and I don't want to make it loose without enough thinking.

Thanks
Ken Ohmichi