create_port is failing for non admin context as Nova neutronclient interactions are not using admin context consistently

[Esha Seth]

There have been several changes in community in Newton on nova and neutron client interactions. The interactions are not consistently using admin context and credentials/token of nova user always instead sometimes using the logged in user credentials. Due to this the create_port is not getting allowed for non-admin users.

https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L117-L137

# NOTE(dprince): In the case where no auth_token is present we allow use of # neutron admin tenant credentials if it is an admin context. This is to # support some services (metadata API) where an admin context is used # without an auth token

[Divya K Konoor (dikonoor)]

I think the description is slightly wrong.

I took a look at neutron policy file and create_port can be created by any user by default.
https://github.com/openstack/neutron/blob/master/etc/policy.json#L72 and that must be the reason the code was written with a neutronclient without admin context. If the default policy.json is used (which allows all users to create port), everything is good . However, if we customize the policy.json to something like below:

"create_port": "role:member or role:im-custom-role"

For creating port in the deploy flow (which is the flow referred to here), the get_client() does not pass admin as true meaning it uses the token of the logged in user
https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L749

In the same flow the client context is updated to admin to enable a neutron flow that needs admin (here admin is set to true because the default policy.json doesn't allow all roles to update ports and thus without admin context the below call fails)
https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L789

        # We always need admin_client to build nw_info,
        # we sometimes need it when updating ports
        admin_client = get_client(context, admin=True)

        ordered_nets, ordered_ports, preexisting_port_ids, \
            created_port_ids = self._update_ports_for_instance(
                context, instance,
                neutron, admin_client, requests_and_created_ports, nets,
                bind_host_id, dhcp_opts, available_macs)

It doesn't make sense for nova to use neutronclient with admin in some cases and without admin in others. It might work with the default neutron policy.json but not with a custom one. The code is not written consistently.

The nova-neutronclient code must either use admin context consistently , in which case it will always use the neutron service user credentials from the nova.conf file from the [neutron] section to make calls to neutron. Or, it must always use the token of the logged-in user. The problem with using the credentials of the logged-in user is that there would be cases where the logged in user's token gets expired mid-way and the nova-neutronclient logic would a fresh authentication with keystone and the credentials used at that time would be service user credentials (as the logged in user's credentials are not available to generate a fresh token) and that would again lead to inconsistencies.

This defect must be used to decide the course of action either way and not leave things inconsistently.

[Markus Zoeller (markus_z) (mzoeller)]

I'm not sure how to proceed here and asked on the ML for advice: http://lists.openstack.org/pipermail/openstack-dev/2016-August/100610.html

[Esha Seth (eshaseth)]

The issue could be solved by having either get_client(context, admin=True) at all places or get_client(context) while getting neutron_client for nova. The call should be consistent and not having admin=True for some cases and not for others.

[Matthew Edmonds (edmondsw)]

The issue can be summed up like this... when nova has asked neutron to create a port in previous releases, it has always used a token with the admin role. Something has gone into newton that has changed this behavior, which has the potential to break users who have customized neutron's policy.json file. Unless there's a very good reason for this change and it is appropriately documented in the release notes so that folks will know to look for and handle this on upgrades, that behavioral change needs to be reverted.

[Matt Riedemann (mriedem)]

OK so here is the problem, in master (newton) it's using the non-admin port client:

https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L702

That's coming from here:

https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L749

Before newton, that would be the port_client here with admin=True if using the port binding extension:

https://github.com/openstack/nova/blob/stable/mitaka/nova/network/neutronv2/api.py#L564

So before newton we used an admin port client, but with the refactor changes in newton it's using the non-admin client hence the regression.

[Augustina Ragwitz (auggy)]

Per conversation in IRC I volunteered to work on this bug.

[OpenStack Infra (hudson-openstack)]

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

[John Garbutt (johngarbutt)]

So this change was intentional, we need to get off the admin token.

My preferred way to fix this is to stop deciding if we should use the admin token or not based on if the port_binding extension is enabled.

There is no (sensible) Neutron deployment that would ever have port binding disabled right now, so we probably want to fix that anyway (I will work on a patch for that).

[OpenStack Infra (hudson-openstack)]

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

[Matthew Edmonds (edmondsw)]

@John we noticed this in a deployment that has port_binding extension enabled, so I'm not sure how that would be related.

[Augustina Ragwitz (auggy)]

Thanks John for the clarification and the update! I will work on getting the tests passing with this new change.

[Esha Seth (eshaseth)]

@auggy Is your change addressing:

So before newton we used an admin port client, but with the refactor changes in newton it's using the non-admin client hence the regression.
So we will change it all to make it consistent.

[Sean Dague (sdague)]

Patch in merge conflict, moving this out of In Progress

[Sean Dague (sdague)]

Automatically discovered version newton in description. If this is incorrect, please update the description to include 'nova version: ...'

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/357726
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=7f38e25e7df7be805459998c591e7bf8d900f766
Submitter: Jenkins
Branch: master

commit 7f38e25e7df7be805459998c591e7bf8d900f766
Author: John Garbutt <email address hidden>
Date: Fri Aug 19 10:17:58 2016 +0100

    Assume neutron port_binding extensions enabled

    No recent neutron deployment should ever have the port_binding extension
    missing in its list.

    It appears like this has been the case since this commit in Liberty:
    61121c5f2af27e31092db7ac6947f796198410a8

    It causing lots of confusion around when an admin_client should be used,
    among other things, so lets remove this needless complexity.

    Co-Authored-By: Augustina Ragwitz <email address hidden>
    Change-Id: I5fa73fa0610b23ef231952b2035a284819186a7c
    Related-Bug: 1608601

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.opendev.org/357540
Reason: This is super old and in merge conflict so I'm going to abandon. Can be restored and rebased if someone wants to continue working on this.