Use auth values from neutron conf when managing Neutron ports

Using the auth values from the neutron section of the Ironic
configuration prevents issues where a non-admin user is not
allowed to manage Neutron ports.

Change-Id: I66cf1acba10a7b7b82f5a2a55453c3d5a29a086e
Story: #2006506
Task: #38789

ironic/common/neutron.py

@@ -12,6 +12,7 @@
 
 import copy
 
+from keystoneauth1 import loading as ks_loading
 import netaddr
 from neutronclient.common import exceptions as neutron_exceptions
 from neutronclient.v2_0 import client as clientv20
@@ -79,6 +80,49 @@ def get_client(token=None, context=None):
                             timeout=CONF.neutron.request_timeout)
 
 
+def _get_conf_client(context):
+    """Retrieve a neutron client connection using conf parameters.
+
+    :param context: request context,
+                    instance of ironic.common.context.RequestContext
+    :returns: A neutron client.
+    """
+
+    auth = ks_loading.load_auth_from_conf_options(CONF, 'neutron')
+    session = ks_loading.load_session_from_conf_options(
+        CONF,
+        'neutron',
+        auth=auth)
+    endpoint = keystone.get_endpoint('neutron', session=session,
+                                     auth=auth)
+    return clientv20.Client(session=session,
+                            auth=auth,
+                            endpoint_override=endpoint,
+                            retries=CONF.neutron.retries,
+                            global_request_id=context.global_id,
+                            timeout=CONF.neutron.request_timeout)
+
+
+def update_neutron_port(context, port_id, update_body, client=None):
+    """Undate a neutron port
+
+    Uses neutron client from conf client to update a neutron client
+    an unbound state.
+
+    :param context: request context,
+                    instance of ironic.common.context.RequestContext
+    :param port_id: Neutron port ID.
+    :param update_body: Body of update
+    :param client: Optional Neutron client
+    """
+    if not client:
+        # verify that user can see the port before updating it
+        get_client(context=context).show_port(port_id)
+        client = _get_conf_client(context)
+
+    return client.update_port(port_id, update_body)
+
+
 def unbind_neutron_port(port_id, client=None, context=None):
     """Unbind a neutron port
 
@@ -92,20 +136,17 @@ def unbind_neutron_port(port_id, client=None, context=None):
     :raises: NetworkError
     """
 
-    if not client:
-        client = get_client(context=context)
-
     body_unbind = {'port': {'binding:host_id': '',
                             'binding:profile': {}}}
     body_reset_mac = {'port': {'mac_address': None}}
 
     try:
-        client.update_port(port_id, body_unbind)
+        update_neutron_port(context, port_id, body_unbind, client)
         # NOTE(hjensas): We need to reset the mac address in a separate step.
         #   Exception PortBound will be raised by neutron as it refuses to
         #   update the mac address of a bound port if we attempt to unbind and
         #   reset the mac in the same call.
-        client.update_port(port_id, body_reset_mac)
+        update_neutron_port(context, port_id, body_reset_mac, client)
     # NOTE(vsaienko): Ignore if port was deleted before calling vif detach.
     except neutron_exceptions.PortNotFoundClient:
         LOG.info('Port %s was not found while unbinding.', port_id)
@@ -142,10 +183,10 @@ def update_port_address(port_id, address, context=None):
             msg = (_("Failed to remove the current binding from "
                      "Neutron port %s, while updating its MAC "
                      "address.") % port_id)
-            unbind_neutron_port(port_id, client=client, context=context)
+            unbind_neutron_port(port_id, context=context)
 
         msg = (_("Failed to update MAC address on Neutron port %s.") % port_id)
-        client.update_port(port_id, port_req_body)
+        update_neutron_port(context, port_id, port_req_body)
 
         # Restore original binding:profile and host_id
         if binding_host_id:
@@ -154,7 +195,7 @@ def update_port_address(port_id, address, context=None):
             port_req_body = {'port': {'binding:host_id': binding_host_id,
                                       'binding:profile': binding_profile}}
 
-            client.update_port(port_id, port_req_body)
+            update_neutron_port(context, port_id, port_req_body)
     except (neutron_exceptions.NeutronClientException, exception.NetworkError):
         LOG.exception(msg)
         raise exception.FailedToUpdateMacOnPort(port_id=port_id)
@@ -193,7 +234,7 @@ def _verify_security_groups(security_groups, client):
     raise exception.NetworkError(msg)
 
 
-def _add_ip_addresses_for_ipv6_stateful(port, client):
+def _add_ip_addresses_for_ipv6_stateful(context, port, client):
     """Add additional IP addresses to the ipv6 stateful neutron port
 
     When network booting with DHCPv6-stateful we cannot control the CLID/IAID
@@ -216,7 +257,7 @@ def _add_ip_addresses_for_ipv6_stateful(port, client):
             fixed_ips.append({'subnet_id': subnet['id']})
 
         body = {'port': {'fixed_ips': fixed_ips}}
-        client.update_port(port['port']['id'], body)
+        update_neutron_port(context, port['port']['id'], body)
 
 
 def add_ports_to_network(task, network_uuid, security_groups=None):
@@ -254,8 +295,13 @@ def add_ports_to_network(task, network_uuid, security_groups=None):
             'network_id': network_uuid,
             'admin_state_up': True,
             'binding:vnic_type': VNIC_BAREMETAL,
-            'device_owner': 'baremetal:none',
+        }
+    }
+    # separate out fields that can only be updated by admins
+    update_body = {
+        'port': {
             'binding:host_id': node.uuid,
+            'device_owner': 'baremetal:none',
         }
     }
     if security_groups:
@@ -283,14 +329,15 @@ def add_ports_to_network(task, network_uuid, security_groups=None):
     for ironic_port in ports_to_create:
         # Start with a clean state for each port
         port_body = copy.deepcopy(body)
+        update_port_body = copy.deepcopy(update_body)
         # Skip ports that are missing required information for deploy.
         if not validate_port_info(node, ironic_port):
             failures.append(ironic_port.uuid)
             continue
-        port_body['port']['mac_address'] = ironic_port.address
+        update_port_body['port']['mac_address'] = ironic_port.address
         binding_profile = {'local_link_information':
                            [portmap[ironic_port.uuid]]}
-        port_body['port']['binding:profile'] = binding_profile
+        update_port_body['port']['binding:profile'] = binding_profile
 
         if not ironic_port.pxe_enabled:
             LOG.debug("Adding port %(port)s to network %(net)s for "
@@ -306,7 +353,7 @@ def add_ports_to_network(task, network_uuid, security_groups=None):
                       'port %(port_id)s, hostname %(hostname)s',
                       {'port_id': ironic_port.uuid,
                        'hostname': link_info['hostname']})
-            port_body['port']['binding:host_id'] = link_info['hostname']
+            update_port_body['port']['binding:host_id'] = link_info['hostname']
 
             # TODO(hamdyk): use portbindings.VNIC_SMARTNIC from neutron-lib
             port_body['port']['binding:vnic_type'] = VNIC_SMARTNIC
@@ -319,11 +366,13 @@ def add_ports_to_network(task, network_uuid, security_groups=None):
             port_body['port']['extra_dhcp_opts'] = extra_dhcp_opts
         try:
             if is_smart_nic:
-                wait_for_host_agent(client,
+                wait_for_host_agent(
-                                    port_body['port']['binding:host_id'])
+                    client, update_port_body['port']['binding:host_id'])
             port = client.create_port(port_body)
+            update_neutron_port(task.context, port['port']['id'],
+                                update_port_body)
             if CONF.neutron.dhcpv6_stateful_address_count > 1:
-                _add_ip_addresses_for_ipv6_stateful(port, client)
+                _add_ip_addresses_for_ipv6_stateful(task.context, port, client)
             if is_smart_nic:
                 wait_for_port_status(client, port['port']['id'], 'ACTIVE')
         except neutron_exceptions.NeutronClientException as e:

ironic/dhcp/neutron.py

@@ -61,8 +61,7 @@ class NeutronDHCPApi(base.BaseDHCP):
             port_id, dhcp_options, token=token, context=context)
         port_req_body = {'port': {'extra_dhcp_opts': dhcp_options}}
         try:
-            neutron.get_client(token=token, context=context).update_port(
+            neutron.update_neutron_port(context, port_id, port_req_body)
-                port_id, port_req_body)
         except neutron_client_exc.NeutronClientException:
             LOG.exception("Failed to update Neutron port %s.", port_id)
             raise exception.FailedToUpdateDHCPOptOnPort(port_id=port_id)

ironic/drivers/modules/network/common.py

@@ -283,7 +283,7 @@ def plug_port_to_tenant_network(task, port_like_obj, client=None):
         neutron.wait_for_host_agent(client, body['port']['binding:host_id'])
 
     try:
-        client.update_port(vif_id, body)
+        neutron.update_neutron_port(task.context, vif_id, body)
         if is_smart_nic:
             neutron.wait_for_port_status(client, vif_id, 'ACTIVE')
     except neutron_exceptions.ConnectionFailed as e:

ironic/drivers/modules/network/flat.py

@@ -55,7 +55,6 @@ class FlatNetwork(common.NeutronVIFPortIDMixin,
 
     def _bind_flat_ports(self, task):
         LOG.debug("Binding flat network ports")
-        client = neutron.get_client(context=task.context)
         for port_like_obj in task.ports + task.portgroups:
             vif_port_id = (
                 port_like_obj.internal_info.get(common.TENANT_VIF_KEY)
@@ -71,7 +70,8 @@ class FlatNetwork(common.NeutronVIFPortIDMixin,
                 }
             }
             try:
-                client.update_port(vif_port_id, body)
+                neutron.update_neutron_port(task.context,
+                                            vif_port_id, body)
             except neutron_exceptions.NeutronClientException as e:
                 msg = (_('Unable to set binding:host_id for '
                          'neutron port %(port_id)s. Error: '

ironic/tests/unit/common/test_neutron.py

@@ -145,6 +145,104 @@ class TestNeutronClient(base.TestCase):
         self.assertEqual(0, mock_sauth.call_count)
 
 
+class TestNeutronConfClient(base.TestCase):
+
+    def setUp(self):
+        super(TestNeutronConfClient, self).setUp()
+        # NOTE(pas-ha) register keystoneauth dynamic options manually
+        plugin = kaloading.get_plugin_loader('password')
+        opts = kaloading.get_auth_plugin_conf_options(plugin)
+        self.cfg_fixture.register_opts(opts, group='neutron')
+        self.config(retries=2,
+                    group='neutron')
+        self.config(username='test-admin-user',
+                    project_name='test-admin-tenant',
+                    password='test-admin-password',
+                    auth_url='test-auth-uri',
+                    auth_type='password',
+                    interface='internal',
+                    service_type='network',
+                    timeout=10,
+                    group='neutron')
+        # force-reset the global session object
+        neutron._NEUTRON_SESSION = None
+        self.context = context.RequestContext(global_request_id='global')
+
+    @mock.patch('keystoneauth1.loading.load_auth_from_conf_options',
+                autospec=True, return_value=mock.sentinel.auth)
+    @mock.patch('keystoneauth1.loading.load_session_from_conf_options',
+                autospec=True, return_value=mock.sentinel.session)
+    @mock.patch('ironic.common.keystone.get_endpoint', autospec=True,
+                return_value='neutron_url')
+    @mock.patch.object(client.Client, "__init__", return_value=None,
+                       autospec=True)
+    def test_get_neutron_conf_client(self, mock_client, mock_get_endpoint,
+                                     mock_session, mock_auth):
+        neutron._get_conf_client(self.context)
+        mock_client.assert_called_once_with(mock.ANY,  # this is 'self'
+                                            session=mock.sentinel.session,
+                                            auth=mock.sentinel.auth, retries=2,
+                                            endpoint_override='neutron_url',
+                                            global_request_id='global',
+                                            timeout=45)
+
+
+class TestUpdateNeutronPort(base.TestCase):
+    def setUp(self):
+        super(TestUpdateNeutronPort, self).setUp()
+
+        self.uuid = uuidutils.generate_uuid()
+        self.context = context.RequestContext()
+        self.update_body = {'port': {}}
+
+    @mock.patch.object(neutron, 'get_client', autospec=True)
+    @mock.patch.object(neutron, '_get_conf_client', autospec=True)
+    def test_update_neutron_port(self, conf_client_mock, client_mock):
+        client_mock.return_value.show_port.return_value = {'port': {}}
+        conf_client_mock.return_value.update_port.return_value = {'port': {}}
+
+        neutron.update_neutron_port(self.context, self.uuid, self.update_body)
+
+        client_mock.assert_called_once_with(context=self.context)
+        client_mock.return_value.show_port.assert_called_once_with(self.uuid)
+        conf_client_mock.assert_called_once_with(self.context)
+        conf_client_mock.return_value.update_port.assert_called_once_with(
+            self.uuid, self.update_body)
+
+    @mock.patch.object(neutron, 'get_client', autospec=True)
+    @mock.patch.object(neutron, '_get_conf_client', autospec=True)
+    def test_update_neutron_port_with_client(self, conf_client_mock,
+                                             client_mock):
+        client_mock.return_value.show_port.return_value = {'port': {}}
+        conf_client_mock.return_value.update_port.return_value = {'port': {}}
+        client = mock.Mock()
+        client.update_port.return_value = {'port': {}}
+
+        neutron.update_neutron_port(self.context, self.uuid, self.update_body,
+                                    client)
+
+        self.assertFalse(client_mock.called)
+        self.assertFalse(conf_client_mock.called)
+        client.update_port.assert_called_once_with(self.uuid, self.update_body)
+
+    @mock.patch.object(neutron, 'get_client', autospec=True)
+    @mock.patch.object(neutron, '_get_conf_client', autospec=True)
+    def test_update_neutron_port_with_exception(self, conf_client_mock,
+                                                client_mock):
+        client_mock.return_value.show_port.side_effect = \
+            neutron_client_exc.NeutronClientException
+        conf_client_mock.return_value.update_port.return_value = {'port': {}}
+
+        self.assertRaises(
+            neutron_client_exc.NeutronClientException,
+            neutron.update_neutron_port,
+            self.context, self.uuid, self.update_body)
+
+        client_mock.assert_called_once_with(context=self.context)
+        client_mock.return_value.show_port.assert_called_once_with(self.uuid)
+        self.assertFalse(conf_client_mock.called)
+
+
 class TestNeutronNetworkActions(db_base.DbTestCase):
 
     _CLIENT_ID = (
@@ -172,7 +270,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
         patcher.start()
         self.addCleanup(patcher.stop)
 
-    def _test_add_ports_to_network(self, is_client_id,
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
+    def _test_add_ports_to_network(self, update_mock, is_client_id,
                                    security_groups=None,
                                    add_all_ports=False):
         # Ports will be created only if pxe_enabled is True
@@ -192,14 +291,18 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
             extra['client-id'] = self._CLIENT_ID
             port.extra = extra
             port.save()
-        expected_body = {
+        expected_create_body = {
             'port': {
                 'network_id': self.network_uuid,
                 'admin_state_up': True,
                 'binding:vnic_type': 'baremetal',
+                'device_id': self.node.uuid,
+            }
+        }
+        expected_update_body = {
+            'port': {
                 'device_owner': 'baremetal:none',
                 'binding:host_id': self.node.uuid,
-                'device_id': self.node.uuid,
                 'mac_address': port.address,
                 'binding:profile': {
                     'local_link_information': [port.local_link_connection]
@@ -207,16 +310,17 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
             }
         }
         if security_groups:
-            expected_body['port']['security_groups'] = security_groups
+            expected_create_body['port']['security_groups'] = security_groups
 
         if is_client_id:
-            expected_body['port']['extra_dhcp_opts'] = (
+            expected_create_body['port']['extra_dhcp_opts'] = (
                 [{'opt_name': '61', 'opt_value': self._CLIENT_ID}])
 
         if add_all_ports:
-            expected_body2 = copy.deepcopy(expected_body)
+            expected_create_body2 = copy.deepcopy(expected_create_body)
-            expected_body2['port']['mac_address'] = port2.address
+            expected_update_body2 = copy.deepcopy(expected_update_body)
-            expected_body2['fixed_ips'] = []
+            expected_update_body2['port']['mac_address'] = port2.address
+            expected_create_body2['fixed_ips'] = []
             neutron_port2 = {'id': '132f871f-eaec-4fed-9475-0d54465e0f01',
                              'mac_address': port2.address,
                              'fixed_ips': []}
@@ -237,12 +341,21 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
                 task, self.network_uuid, security_groups=security_groups)
             self.assertEqual(expected, ports)
             if add_all_ports:
-                calls = [mock.call(expected_body),
+                create_calls = [mock.call(expected_create_body),
-                         mock.call(expected_body2)]
+                                mock.call(expected_create_body2)]
-                self.client_mock.create_port.assert_has_calls(calls)
+                update_calls = [
+                    mock.call(self.context, self.neutron_port['id'],
+                              expected_update_body),
+                    mock.call(self.context, neutron_port2['id'],
+                              expected_update_body2)]
+                self.client_mock.create_port.assert_has_calls(create_calls)
+                update_mock.assert_has_calls(update_calls)
             else:
                 self.client_mock.create_port.assert_called_once_with(
-                    expected_body)
+                    expected_create_body)
+                update_mock.assert_called_once_with(
+                    self.context, self.neutron_port['id'],
+                    expected_update_body)
 
     def test_add_ports_to_network(self):
         self._test_add_ports_to_network(is_client_id=False,
@@ -261,7 +374,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
         self._test_add_ports_to_network(is_client_id=False,
                                         security_groups=sg_ids)
 
-    def test__add_ip_addresses_for_ipv6_stateful(self):
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
+    def test__add_ip_addresses_for_ipv6_stateful(self, mock_update):
         subnet_id = uuidutils.generate_uuid()
         self.client_mock.show_subnet.return_value = {
             'subnet': {
@@ -285,11 +399,12 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
         }
 
         neutron._add_ip_addresses_for_ipv6_stateful(
+            self.context,
             {'port': self.neutron_port},
             self.client_mock
         )
-        self.client_mock.update_port.assert_called_once_with(
+        mock_update.assert_called_once_with(
-            self.neutron_port['id'], expected_body)
+            self.context, self.neutron_port['id'], expected_body)
 
     def test_verify_sec_groups(self):
         sg_ids = []
@@ -371,20 +486,25 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
     def test_add_ports_with_client_id_to_network(self):
         self._test_add_ports_to_network(is_client_id=True)
 
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron, 'validate_port_info', autospec=True)
-    def test_add_ports_to_network_instance_uuid(self, vpi_mock):
+    def test_add_ports_to_network_instance_uuid(self, vpi_mock, update_mock):
         self.node.instance_uuid = uuidutils.generate_uuid()
         self.node.network_interface = 'neutron'
         self.node.save()
         port = self.ports[0]
-        expected_body = {
+        expected_create_body = {
             'port': {
                 'network_id': self.network_uuid,
                 'admin_state_up': True,
                 'binding:vnic_type': 'baremetal',
+                'device_id': self.node.instance_uuid,
+            }
+        }
+        expected_update_body = {
+            'port': {
                 'device_owner': 'baremetal:none',
                 'binding:host_id': self.node.uuid,
-                'device_id': self.node.instance_uuid,
                 'mac_address': port.address,
                 'binding:profile': {
                     'local_link_information': [port.local_link_connection]
@@ -398,7 +518,11 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
         with task_manager.acquire(self.context, self.node.uuid) as task:
             ports = neutron.add_ports_to_network(task, self.network_uuid)
             self.assertEqual(expected, ports)
-            self.client_mock.create_port.assert_called_once_with(expected_body)
+            self.client_mock.create_port.assert_called_once_with(
+                expected_create_body)
+            update_mock.assert_called_once_with(self.context,
+                                                self.neutron_port['id'],
+                                                expected_update_body)
         self.assertTrue(vpi_mock.called)
 
     @mock.patch.object(neutron, 'rollback_ports', autospec=True)
@@ -413,8 +537,9 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
                 self.network_uuid)
             rollback_mock.assert_called_once_with(task, self.network_uuid)
 
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron, 'LOG', autospec=True)
-    def test_add_network_create_some_ports_fail(self, log_mock):
+    def test_add_network_create_some_ports_fail(self, log_mock, update_mock):
         object_utils.create_test_port(
             self.context, node_id=self.node.id,
             uuid=uuidutils.generate_uuid(),
@@ -788,10 +913,11 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
                           neutron.wait_for_port_status,
                           self.client_mock, 'port_id', 'DOWN')
 
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron, 'wait_for_host_agent', autospec=True)
     @mock.patch.object(neutron, 'wait_for_port_status', autospec=True)
     def test_add_smartnic_port_to_network(
-            self, wait_port_mock, wait_agent_mock):
+            self, wait_port_mock, wait_agent_mock, update_mock):
         # Ports will be created only if pxe_enabled is True
         self.node.network_interface = 'neutron'
         self.node.save()
@@ -809,14 +935,18 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
         port.is_smartnic = True
         port.save()
 
-        expected_body = {
+        expected_create_body = {
             'port': {
                 'network_id': self.network_uuid,
                 'admin_state_up': True,
                 'binding:vnic_type': 'smart-nic',
+                'device_id': self.node.uuid,
+            }
+        }
+        expected_update_body = {
+            'port': {
                 'device_owner': 'baremetal:none',
                 'binding:host_id': port.local_link_connection['hostname'],
-                'device_id': self.node.uuid,
                 'mac_address': port.address,
                 'binding:profile': {
                     'local_link_information': [port.local_link_connection]
@@ -832,7 +962,9 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
             ports = neutron.add_ports_to_network(task, self.network_uuid)
             self.assertEqual(expected, ports)
             self.client_mock.create_port.assert_called_once_with(
-                expected_body)
+                expected_create_body)
+            update_mock.assert_called_once_with(
+                self.context, self.neutron_port['id'], expected_update_body)
             wait_agent_mock.assert_called_once_with(
                 self.client_mock, 'hostname')
             wait_port_mock.assert_called_once_with(
@@ -935,18 +1067,20 @@ class TestUpdatePortAddress(base.TestCase):
         super(TestUpdatePortAddress, self).setUp()
         self.context = context.RequestContext()
 
-    def test_update_port_address(self, mock_client):
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
+    def test_update_port_address(self, mock_unp, mock_client):
         address = 'fe:54:00:77:07:d9'
         port_id = 'fake-port-id'
         expected = {'port': {'mac_address': address}}
         mock_client.return_value.show_port.return_value = {}
 
         neutron.update_port_address(port_id, address, context=self.context)
-        mock_client.return_value.update_port.assert_called_once_with(port_id,
+        mock_unp.assert_called_once_with(self.context, port_id, expected)
-                                                                     expected)
 
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True)
-    def test_update_port_address_with_binding(self, mock_unp, mock_client):
+    def test_update_port_address_with_binding(self, mock_unp, mock_update,
+                                              mock_client):
         address = 'fe:54:00:77:07:d9'
         port_id = 'fake-port-id'
 
@@ -954,19 +1088,22 @@ class TestUpdatePortAddress(base.TestCase):
             'port': {'binding:host_id': 'host',
                      'binding:profile': 'foo'}}
 
-        calls = [mock.call(port_id, {'port': {'mac_address': address}}),
+        calls = [mock.call(self.context, port_id,
-                 mock.call(port_id, {'port': {'binding:host_id': 'host',
+                           {'port': {'mac_address': address}}),
+                 mock.call(self.context, port_id,
+                           {'port': {'binding:host_id': 'host',
                                      'binding:profile': 'foo'}})]
 
         neutron.update_port_address(port_id, address, context=self.context)
         mock_unp.assert_called_once_with(
             port_id,
-            client=mock_client(context=self.context),
             context=self.context)
-        mock_client.return_value.update_port.assert_has_calls(calls)
+        mock_update.assert_has_calls(calls)
 
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True)
-    def test_update_port_address_without_binding(self, mock_unp, mock_client):
+    def test_update_port_address_without_binding(self, mock_unp, mock_update,
+                                                 mock_client):
         address = 'fe:54:00:77:07:d9'
         port_id = 'fake-port-id'
         expected = {'port': {'mac_address': address}}
@@ -975,7 +1112,7 @@ class TestUpdatePortAddress(base.TestCase):
 
         neutron.update_port_address(port_id, address, context=self.context)
         self.assertFalse(mock_unp.called)
-        mock_client.return_value.update_port.assert_any_call(port_id, expected)
+        mock_update.assert_any_call(self.context, port_id, expected)
 
     def test_update_port_address_show_failed(self, mock_client):
         address = 'fe:54:00:77:07:d9'
@@ -1002,17 +1139,18 @@ class TestUpdatePortAddress(base.TestCase):
                           port_id, address, context=self.context)
         mock_unp.assert_called_once_with(
             port_id,
-            client=mock_client(context=self.context),
             context=self.context)
         self.assertFalse(mock_client.return_value.update_port.called)
 
+    @mock.patch.object(neutron, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True)
     def test_update_port_address_with_exception(self, mock_unp,
+                                                mock_update,
                                                 mock_client):
         address = 'fe:54:00:77:07:d9'
         port_id = 'fake-port-id'
         mock_client.return_value.show_port.return_value = {}
-        mock_client.return_value.update_port.side_effect = (
+        mock_update.side_effect = (
             neutron_client_exc.NeutronClientException())
 
         self.assertRaises(exception.FailedToUpdateMacOnPort,
@@ -1020,14 +1158,14 @@ class TestUpdatePortAddress(base.TestCase):
                           port_id, address, context=self.context)
 
 
-@mock.patch.object(neutron, 'get_client', autospec=True)
+@mock.patch.object(neutron, 'update_neutron_port', autospec=True)
 class TestUnbindPort(base.TestCase):
 
     def setUp(self):
         super(TestUnbindPort, self).setUp()
         self.context = context.RequestContext()
 
-    def test_unbind_neutron_port_client_passed(self, mock_client):
+    def test_unbind_neutron_port_client_passed(self, mock_unp):
         port_id = 'fake-port-id'
         body_unbind = {
             'port': {
@@ -1040,20 +1178,18 @@ class TestUnbindPort(base.TestCase):
                 'mac_address': None
             }
         }
+        client = mock.MagicMock()
         update_calls = [
-            mock.call(port_id, body_unbind),
+            mock.call(self.context, port_id, body_unbind, client),
-            mock.call(port_id, body_reset_mac)
+            mock.call(self.context, port_id, body_reset_mac, client)
         ]
-        neutron.unbind_neutron_port(port_id,
+        neutron.unbind_neutron_port(port_id, client, context=self.context)
-                                    mock_client(context=self.context),
+        self.assertEqual(2, mock_unp.call_count)
-                                    context=self.context)
+        mock_unp.assert_has_calls(update_calls)
-        self.assertEqual(1, mock_client.call_count)
-        mock_client.return_value.update_port.assert_has_calls(update_calls)
 
     @mock.patch.object(neutron, 'LOG', autospec=True)
-    def test_unbind_neutron_port_failure(self, mock_log, mock_client):
+    def test_unbind_neutron_port_failure(self, mock_log, mock_unp):
-        mock_client.return_value.update_port.side_effect = (
+        mock_unp.side_effect = (neutron_client_exc.NeutronClientException())
-            neutron_client_exc.NeutronClientException())
         body = {
             'port': {
                 'binding:host_id': '',
@@ -1063,12 +1199,10 @@ class TestUnbindPort(base.TestCase):
         port_id = 'fake-port-id'
         self.assertRaises(exception.NetworkError, neutron.unbind_neutron_port,
                           port_id, context=self.context)
-        mock_client.assert_called_once_with(context=self.context)
+        mock_unp.assert_called_once_with(self.context, port_id, body, None)
-        mock_client.return_value.update_port.assert_called_once_with(port_id,
-                                                                     body)
         mock_log.exception.assert_called_once()
 
-    def test_unbind_neutron_port(self, mock_client):
+    def test_unbind_neutron_port(self, mock_unp):
         port_id = 'fake-port-id'
         body_unbind = {
             'port': {
@@ -1082,17 +1216,16 @@ class TestUnbindPort(base.TestCase):
             }
         }
         update_calls = [
-            mock.call(port_id, body_unbind),
+            mock.call(self.context, port_id, body_unbind, None),
-            mock.call(port_id, body_reset_mac)
+            mock.call(self.context, port_id, body_reset_mac, None)
         ]
         neutron.unbind_neutron_port(port_id, context=self.context)
-        mock_client.assert_called_once_with(context=self.context)
+        mock_unp.assert_has_calls(update_calls)
-        mock_client.return_value.update_port.assert_has_calls(update_calls)
 
     @mock.patch.object(neutron, 'LOG', autospec=True)
-    def test_unbind_neutron_port_not_found(self, mock_log, mock_client):
+    def test_unbind_neutron_port_not_found(self, mock_log, mock_unp):
         port_id = 'fake-port-id'
-        mock_client.return_value.update_port.side_effect = (
+        mock_unp.side_effect = (
             neutron_client_exc.PortNotFoundClient())
         body = {
             'port': {
@@ -1101,9 +1234,7 @@ class TestUnbindPort(base.TestCase):
             }
         }
         neutron.unbind_neutron_port(port_id, context=self.context)
-        mock_client.assert_called_once_with(context=self.context)
+        mock_unp.assert_called_once_with(self.context, port_id, body, None)
-        mock_client.return_value.update_port.assert_called_once_with(port_id,
-                                                                     body)
         mock_log.info.assert_called_once_with('Port %s was not found while '
                                               'unbinding.', port_id)
 

ironic/tests/unit/dhcp/test_neutron.py

@@ -48,8 +48,8 @@ class TestNeutron(db_base.DbTestCase):
 
         dhcp_factory.DHCPFactory._dhcp_provider = None
 
-    @mock.patch('ironic.common.neutron.get_client', autospec=True)
+    @mock.patch('ironic.common.neutron.update_neutron_port', autospec=True)
-    def test_update_port_dhcp_opts(self, client_mock):
+    def test_update_port_dhcp_opts(self, update_mock):
         opts = [{'opt_name': 'bootfile-name',
                  'opt_value': 'pxelinux.0'},
                 {'opt_name': 'tftp-server',
@@ -63,14 +63,14 @@ class TestNeutron(db_base.DbTestCase):
         with task_manager.acquire(self.context, self.node.uuid) as task:
             api.provider.update_port_dhcp_opts(port_id, opts,
                                                context=task.context)
-        client_mock.return_value.update_port.assert_called_once_with(
+        update_mock.assert_called_once_with(
-            port_id, expected)
+            task.context, port_id, expected)
 
-    @mock.patch('ironic.common.neutron.get_client', autospec=True)
+    @mock.patch('ironic.common.neutron.update_neutron_port', autospec=True)
-    def test_update_port_dhcp_opts_with_exception(self, client_mock):
+    def test_update_port_dhcp_opts_with_exception(self, update_mock):
         opts = [{}]
         port_id = 'fake-port-id'
-        client_mock.return_value.update_port.side_effect = (
+        update_mock.side_effect = (
             neutron_client_exc.NeutronClientException())
 
         api = dhcp_factory.DHCPFactory()

ironic/tests/unit/drivers/modules/network/test_common.py

@@ -400,22 +400,26 @@ class TestCommonFunctions(db_base.DbTestCase):
                               common.get_free_port_like_object,
                               task, self.vif_id, {'physnet2'})
 
+    @mock.patch.object(neutron_common, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron_common, 'get_client', autospec=True)
-    def test_plug_port_to_tenant_network_client(self, mock_gc):
+    def test_plug_port_to_tenant_network_client(self, mock_gc, mock_update):
         self.port.internal_info = {common.TENANT_VIF_KEY: self.vif_id}
         self.port.save()
         with task_manager.acquire(self.context, self.node.id) as task:
             common.plug_port_to_tenant_network(task, self.port,
                                                client=mock.MagicMock())
         self.assertFalse(mock_gc.called)
+        self.assertTrue(mock_update.called)
 
+    @mock.patch.object(neutron_common, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron_common, 'get_client', autospec=True)
-    def test_plug_port_to_tenant_network_no_client(self, mock_gc):
+    def test_plug_port_to_tenant_network_no_client(self, mock_gc, mock_update):
         self.port.internal_info = {common.TENANT_VIF_KEY: self.vif_id}
         self.port.save()
         with task_manager.acquire(self.context, self.node.id) as task:
             common.plug_port_to_tenant_network(task, self.port)
         self.assertTrue(mock_gc.called)
+        self.assertTrue(mock_update.called)
 
     @mock.patch.object(neutron_common, 'get_client', autospec=True)
     def test_plug_port_to_tenant_network_no_tenant_vif(self, mock_gc):
@@ -432,9 +436,10 @@ class TestCommonFunctions(db_base.DbTestCase):
 
     @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True)
     @mock.patch.object(neutron_common, 'wait_for_port_status', autospec=True)
+    @mock.patch.object(neutron_common, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron_common, 'get_client', autospec=True)
     def test_plug_port_to_tenant_network_smartnic_port(
-            self, mock_gc, wait_port_mock, wait_agent_mock):
+            self, mock_gc, mock_update, wait_port_mock, wait_agent_mock):
         nclient = mock.MagicMock()
         mock_gc.return_value = nclient
         local_link_connection = self.port.local_link_connection
@@ -449,6 +454,7 @@ class TestCommonFunctions(db_base.DbTestCase):
                 nclient, 'hostname')
             wait_port_mock.assert_called_once_with(
                 nclient, self.vif_id, 'ACTIVE')
+            self.assertTrue(mock_update.called)
 
 
 class TestVifPortIDMixin(db_base.DbTestCase):

ironic/tests/unit/drivers/modules/network/test_flat.py

@@ -170,10 +170,8 @@ class TestFlatInterface(db_base.DbTestCase):
         self.port.refresh()
         self.assertNotIn('cleaning_vif_port_id', self.port.internal_info)
 
-    @mock.patch.object(neutron, 'get_client')
+    @mock.patch.object(neutron, 'update_neutron_port')
-    def test__bind_flat_ports_set_binding_host_id(self, client_mock):
+    def test__bind_flat_ports_set_binding_host_id(self, update_mock):
-        upd_mock = mock.Mock()
-        client_mock.return_value.update_port = upd_mock
         extra = {'vif_port_id': 'foo'}
         utils.create_test_port(self.context, node_id=self.node.id,
                                address='52:54:00:cf:2d:33', extra=extra,
@@ -183,12 +181,10 @@ class TestFlatInterface(db_base.DbTestCase):
                              'mac_address': '52:54:00:cf:2d:33'}}
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface._bind_flat_ports(task)
-        upd_mock.assert_called_once_with('foo', exp_body)
+        update_mock.assert_called_once_with(self.context, 'foo', exp_body)
 
-    @mock.patch.object(neutron, 'get_client')
+    @mock.patch.object(neutron, 'update_neutron_port')
-    def test__bind_flat_ports_set_binding_host_id_portgroup(self, client_mock):
+    def test__bind_flat_ports_set_binding_host_id_portgroup(self, update_mock):
-        upd_mock = mock.Mock()
-        client_mock.return_value.update_port = upd_mock
         internal_info = {'tenant_vif_port_id': 'foo'}
         utils.create_test_portgroup(
             self.context, node_id=self.node.id, internal_info=internal_info,
@@ -204,8 +200,9 @@ class TestFlatInterface(db_base.DbTestCase):
                               'mac_address': '52:54:00:cf:2d:31'}}
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface._bind_flat_ports(task)
-        upd_mock.assert_has_calls([
+        update_mock.assert_has_calls([
-            mock.call('bar', exp_body1), mock.call('foo', exp_body2)])
+            mock.call(self.context, 'bar', exp_body1),
+            mock.call(self.context, 'foo', exp_body2)])
 
     @mock.patch.object(neutron, 'unbind_neutron_port')
     def test__unbind_flat_ports(self, unbind_neutron_port_mock):
@@ -234,10 +231,9 @@ class TestFlatInterface(db_base.DbTestCase):
             [mock.call('foo', context=self.context),
              mock.call('bar', context=self.context)])
 
-    @mock.patch.object(neutron, 'get_client')
+    @mock.patch.object(neutron, 'update_neutron_port')
-    def test__bind_flat_ports_set_binding_host_id_raise(self, client_mock):
+    def test__bind_flat_ports_set_binding_host_id_raise(self, update_mock):
-        client_mock.return_value.update_port.side_effect = \
+        update_mock.side_effect = (neutron_exceptions.ConnectionFailed())
-            (neutron_exceptions.ConnectionFailed())
         extra = {'vif_port_id': 'foo'}
         utils.create_test_port(self.context, node_id=self.node.id,
                                address='52:54:00:cf:2d:33', extra=extra,

ironic/tests/unit/drivers/modules/network/test_neutron.py

@@ -572,10 +572,11 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
                       log_mock.error.call_args[0][0])
 
     @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True)
+    @mock.patch.object(neutron_common, 'update_neutron_port')
     @mock.patch.object(neutron_common, 'get_client')
     @mock.patch.object(neutron, 'LOG')
     def test_configure_tenant_networks_multiple_ports_one_vif_id(
-            self, log_mock, client_mock, wait_agent_mock):
+            self, log_mock, client_mock, update_mock, wait_agent_mock):
         expected_body = {
             'port': {
                 'binding:vnic_type': 'baremetal',
@@ -585,20 +586,20 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
                 'mac_address': '52:54:00:cf:2d:32'
             }
         }
-        upd_mock = mock.Mock()
-        client_mock.return_value.update_port = upd_mock
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface.configure_tenant_networks(task)
             client_mock.assert_called_once_with(context=task.context)
-        upd_mock.assert_called_once_with(self.port.extra['vif_port_id'],
+        update_mock.assert_called_once_with(self.context,
-                                         expected_body)
+                                            self.port.extra['vif_port_id'],
+                                            expected_body)
 
     @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True)
+    @mock.patch.object(neutron_common, 'update_neutron_port')
     @mock.patch.object(neutron_common, 'get_client')
     def test_configure_tenant_networks_update_fail(self, client_mock,
+                                                   update_mock,
                                                    wait_agent_mock):
-        client = client_mock.return_value
+        update_mock.side_effect = neutron_exceptions.ConnectionFailed(
-        client.update_port.side_effect = neutron_exceptions.ConnectionFailed(
             reason='meow')
         with task_manager.acquire(self.context, self.node.id) as task:
             self.assertRaisesRegex(
@@ -607,12 +608,12 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
             client_mock.assert_called_once_with(context=task.context)
 
     @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True)
+    @mock.patch.object(neutron_common, 'update_neutron_port')
     @mock.patch.object(neutron_common, 'get_client')
-    def _test_configure_tenant_networks(self, client_mock, wait_agent_mock,
+    def _test_configure_tenant_networks(self, client_mock, update_mock,
+                                        wait_agent_mock,
                                         is_client_id=False,
                                         vif_int_info=False):
-        upd_mock = mock.Mock()
-        client_mock.return_value.update_port = upd_mock
         if vif_int_info:
             kwargs = {'internal_info': {
                 'tenant_vif_port_id': uuidutils.generate_uuid()}}
@@ -668,9 +669,9 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
         else:
             portid1 = self.port.extra['vif_port_id']
             portid2 = second_port.extra['vif_port_id']
-        upd_mock.assert_has_calls(
+        update_mock.assert_has_calls(
-            [mock.call(portid1, port1_body),
+            [mock.call(self.context, portid1, port1_body),
-             mock.call(portid2, port2_body)],
+             mock.call(self.context, portid2, port2_body)],
             any_order=True
         )
 
@@ -693,11 +694,12 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
         self._test_configure_tenant_networks(is_client_id=True)
 
     @mock.patch.object(neutron_common, 'wait_for_host_agent', autospec=True)
+    @mock.patch.object(neutron_common, 'update_neutron_port', autospec=True)
     @mock.patch.object(neutron_common, 'get_client', autospec=True)
     @mock.patch.object(neutron_common, 'get_local_group_information',
                        autospec=True)
     def test_configure_tenant_networks_with_portgroups(
-            self, glgi_mock, client_mock, wait_agent_mock):
+            self, glgi_mock, client_mock, update_mock, wait_agent_mock):
         pg = utils.create_test_portgroup(
             self.context, node_id=self.node.id, address='ff:54:00:cf:2d:32',
             extra={'vif_port_id': uuidutils.generate_uuid()})
@@ -717,8 +719,6 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
                                    'port_id': 'Ethernet1/2',
                                    'switch_info': 'switch2'}
         )
-        upd_mock = mock.Mock()
-        client_mock.return_value.update_port = upd_mock
         local_group_info = {'a': 'b'}
         glgi_mock.return_value = local_group_info
         expected_body = {
@@ -747,9 +747,11 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
             self.interface.configure_tenant_networks(task)
             client_mock.assert_called_once_with(context=task.context)
             glgi_mock.assert_called_once_with(task, pg)
-        upd_mock.assert_has_calls(
+        update_mock.assert_has_calls(
-            [mock.call(self.port.extra['vif_port_id'], call1_body),
+            [mock.call(self.context, self.port.extra['vif_port_id'],
-             mock.call(pg.extra['vif_port_id'], call2_body)]
+                       call1_body),
+             mock.call(self.context, pg.extra['vif_port_id'],
+                       call2_body)]
         )
 
     def test_need_power_on_true(self):

releasenotes/notes/neutron-port-update-598183909d44396c.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Changes neutron port updates to use auth values from Ironic's neutron
+    conf, preventing issues that can arise when a non-admin user manages
+    Ironic nodes. A check is added to the port update function to verify that
+    the user can actually see the port. This adds an additional Neutron
+    request call to all port updates.