XML support not required for ironic

[aeva black]

Several folks have pointed out bugs in our handling of XML today, most importantly that our use of JSON PATCH documents is not compatible with XML at all, and would take considerable work to munge incoming XML in a way that we could process.

Given that the TC has suggested that XML support not be required in forth-coming API versions for existing projects (see http://lists.openstack.org/pipermail/openstack-tc/2014-January/000494.html), I think we should just disable XML in our API now and continue to focus on the JSON API.

[Roman Prykhodchenko (romcheg)]

I agree.

[Max Lobur (max-lobur)]

I agree as well

[Lucas Alvares Gomes (lucasagomes)]

+1 I agree that we should disable the XML support in our API

[Haomeng,Wang (whaom)]

I agree, but so far no solution, I try to nput invalid content_types http request such as 'application/abc', and run API, we got error - 2014-01-22 02:15:21.336 6377 ERROR pecan.core [-] Controller 'get_all' defined does not support content_type 'None'. Supported type(s): ['application/json', 'application/xml'] , this message is raised from pecan, debug it, but still not sure how to disable it.

[Ghe Rivero (ghe.rivero)]

What is XML? +1

[Michael Davies (mrda)]

This is non-trivial to a non-wsgi/pecan/wsme expert :)

The actual server appears to be built in ...ironic/cmd/api.py as simple_server.make_server(). This specifies app.VersionSelectorApplication as the request handler. This class is specified in ...ironic/api/app.py, and it appears that you can specify a default_renderer to pecan.make_app() but this only specifies the default renderer, rather than a list of allowed renderers.

In ...pecan/core.py a list of content_types is read from configuration. This is where I'll continue searching to see if we can specify only 'application/json' as the valid content type.

[aeva black (tenbrae)]

I did a little more digging and found an RFC for XML PATCH support, but still haven't found a python library that implements it.
  http://tools.ietf.org/search/rfc5261

Not sure that it is a good idea, but it's an idea -- instead of completely disabling XML, would be to return a 415 error code for an HTTP PATCH request with a content-type of XML. This would meet the requirements of HTTP PATCH support, but would leave XML support severely handicapped.
  https://tools.ietf.org/html/rfc5789#section-2.2
 It's probably better to just remove it (as initially suggested).

[Michael Davies (mrda)]

I'd like to see the content_types that WSME handles be configurable, so that ironic can choose not to support XML. Would WSME be open to supporting this?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/78108

[Michael Davies (mrda)]

I've got stuck, so unassigning

[Ling Gao (linggao)]

I am a little confused here, are we saying that we do not support user querying data using the Ironic urls from the browser? That's a BIG limit of the functions.

[Michael Davies (mrda)]

There's been plenty of discussion surrounding XML support in OpenStack in general. I believe that in Nova generally, Nova APIv3 discussions, and Ironic communities (at least) there's been a generally expressed desire to remove XML support and only support JSON.

The two interfaces have diverged, and we have no meaningful way to generate one from the other that supports our use cases. And it's believed that JSON is the much greater preferred interface by users.

Having this support in WSME allows individual projects to choose to leave it enabled by default, or remove it as required.

[aeva black (tenbrae)]

Our ability to address this in Ironic depends on changes in WSME. The change initially proposed by mrda was stalled until recently; new fixes have been proposed by Ryan and Doug:

https://review.openstack.org/#/c/123520
https://review.openstack.org/#/c/123792

Once those land and are released, Ironic will need to bump the required version of WSME

It is, however, too late in the Juno cycle to get this fix in, so we'll have to address this in Kilo.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/123520
Committed: https://git.openstack.org/cgit/stackforge/wsme/commit/?id=26a6acdadd4873be4b813b3f6ebf134022ca8f73
Submitter: Jenkins
Branch: master

commit 26a6acdadd4873be4b813b3f6ebf134022ca8f73
Author: Ryan Petrello <email address hidden>
Date: Tue Sep 23 13:23:48 2014 -0400

    Add support for manually specifying supported content types in @wsmeexpose.

    Closes-bug: #1271317
    Change-Id: Ia28a912f4444a6ff77b1feaf2ea6440b0c738e86

[OpenStack Infra (hudson-openstack)]

Change abandoned by Michael Davies (<email address hidden>) on branch: master
Review: https://review.openstack.org/78108
Reason: Another patch addressing this has already merged, so abandoning this change. Note that the change in Ironic will need to be on a method-by-method basis as a result of the merged fix, rather than a once-off config option change.

[John Stafford (john-stafford)]

If there is another patch merged that addresses this issue can this bug be closed?

[aeva black (tenbrae)]

John,

No. There were two competing solutions initially, and the "right" one was to enable this in WSME. That landed, but no one has resumed the work in Ironic to use the solution implemented in WSME yet. Ryan's patch describes what we need to do in Ironic to fix this.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/169643
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=f91a1fd6e7bcd66240c0061af9424bb61dd029f2
Submitter: Jenkins
Branch: master

commit f91a1fd6e7bcd66240c0061af9424bb61dd029f2
Author: Michael Davies <email address hidden>
Date: Tue Mar 31 21:33:41 2015 -0700

    Disable XML now that we have WSME/Pecan support

    We've had a desire to disable XML from the Ironic API for some
    time, however to do so needed support in WSME/Pecan. That's now
    been added, so we can implement the Ironic side.

    To do so, we wrap wsmeext.pecan.expose() and replace all the decorators
    with a call to ironic.api.expose.expose().

    Change-Id: I94d0387722225c4356c867f63b6f1a61356f317d
    Closes-bug: 1271317