Open Stack

On 02/19/2016 11:24 AM, Sean Dague wrote:
> On 02/19/2016 11:15 AM, Ben Swartzlander wrote:
>> On 02/19/2016 10:57 AM, Sean Dague wrote:
>>> On 02/18/2016 10:38 AM, D'Angelo, Scott wrote:
>>>> Cinder team is proposing to add support for API microversions [1]. It
>>>> came up at our mid-cycle that we should add a new /v3 endpoint [2].
>>>> Discussions on IRC have raised questions about this [3]
>>>>
>>>> Please weigh in on the design decision to add a new /v3 endpoint for
>>>> Cinder for clients to use when they wish to have api-microversions.
>>>>
>>>> PRO add new /v3 endpoint: A client should not ask for new-behaviour
>>>> against old /v2 endpoint, because that might hit an old
>>>> pre-microversion (i.e. Liberty) server, and that server might carry
>>>> on with old behaviour. The client would not know this without
>>>> checking, and so strange things happen silently.
>>>> It is possible for client to check the response from the server, but
>>>> his requires an extra round trip.
>>>> It is possible to implement some type of caching of supported
>>>> (micro-)version, but not all clients will do this.
>>>> Basic argument is that  continuing to use /v2 endpoint either
>>>> requires an extra trip for each request (absent caching) meaning
>>>> performance slow-down, or possibility of unnoticed errors.
>>>>
>>>> CON add new endpoint:
>>>> Downstream cost of changing endpoints is large. It took ~3 years to
>>>> move from /v1 -> /v2 and we will have to support the deprecated /v2
>>>> endpoint forever.
>>>> If we add microversions with /v2 endpoint, old scripts will keep
>>>> working on /v2 and they will continue to work.
>>>> We would assume that people who choose to use microversions will
>>>> check that the server supports it.
>>>
>>> The concern as I understand it is that by extending the v2 API with
>>> microversions the following failure scenario exists
>>>
>>> If:
>>>
>>> 1) a client already is using the /v2 API
>>> 2) a client opt's into using microversions on /v2
>>> 3) that client issues a request on a Cinder API v2 endpoint without
>>> microversion support
>>> 4) that client fails check if micoversions are supported by a GET of /v2
>>> or by checking the return of the OpenStack-API-Version return header
>>
>> It disagree that this (step 4) is a failure. Clients should not have to
>> do a check at all. The client should tell the server what it wants to do
>> (send the request and version) and the server should do exactly that if
>> and only if it can. Any requirement that the client check the server's
>> version is a massive violation of good API design and will cause either
>> performance problems or correctness problems or both.
>
> That is a fair concern. However the Cinder API today doesn't do strict
> input validation (in my understanding). Which means it's never given
> users that guaruntee. Adding ?foo=bar to random resources, or extra
> headers, it likely to just get silently dropped.
>
> Strict input validation is a good thing to do, and would make a very
> sensible initial microversion to get onto that path.
>
> So this isn't really worse than the current situation. And the upside is
> easier adoption.

I'm not okay with shipping a broken design just because adoption will be 
easier.

I agree the current situation could be better, but let's not let a bad 
status quo give us an excuse to build a bad future. I'm also in favor of 
input validation. Arguably it was harder to do in the past because we 
didn't have a clear versioning mechanism and we needed to to give 
ourselves a way to make backwards-compatible changes to APIs. With a 
proper versioning scheme, input validation is very practical, and the 
only hurdle to getting it implemented is the amount of work.

-Ben


> 	-Sean
>