400 should be returned when invalid attributes are passed to Quantum API

[Jason]

Issues like the one reported below apply to all Quantum entities, core and extended.
This should be fixed soon.

------------

When testing the updating the router with invalidate values, the following case returns 200,

    def test_router_update_invalid_value(self):
        fmt = 'json'
        data = {'router': {'tenant_id': _uuid(),
                           'name': 'router1',
                           'admin_state_up': True}}
        req = self.new_create_request('routers', data, fmt)
        res = req.get_response(self.ext_api)
        router = self.deserialize(fmt, res)

        res = self._update('routers', router['router']['id'],
                           {'router': {}},
                           expected_code=exc.HTTPBadRequest.code)

        res = self._update('routers', router['router']['id'],
                           {'router': {'gibberish': 'anything'}},
                           expected_code=exc.HTTPBadRequest.code)

Thanks,

Jason

[dan wendlandt (danwent)]

to be clear, the example here is that "tenant_id" is not a valid value to update? I suspect it would also be the case if you just put a gibberish field into the router dictionary?

I think this is related to the fact that due to the behavior of the old extension mechanisms, the quantum API did not produce an error when someone was passing in an invalid key. Instead, it would just ignore it. With our new extension mechanism, I think we could do away with this. Adding Salvatore to see what he thinks.

[Jason (zzs)]

Hi Dan,

Thanks for update!

Sorry for confusing, I updated the title and description.

[OpenStack Infra (hudson-openstack)]

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

[dan wendlandt (danwent)]

since this change affects API behavior, I'd like to get it in for G-3, rather than waiting until RC1. That way, it gets testing before the RC.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/21849
Committed: http://github.com/openstack/quantum/commit/61bdda62e8950a9747073e2be7b81cdea0af2f25
Submitter: Jenkins
Branch: master

commit 61bdda62e8950a9747073e2be7b81cdea0af2f25
Author: Jason Zhang <email address hidden>
Date: Tue Feb 12 18:40:12 2013 -0800

    Raising error if invalid attribute passed in.

    400 will be returned when invalid attributes
    are passed into Quantum API.

    Fixed the some test cases failed since the
    enforcement of invalid attribute checking

    Fixes: bug #1076179

    Change-Id: I4e9e2891c444f9dcd051f7b325d3c9403b28db86

[Ronak Shah (ronak-malav-shah)]

Hi Jason and Salvatore,

As I understand, this bug was meant to fix a problem of "incorrect" attribute being passed in CRUD operation.
But, it ended up doing a fix for "unrecognized" attribute.

They are not same.

CLI was giving one a nice ability to pass extra attributes which a given plugin might be interested in understanding without modifying a core quantumclient API.

For example,
quantum net-create net1 --router rtr1.
An easier way to stitch all the subnets of net1 to rtr1. (We have an alterantive to this example in quantum - router-interface-add which indirectly does the same.)

There are many more good cases within a given plugin.

Now, one can make a counter argument that Horizon will anyways not support this but I would counter that again making it a horizon limitations.

Shouldnt we rethink the fix such that we really fix the "incorrect" attribute rather than "unrecognized".

[Salvatore Orlando (salvatore-orlando)]

Hi Ronak,

unfortunately this fix prevents exactly what you want to do.
We have a good reason for that, and that if one does not format correctly a request, the API succeeds whereas it should fail.
For instance in a PUT /networks:

{ 'network': {'admin_stute_up': False}}

will succeed, but won't change anything because there a typo in the attribute.
With this fix they will return 400.

This is also preventing, as you say, plugins to handle arbitrary extra attributes.
And this is intentional too.
If a plugin allows extra attributes, this should be documented via an API extension. I think if you add an API extension for your attributes then it should work again. Extension are located by default in /quantum/extension but you can add your own path tweaking api_extension_path in quantum.conf

Anyway, if you feel the fix should be reverted, please file a new bug.
In my opinion any fix proposed should at least preserve the behaviour that if an argument is mystyped a 400 should be returned.