Quantum create port API accepts invalid state value

[Unmesh Gurjar]

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)]

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)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Salvatore Orlando (salvatore-orlando)]

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

[dan wendlandt (danwent)]

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
~~~~~~~~~~~~~~~~~~~~~~~~~~~