neutronclient: incorrect treatment of input parameters

[Volodymyr Litovka]

Hi colleagues,

Neutron 10.0.0 (as part Ocata) on Ubuntu 16.04

Everything is ok when I use Neutron CLI to update port parameters, but when I'm doing the same using Heat, it fails.

Heat's template for the object is:

  e-secgroup:
    type: OS::Neutron::SecurityGroup
    properties:
      name: SSH_ICMP
      rules:
        - direction: ingress
        - protocol: tcp
          remote_ip_prefix: 0.0.0.0/0
          port_range_min: 22
          port_range_max: 22
        - protocol: icmp
          remote_ip_prefix: 0.0.0.0/0

  node1-wan:
    type: OS::Neutron::Port
    properties:
      name: jadm-node1-wan
      network: e-net
      port_security_enabled: True
      security_groups: [ default, { get_resource: e-secgroup } ]

Heat makes a call to Neutron using neutronclient and provides input parameters in such way:

updating port with {'allowed_address_pairs': [], 'binding:vnic_type': None, 'device_owner': None, 'mac_address': None, 'security_groups': [u'53ede63e-b08f-4c95-b5fe-29cd21ed442a', u'0a48c45e-5a6d-4b80-8226-08d2e8c5bb00'], 'device_id': None} handle_update /usr/lib/python2.7/dist-packages/heat/engine/resources/openstack/neutron/port.py:520

In return, I get the following error:

2017-04-11 09:51:31.809 14474 DEBUG neutronclient.v2_0.client [req-54a51260-8701-4f94-9141-562443a3ad7e - bush - - -] Error message: {"NeutronError": {"message": "Invalid input for device_owner. Reason: 'None' is not a valid string.", "type": "HTTPBadRequest", "detail": ""}} _handle_fault_response /usr/lib/python2.7/dist-packages/neutronclient/v2_0/client.py:266

If I explicitly set device_owner='' in Heat template, then neutronclient accepts this key-value pair and returns error for the next parameter (e.g. device_id):

2017-04-11 10:45:26.808 14474 DEBUG neutronclient.v2_0.client [req-3f5c1c53-bec4-418f-bc0e-e04b23474c0e - bush - - -] Error message: {"NeutronError": {"message": "Invalid input for device_id. Reason: 'None' is not a valid string.", "type": "HTTPBadRequest", "detail": ""}} _handle_fault_response /usr/lib/python2.7/dist-packages/neutronclient/v2_0/client.py:266

and this affects not only parameters of OS::Neutron::Port object, but OS::Neutron::SecurityGroup's ones as well:

Resource UPDATE failed: BadRequest: resources.e-secgroup: Invalid input for description. Reason: 'None' is not a valid string. Neutron server returns request_ids: ['req-287db7f6-06be-4bc8-a11b-94be203c67da']

So, the problem is general and it seems that something wrong with treatment of 'None' in input parameters: all clients should always treat None values the same as not passing any value, while neutronclient accepts '' and refuses None.

Severity: this bug blocks using Heat.

Thank you.

[Volodymyr Litovka (doka.ua)]

Corresponding bug was previously opened in Heat section - https://bugs.launchpad.net/heat/+bug/1681769 - but according to Steven Hardy's comment, I reopened it here.

[Reedip (reedip-banerjee-deactivatedaccount)]

This is not coming from NeutronClient, but neutron itself and it is as per Neutron's API Definition.
Ports do not accept device_id/device_owner as None [1]
Neither do Security Groups for the fields you have specified [2]
[1]https://github.com/openstack/neutron/blob/master/neutron/api/v2/attributes.py#L120-L129
[2]https://github.com/openstack/neutron/blob/master/neutron/extensions/securitygroup.py#L210-L227

[Volodymyr Litovka (doka.ua)]

Hi Reedip,

from both snippets you've provided it's clear that these fields can be empty. Whether 'None' is nothing (empty) as well?

Heat team says the following: "IMO this is a neutronclient bug - [...] all clients should always treat None values the same as not passing any value." and they pass None to Neutron, using neutronclient in the middle.

So where is problem located? Who one - Heat, Neutron or neutronclient - don't conform to openstack rules of passing/treating parameters?

[Trevor McCasland (twm2016)]

The bug is valid, it just needs to be discussed where the fix should be. (Heat, neutron or neutronclient).

[Volodymyr Litovka (doka.ua)]

My $.05 ... as a man who discovered this problem, naturally, I'm intersted in the fastest solution regardless of where it will be fixed :) but on the other side, it should be solved in the most right way.

I think, all syntax- and security related checks must be done on the frontend, which, in our situations, is neutronclient. There can be lot of clients (Heat is particular one) incl 3rd party ones and it's unsafe to put on their correctness, while Neutron itself is core and need to do core tasks. Also, it will be easier to maintain integrity if all safety and cleanup tasks will be done in single place.

Thank you.

[Kevin Benton (kevinbenton)]

Is this a regression on the Neutron side? (i.e. Did we used to allow this and something in the client/server changed? Or did heat change its usage of the client?)

[Volodymyr Litovka (doka.ua)]

There was no this problem in Newton. It appeared in Ocata.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/457519

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/ocata
Review: https://review.openstack.org/457521

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/newton
Review: https://review.openstack.org/457522

[Kevin Benton (kevinbenton)]

I confirmed with the patches above that our API behavior has been consistent on the server side and we have always rejected None as an input for device_id.

This is not something we can fix in the Neutron server because it's an API change and Heat is using the neutron client to explicitly ask for None as the value for a field. Explicitly requesting None is not the same thing as leaving a field out of the request to allow the server to use the default value.

Changing the neutron client would block people from actually requesting a value of None in a field on the server, so that's a non-starter as well.

For example, if you leave 'gateway_ip' out of a subnet request, it defaults to the first IP in the CIDR. However, if you explicitly set it to None, it means you don't want a gateway_ip and we honor that.

In summary, None is a value in Neutron, if you don't want it, you can't request it.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/457537

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/457519
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=84d2d027625e15fdd5cc887c2c8b810e994b7a30
Submitter: Jenkins
Branch: master

commit 84d2d027625e15fdd5cc887c2c8b810e994b7a30
Author: Kevin Benton <email address hidden>
Date: Tue Apr 18 01:40:38 2017 -0700

    Ensure behavior of None for device_id

    A simple unit test to ensure that we have had the same behavior
    across releases WRT to explicitly passing None for device_id.

    Change-Id: I153f3e1b4258f33c5fa3f16cc30a411052670b30
    Related-Bug: #1681784

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/457521
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=9d5f39b64d949822ee9169b7a730fc2fd80dbcac
Submitter: Jenkins
Branch: stable/ocata

commit 9d5f39b64d949822ee9169b7a730fc2fd80dbcac
Author: Kevin Benton <email address hidden>
Date: Tue Apr 18 01:40:38 2017 -0700

    Ensure behavior of None for device_id

    A simple unit test to ensure that we have had the same behavior
    across releases WRT to explicitly passing None for device_id.

    Change-Id: I153f3e1b4258f33c5fa3f16cc30a411052670b30
    Related-Bug: #1681784
    (cherry picked from commit 84d2d027625e15fdd5cc887c2c8b810e994b7a30)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/457522
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=fe7e49a450f69eb6487ed3f270d45599ea78a086
Submitter: Jenkins
Branch: stable/newton

commit fe7e49a450f69eb6487ed3f270d45599ea78a086
Author: Kevin Benton <email address hidden>
Date: Tue Apr 18 01:40:38 2017 -0700

    Ensure behavior of None for device_id

    A simple unit test to ensure that we have had the same behavior
    across releases WRT to explicitly passing None for device_id.

    Change-Id: I153f3e1b4258f33c5fa3f16cc30a411052670b30
    Related-Bug: #1681784
    (cherry picked from commit 84d2d027625e15fdd5cc887c2c8b810e994b7a30)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/457537
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=4d8685da8050df79d9193f91cab572cfc6d67a47
Submitter: Jenkins
Branch: stable/mitaka

commit 4d8685da8050df79d9193f91cab572cfc6d67a47
Author: Kevin Benton <email address hidden>
Date: Tue Apr 18 01:40:38 2017 -0700

    Ensure behavior of None for device_id

    A simple unit test to ensure that we have had the same behavior
    across releases WRT to explicitly passing None for device_id.

    Change-Id: I153f3e1b4258f33c5fa3f16cc30a411052670b30
    Related-Bug: #1681784
    (cherry picked from commit 84d2d027625e15fdd5cc887c2c8b810e994b7a30)