Quantum create port API accepts invalid state value

Bug #919265 reported by Unmesh Gurjar on 2012-01-20
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Undecided
Madhav Puri
quantum (Ubuntu)
Undecided
Unassigned
Nominated for Precise by Yolanda Robla

Bug Description

The Quantum Create port API should accept only 2 states (ACTIVE and DOWN). However, when the create port API is provided with an invalid state, it creates a port and returns an Accepted(202) response. The API should return a BadRequest(400) response instead.

Madhav Puri (madhav-puri) wrote :

Hi,

I tried to debug the behavior seen here. And based on my understanding, I see two issues and hence two potential places of fixing this:

1. We are missing validation of allowed config-port-states passed to the Quantum API from a quantum-client, as seen in code snippet below.

++++++
class Controller(common.QuantumController):
    """ Port API controller for Quantum API """

...
...
   @common.APIFaultWrapper([exception.NetworkNotFound,
                             exception.StateInvalid])
    def create(self, request, tenant_id, network_id, body=None):
        """ Creates a new port for a given network
            The request body is optional for a port object.

        """
        body = self._prepare_request_body(body, self._port_ops_param_list)
        port = self._plugin.create_port(tenant_id,
                                        network_id, body['port']['state'], <<< no state validation don at API level
                                        **body)
        builder = ports_view.get_view_builder(request, self.version)
        result = builder.build(port)['port']
        return dict(port=result)

...
...
++++++

I believe we need a fix at this level if we anticipate plugins to only expect certain API prescribed states. This become a bit restrictive wrt plugin and needs more intelligence in API layer. But at same time it presents a uniform API to user irrespective of underlying plugin.

2. Looking at plugin-code (cisco and ovs) looks like the behavior might differ at plugin level as well. Like for Cisco nexus-plugin create-port API seems a no-op (this is just based on going through some code and I might be wrong). For the ovs-plugin (class OVSQuantumPlugin), if a state is passed it is used as-is otherwise 'DOWN' state is used, as seen in snippet below.

+++++
class OVSQuantumPlugin(QuantumPluginBase):
...
...
def port_create(net_id, state=None, op_status=OperationalStatus.UNKNOWN):
    # confirm network exists
    network_get(net_id)

    session = get_session()
    with session.begin():
        port = models.Port(net_id, op_status)
        port['state'] = state or 'DOWN' <<< no state validation done at plugin level here.
        session.add(port)
        session.flush()
        return port
...
...

+++++

We might need a fix at plugin level if we are to allow plugins to accept potentially different states or handle create-port request differently. In this case API just acts as a layer that passes this info to plugin and all intelligence shall reside in plugin.

Let me know in case I am missing something and took off a totally wrong path :). Or else if there are/were any relevant discussions that might have taken place on desired behavior.

Thanks.

dan wendlandt (danwent) wrote :

Yeah, the right thing would probably be to take the logic from the port_update call in db/api.py and use it in port_create:

def port_update(port_id, net_id, **kwargs):
    # confirm network exists
    network_get(net_id)
    port = port_get(port_id, net_id)
    session = get_session()
    for key in kwargs.keys():
        if key == "state":
            if kwargs[key] not in ('ACTIVE', 'DOWN'):
                raise q_exc.StateInvalid(port_state=kwargs[key])
        port[key] = kwargs[key]
    session.merge(port)
    session.flush()
    return port

Changed in quantum:
assignee: nobody → Madhav Puri (madhav-puri)

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

Changed in quantum:
status: New → In Progress

Reviewed: https://review.openstack.org/4495
Committed: http://github.com/openstack/quantum/commit/b9805bd234edc0076ff69fafc2ab020562863b58
Submitter: Jenkins
Branch: master

commit b9805bd234edc0076ff69fafc2ab020562863b58
Author: Madhav Puri <email address hidden>
Date: Thu Feb 23 22:44:07 2012 -0800

    Return appropriate error for invalid-port state in create port API.

    Fixes ovs-plugin to return appropriate error code when create port API is passed a port state value other than ACTIVE or DOWN. Fixes bug 919265.

    Also added unit-test to test the behavior and verified it using ovs-plugin with devstack.

    Change-Id: Ibd4e7bfdf4483c7ad1ef4ca70a336cb164493ae1

Changed in quantum:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2012-02-29
Changed in quantum:
milestone: none → essex-4
status: Fix Committed → Fix Released

I will look into this.
I actually think we have a unit tesys for checking the state. However there is a chance the check might be enforced only for the update operation.

Inviato da Samsung Mobile

Unmesh Gurjar <email address hidden> ha scritto:

Public bug reported:

The Quantum Create port API should accept only 2 states (ACTIVE and
DOWN). However, when the create port API is provided with an invalid
state, it creates a port and returns an Accepted(202) response. The API
should return a BadRequest(400) response instead.

** Affects: quantum
     Importance: Undecided
         Status: New

--
You received this bug notification because you are a member of Netstack
Core Developers, which is subscribed to quantum.
https://bugs.launchpad.net/bugs/919265

Title:
  Quantum create port API accepts invalid state value

Status in OpenStack Quantum (virtual network service):
  New

Bug description:
  The Quantum Create port API should accept only 2 states (ACTIVE and
  DOWN). However, when the create port API is provided with an invalid
  state, it creates a port and returns an Accepted(202) response. The
  API should return a BadRequest(400) response instead.

To manage notifications about this bug go to:
https://bugs.launchpad.net/quantum/+bug/919265/+subscriptions

this bug has actually been fixed by madhav:
https://review.openstack.org/#change,4495

On Tue, Mar 6, 2012 at 8:33 AM, Salvatore Orlando <<email address hidden>
> wrote:

> I will look into this.
> I actually think we have a unit tesys for checking the state. However
> there is a chance the check might be enforced only for the update operation.
>
>
> Inviato da Samsung Mobile
>
>
> Unmesh Gurjar <email address hidden> ha scritto:
>
>
> Public bug reported:
>
> The Quantum Create port API should accept only 2 states (ACTIVE and
> DOWN). However, when the create port API is provided with an invalid
> state, it creates a port and returns an Accepted(202) response. The API
> should return a BadRequest(400) response instead.
>
> ** Affects: quantum
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are a member of Netstack
> Core Developers, which is subscribed to quantum.
> https://bugs.launchpad.net/bugs/919265
>
> Title:
> Quantum create port API accepts invalid state value
>
> Status in OpenStack Quantum (virtual network service):
> New
>
> Bug description:
> The Quantum Create port API should accept only 2 states (ACTIVE and
> DOWN). However, when the create port API is provided with an invalid
> state, it creates a port and returns an Accepted(202) response. The
> API should return a BadRequest(400) response instead.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/quantum/+bug/919265/+subscriptions
>
> --
> You received this bug notification because you are a member of Netstack
> Core Developers, which is subscribed to quantum.
> https://bugs.launchpad.net/bugs/919265
>
> Title:
> Quantum create port API accepts invalid state value
>
> Status in OpenStack Quantum (virtual network service):
> Fix Released
>
> Bug description:
> The Quantum Create port API should accept only 2 states (ACTIVE and
> DOWN). However, when the create port API is provided with an invalid
> state, it creates a port and returns an Accepted(202) response. The
> API should return a BadRequest(400) response instead.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/quantum/+bug/919265/+subscriptions
>

--
~~~~~~~~~~~~~~~~~~~~~~~~~~~
Dan Wendlandt
Nicira Networks: www.nicira.com
twitter: danwendlandt
~~~~~~~~~~~~~~~~~~~~~~~~~~~

Thierry Carrez (ttx) on 2012-04-05
Changed in quantum:
milestone: essex-4 → 2012.1
Changed in quantum (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers