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....@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 <dtant...@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-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > -- Rackspace Australia __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
