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

Bug #1076179 reported by Jason
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
Fix Released
High
Jason

Bug Description

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

Tags: api
Jason (zzs)
description: updated
Jason (zzs)
Changed in quantum:
assignee: nobody → Jason Zhang (bearovercloud)
Revision history for this message
dan wendlandt (danwent) wrote :

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.

Changed in quantum:
importance: Undecided → Low
importance: Low → Undecided
tags: added: api
dan wendlandt (danwent)
Changed in quantum:
status: New → Incomplete
Jason (zzs)
summary: - 400 should be returned when updating the router with dummy router vaule
- set
+ 400 should be returned when updating the router with invalidate Key
Jason (zzs)
description: updated
Revision history for this message
Jason (zzs) wrote : Re: 400 should be returned when updating the router with invalid Key

Hi Dan,

Thanks for update!

Sorry for confusing, I updated the title and description.

summary: - 400 should be returned when updating the router with invalidate Key
+ 400 should be returned when updating the router with invalid Key
summary: - 400 should be returned when updating the router with invalid Key
+ 400 should be returned when invalid attributes are passed to Quantum API
Changed in quantum:
importance: Undecided → High
milestone: none → grizzly-3
description: updated
Changed in quantum:
assignee: Jason Zhang (zzs) → Salvatore Orlando (salvatore-orlando)
Jason (zzs)
Changed in quantum:
status: Incomplete → In Progress
assignee: Salvatore Orlando (salvatore-orlando) → Jason Zhang (zzs)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to quantum (master)

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

Revision history for this message
dan wendlandt (danwent) wrote :

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.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to quantum (master)

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

Changed in quantum:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in quantum:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in quantum:
milestone: grizzly-3 → 2013.1
Revision history for this message
Ronak Shah (ronak-malav-shah) wrote :

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".

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

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.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.