Open Stack

As an aside, do we think that exposing the exact version of a server
process is safe from a security perspective?

Michael

On Sat, Jun 20, 2015 at 10:14 AM, Devananda van der Veen
<devananda.vdv at gmail.com> wrote:
> Almost all of our discussions so far on this topic have left something out,
> which Monty pointed out to me last week. I'm following up now because
> E_TRAVEL...
>
> tldr;
> What we're versioning here are API's, not packages. It's not a question of
> numbering and dependency ordering, but of communicating support[ed|able]
> interfaces between running services. Libtool is more relevant than semver.
>
> http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
>
> Right now we lack a means to express that the API response is
> "compatible-with" a particular previously-released version of the API. If we
> had that, instead of "current-version", I believe we would have much happier
> users (and a happier Dmitry and jroll).
>
>
> Long version...
> Every HTTP response from Ironic today includes three headers: min, max, and
> version. The service can present an older API version, as long as it is
> greater-than-or-equal-to the minimum supported version, even if that version
> is incompatible with the maximum supported version.  It does this by
> rewriting responses to match what was expected in the requested (older)
> version.
>
> When the newer version is identical *for all interfaces present* in the
> older version, this can be called compatible. Dmitry's point is that we
> don't need to hide newer interfaces from users who request an older API
> version, because the client won't know or care about things that weren't in
> the version it requested.
>
> However, we *do* need to signal their presence, and we don't have a good
> means for that right now. We also need to signal to the client that the
> response given is "compatible with" a certain "age" of API, even if it's not
> exactly the same. And we don't have any means for that, either.
>
> Time for an example....
>
> Let's say that an incompatible change was made in v1.3. Let's also say that
> a change was made in v1.5 that added a new endpoint. Today, this is what the
> response headers would look like when calling a server running v1.5.
>
> a) client requests v1.2, receives headers (min: 1.1, max: 1.5, current: 1.2)
> b) client requests v1.4, receives headers (min: 1.1, max: 1.5, current 1.4)
> c) client requests v1.5, receives headers (min: 1.1, max: 1.5, current: 1.5)
>
> What we have implemented today is that in case (b), the service will *hide*
> any changes that we introduced in v1.5. But those changes did not affect any
> functionality of the v1.4 API, so Dmitry objects to this. So do I.
>
> The issue at hand is the response in case (b) ... though after spending the
> last several months working on api versioning, I actually think all of those
> are poor responses.
>
> What I believe we should have:
> a) client requests v1.2, receives headers (min: 1.1, max: 1.5,
> compatible-with: 1.1)
> b) client requests v1.4, receives headers  (min: 1.1, max: 1.5,
> compatible-with: 1.3)
> b) client requests v1.5, receives headers  (min: 1.1, max: 1.5,
> compatible-with: 1.3)
>
> Yes -- (b) and (c) are identical responses.
>
> Discuss.
>
> -Devananda
>
>
> On Tue, Jun 16, 2015 at 7:13 AM Dmitry Tantsur <dtantsur at redhat.com> wrote:
>>
>> On 06/16/2015 03:47 PM, Jim Rollenhagen wrote:
>> > On Tue, Jun 16, 2015 at 08:56:37AM +0200, Dmitry Tantsur wrote:
>> >> On 06/04/2015 08:58 AM, Xu, Hejie wrote:
>> >>> Hi, guys,
>> >>> I’m working on adding Microversion into the API-WG’s guideline which
>> >>> make sure we have consistent Microversion behavior in the API for
>> >>> user.
>> >>> The Nova and Ironic already have Microversion implementation, and as I
>> >>> know Magnum _https://review.openstack.org/#/c/184975/_ is going to
>> >>> implement Microversion also.
>> >>> Hope all the projects which support( or plan to) Microversion can join
>> >>> the review of guideline.
>> >>> The Mircoversion specification(this almost copy from nova-specs):
>> >>> _https://review.openstack.org/#/c/187112_
>> >>> And another guideline for when we should bump Mircoversion
>> >>> _https://review.openstack.org/#/c/187896/_
>> >>> As I know, there already have a little different between Nova and
>> >>> Ironic’s implementation. Ironic return min/max version when the
>> >>> requested
>> >>> version doesn’t support in server by http-headers. There isn’t such
>> >>> thing in nova. But that is something for version negotiation we need
>> >>> for
>> >>> nova also.
>> >>> Sean have pointed out we should use response body instead of http
>> >>> headers, the body can includes error message. Really hope ironic team
>> >>> can take a
>> >>> look at if you guys have compelling reason for using http headers.
>> >>> And if we think return body instead of http headers, we probably need
>> >>> think about back-compatible also. Because Microversion itself isn’t
>> >>> versioned.
>> >>> So I think we should keep those header for a while, does make sense?
>> >>> Hope we have good guideline for Microversion, because we only can
>> >>> change
>> >>> Mircoversion itself by back-compatible way.
>> >>> Thanks
>> >>> Alex Xu
>> >>
>> >> Hi all!
>> >>
>> >> I'd like to try put in feedback based on living with microversions in
>> >> Kilo
>> >> release of Ironic.
>> >
>> > And here's my take, based on my experiences. Keep in mind I'm a core
>> > reviewer, a developer, and an operator of Ironic.
>>
>> Thanks Jim, much appreciated!
>>
>> >
>> >  From an ops perspective, our team has built our fair share of tooling
>> > to
>> > help us run Ironic. Some of it uses the REST API via python or node.js,
>> > and of course we all use the CLI client often.
>> >
>> > We also continuously deploy Ironic, for full transparency. My experience
>> > is not with how this works every 6 months, but in the day-to-day.
>> >
>> >>
>> >> First of all, after talking to folks off-list, I realized that we all,
>> >> and
>> >> the spec itself, confuse 3 aspects of microversion usage:
>> >>
>> >> 1. protecting from breaking changes.
>> >> This is clearly a big win from user's point of view, and it allowed us
>> >> to
>> >> conduct painful change with renaming an important node state in our
>> >> state
>> >> machine. It will allows us even worse change this cycle: change of the
>> >> default state.
>> >>
>> >
>> > +1. Good stuff. My tooling doesn't break when I upgrade. Yay.
>> >
>> >> 2. API discoverability.
>> >> While I believe that there maybe be better implementation of this idea,
>> >> I
>> >> think I got it now. People want services to report API versions they
>> >> support. People want to be able to request a specific version, and fail
>> >> early if it is not present. Also +1 from me.
>> >>
>> >
>> > I don't tend to personally do this. I usually am aware of what version
>> > of Ironic I'm running against. However I see how this could be useful
>> > for other folks.
>> >
>> > I do, however, use the versions to say, "Oh, I can now request 1.5 which
>> > has logical names! That's useful, let's set those to the names in our
>> > CMDB." Now my tooling that interacts with the CMDB and Ironic can look
>> > at the version and decide to use node.name instead of the old hack we
>> > used to use.
>> >
>> >> 3. hiding new features from older clients
>> >> This is not directly stated by the spec, but many people imply it, and
>> >> Nova
>> >> and Ironic did it in Kilo. I want us to be clear: it is not the same as
>> >> #2.
>> >> You can report versions, but still allow new features to be used.
>> >>
>> >
>> > This is still totally useful. If you know what version you are running
>> > against, you know exactly what features are available.
>>
>> "You know" is about #2 - that's where confusion is :)
>> so if you know, that moving to inspection state is disallowed for your
>> tooling (but not for the whole system!), what does it give you?
>>
>>
>>
>> >
>> > I think the disconnect here is that we don't expect users (whether those
>> > are people or computers) to explicitly request a version. We need to
>> > message better that if you are using Ironic or building a tool against
>> > Ironic's API, you should be pinning the version. We also need to take
>> > this comment block[0] and put it in our docs, so users know what each
>> > version does.
>> >
>> > Knowing that I get feature X when I upgrade to version Y is useful.
>> >
>> >> It is this particular thing that gets -2 from me, after I've seen how
>> >> it
>> >> worked in practice, and that's why.
>> >>
>> >> First of all, I don't believe anyone needs it. Seriously, I can't
>> >> imagine a
>> >> user asking "please prevent me from using non-breaking changes". And
>> >> attempt
>> >> to implement it was IMO a big failure for the following reasons:
>> >>
>> >> a) It's hard to do. Even we, the core team, got confused, and for
>> >> non-core
>> >> people it took several iteration to do right. It's a big load on both
>> >> developers and reviewers.
>> >>
>> >
>> > I do agree with this. It's been painful. However, I think we're mostly
>> > past that pain at this point. Does this patch[1] look like developer
>> > pain?
>>
>> It's not that painful to write. Now. When we have 10-20 version, it
>> probably will :)
>> anyway, it's hard to explain newcomers how to do it, and it's hard to
>> review the result. we failed at it, e.g. with error codes.
>>
>> >
>> >> b) It's incomplete (at least for Ironic). We have several API-exposed
>> >> things
>> >> that are just impossible to hide. Good example are node states: if node
>> >> is
>> >> in a new state, we can't but expose it to older tooling. Our free-form
>> >> JSON
>> >> fields properties, instance_info, driver_info and driver_internal_info
>> >> are
>> >> examples as well. It's useless to speak about API contract, while we
>> >> have
>> >> those.
>> >>
>> >
>> > I somewhat agree here.
>> >
>> > With node states, there are cases where we were able to hide it
>> > (NOSTATE -> AVAILABLE), and cases where we were not (adding MANAGEABLE).
>> > However, this list of states is (AIUI) not part of the API contract;
>> > rather the verbs available to move between states are.
>>
>> What's the point in contract, if there are things not covered by it that
>> drastically change the system behavior?
>>
>> >
>> > As far as JSON fields, we've never had a contract around what keys are
>> > available. Only the semantics of working with those fields, and which
>> > fields exist.
>>
>> ditto as above: you can request new features by modifying driver_info.
>>
>> >
>> >> c) It gives additional push back to making (required) breaking changes.
>> >> We
>> >> already got suggestions to have ONE MORE feature gating for breaking
>> >> changes. Reason: people will need to increase microversions to get
>> >> features,
>> >> and your breaking change will prevent it.
>> >>
>> >
>> > This is just silly. If 1.10 breaks a user, and the user wants 1.11,
>> > they'll need to fix that breakage.
>>
>> ++ but not everyone agreed on the summit, when I was talking about
>> ENROLL state
>>
>> >
>> >> d) It requires a hard compromise on the CLI tool. You either default it
>> >> to
>> >> 1.0 forever, and force all the people to get used to figuring out
>> >> version
>> >> numbers and using `ironic --ironic-api-version x.y` every time
>> >> (terrible
>> >> user experience), or you default it to some known good version, bumping
>> >> it
>> >> from time to time. This, in turn, has 2 more serious problems:
>> >>
>> >
>> > I disagree that pinning a version all the time is a terrible experience.
>> > We already require a number of options for authentication (OS_USERNAME,
>> > OS_PASSWORD, etc etc). How many folks do you think type these in every
>> > time? Solution is simple: add IRONIC_API_VERSION to whatever exports the
>> > other environment variables.
>>
>> It's not that bad, especially if devstack/tripleo will provide some
>> reasonable default for you.
>>
>> I remember, however, Devananda didn't like the idea.
>>
>> And it definitely makes a quick start guide a bit harder to follow. I
>> already imagine how many people will forget about this pinning (either
>> to do it, or do update when they need new features).
>>
>> >
>> > The version depends on the environment you are running against - why not
>> > treat it as such?
>> >
>> >> d.1) you start to break people \o/ that's not a theoretical concern:
>> >> our
>> >> downstream tooling did get broken by updating to newer ironicclient
>> >> from git
>> >>
>> >
>> > As I said before, we need to encourage folks to pin client versions if
>> > they don't want to break. I'm probably alone here, but I would even
>> > propose making the version *required*. Force people to think about what
>> > they are doing. If folks are okay with being broken, they can pass
>> > "latest".
>>
>> Could be a good default for devstack btw
>>
>> >
>> >> d.2) you require complex version negotiations on the client side.
>> >> Otherwise
>> >> imaging CLI tool defaulting to 1.6 will issue `node-create` to Ironic
>> >> supporting only 1.5. Guess what? It will fail despite node-create being
>> >> very
>> >> old feature. Again, that's not something theoretical: that's how we
>> >> broke
>> >> TripleO CI.
>> >>
>> >
>> > Again, pin it.
>> >
>> >> e) Every microversion should be fully tested separately. Which ended up
>> >> in
>> >> Ironic having 4 versions 1.2-1.5 that were never ever gate tested. Even
>> >> worse, initially, our gate tested only the oldest version 1.1, but we
>> >> solved
>> >> it (though it took time to realize). The only good thing here is that
>> >> these
>> >> versions 1.2-1.5 were probably never used by anyone.
>> >>
>> >
>> > Hi. I've used some of these. :)
>>
>> You didn't tell me last time we talked :) note, that you didn't use
>> them, unless you explicitly requested, because IIRC we never defaulted
>> our client to any of these. So for most people, even deploying from
>> master, it was 1.1 -> 1.6.
>>
>> >
>> > // jim
>> >
>> > [0]
>> > https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/__init__.py#L59-63
>> > [1]
>> > https://review.openstack.org/#/c/188873/1/ironic/api/controllers/v1/node.py
>> >>
>> >> To sum this long post up, I'm seeing that hiding new features based on
>> >> microversions brings much more problems, than it solves (I'm not aware
>> >> of
>> >> the latter at all). I'm very opposed to continuing doing it in Ironic,
>> >> and
>> >> I'm going to propose patch stopping gating Kilo changes (non-breaking
>> >> obviously).
>> >>
>> >> Hope that helps,
>> >> Dmitry
>> >>
>> >>>
>
>
> __________________________________________________________________________
> 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
>



-- 
Rackspace Australia