openstack
/
ironic
Code Issues Proposed changes

Do not use user token in neutron client 

Ironic requires admin rights in Neutron to perform certain
operations like (create provisioning/cleaning ports, unbind port,
update mac_address). Previously only admins were able to use ironic,
with keystone policy implementation it is possible that baremetal
admin doesn't have enough rights (admin rights) in Neutron with leads
to neutron operations failures.

This patch ensures that we do not pass user token to neutron client,
and always pick admin session.

Closes-Bug: #1657675

Change-Id: If17d5501062075fb8d6ca0eb4f2f38c87e2c2cc3
This commit is contained in:
Vasyl Saienko 2017-01-19 13:16:54 +02:00
parent 713a440884
commit 7bfd88aa37
10 changed files with 42 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
ironic/common/neutron.py
View File

@ -69,20 +69,19 @@ def get_client(token=None):
return clientv20.Client(**params)
def unbind_neutron_port(port_id, client=None, token=None):
def unbind_neutron_port(port_id, client=None):
"""Unbind a neutron port
Remove a neutron port's binding profile and host ID so that it returns to
an unbound state.
:param port_id: Neutron port ID.
:param token: Optional auth token.
:param client: Optional a Neutron client object.
:raises: NetworkError
"""
if not client:
client = get_client(token)
client = get_client()
body = {'port': {'binding:host_id': '',
'binding:profile': {}}}
@ -97,15 +96,14 @@ def unbind_neutron_port(port_id, client=None, token=None):
raise exception.NetworkError(msg)
def update_port_address(port_id, address, token=None):
def update_port_address(port_id, address):
"""Update a port's mac address.
:param port_id: Neutron port id.
:param address: new MAC address.
:param token: optional auth token.
:raises: FailedToUpdateMacOnPort
"""
client = get_client(token)
client = get_client()
port_req_body = {'port': {'mac_address': address}}
try:
@ -178,7 +176,7 @@ def add_ports_to_network(task, network_uuid, is_flat=False,
:raises: NetworkError
:returns: a dictionary in the form {port.uuid: neutron_port['id']}
"""
client = get_client(task.context.auth_token)
client = get_client()
node = task.node
# If Security Groups are specified, verify that they exist
@ -283,7 +281,7 @@ def remove_neutron_ports(task, params):
:param params: Dict of params to filter ports.
:raises: NetworkError
"""
client = get_client(task.context.auth_token)
client = get_client()
node_uuid = task.node.uuid
try:

5
ironic/dhcp/neutron.py
View File

@ -125,8 +125,7 @@ class NeutronDHCPApi(base.BaseDHCP):
vif_list = [vif for pdict in vifs.values() for vif in pdict.values()]
for vif in vif_list:
try:
self.update_port_dhcp_opts(vif, options,
token=task.context.auth_token)
self.update_port_dhcp_opts(vif, options)
except exception.FailedToUpdateDHCPOptOnPort:
failures.append(vif)
@ -263,7 +262,7 @@ class NeutronDHCPApi(base.BaseDHCP):
:returns: List of IP addresses associated with
task's ports/portgroups.
"""
client = neutron.get_client(task.context.auth_token)
client = neutron.get_client()
port_ip_addresses = self._get_ip_addresses(task, task.ports, client)
portgroup_ip_addresses = self._get_ip_addresses(

12
ironic/drivers/modules/network/common.py
View File

@ -155,9 +155,7 @@ class VIFPortIDMixin(object):
port_obj.extra.get('vif_port_id'))
if 'address' in port_obj.obj_what_changed():
if vif:
neutron.update_port_address(vif,
port_obj.address,
token=context.auth_token)
neutron.update_port_address(vif, port_obj.address)
if 'extra' in port_obj.obj_what_changed():
original_port = objects.Port.get_by_id(context, port_obj.id)
@ -176,7 +174,7 @@ class VIFPortIDMixin(object):
'opt_value': updated_client_id}
api.provider.update_port_dhcp_opts(
vif, [client_id_opt], token=context.auth_token)
vif, [client_id_opt])
# Log warning if there is no VIF and an instance
# is associated with the node.
elif node.instance_uuid:
@ -223,9 +221,7 @@ class VIFPortIDMixin(object):
pg_vif = (portgroup_obj.internal_info.get(TENANT_VIF_KEY) or
portgroup_obj.extra.get('vif_port_id'))
if pg_vif:
neutron.update_port_address(pg_vif,
portgroup_obj.address,
token=context.auth_token)
neutron.update_port_address(pg_vif, portgroup_obj.address)
if 'extra' in portgroup_obj.obj_what_changed():
original_portgroup = objects.Portgroup.get_by_id(context,
@ -294,7 +290,7 @@ class VIFPortIDMixin(object):
# Check if the requested vif_id is a neutron port. If it is
# then attempt to update the port's MAC address.
try:
client = neutron.get_client(task.context.auth_token)
client = neutron.get_client()
client.show_port(vif_id)
except neutron_exceptions.NeutronClientException:
# NOTE(sambetts): If a client error occurs this is because

2
ironic/drivers/modules/network/flat.py
View File

@ -69,7 +69,7 @@ class FlatNetwork(common.VIFPortIDMixin, neutron.NeutronNetworkInterfaceMixin,
if not host_id:
return
client = neutron.get_client(task.context.auth_token)
client = neutron.get_client()
for port_like_obj in task.ports + task.portgroups:
vif_port_id = (
port_like_obj.internal_info.get('tenant_vif_port_id') or

5
ironic/drivers/modules/network/neutron.py
View File

@ -162,7 +162,7 @@ class NeutronNetwork(common.VIFPortIDMixin,
portmap = neutron.get_node_portmap(task)
client = neutron.get_client(task.context.auth_token)
client = neutron.get_client()
pobj_without_vif = 0
for port_like_obj in ports + portgroups:
vif_port_id = (
@ -244,5 +244,4 @@ class NeutronNetwork(common.VIFPortIDMixin,
port_like_obj.extra.get('vif_port_id'))
if not vif_port_id:
continue
neutron.unbind_neutron_port(vif_port_id,
token=task.context.auth_token)
neutron.unbind_neutron_port(vif_port_id)

24
ironic/tests/unit/common/test_neutron.py
View File

@ -609,20 +609,6 @@ class TestUnbindPort(base.TestCase):
mock_client.return_value.update_port.assert_called_once_with(port_id,
body)
def test_unbind_neutron_port_token_passed(self, mock_client):
port_id = 'fake-port-id'
token = 'token'
body = {
'port': {
'binding:host_id': '',
'binding:profile': {}
}
}
neutron.unbind_neutron_port(port_id, token=token)
mock_client.assert_called_once_with(token)
mock_client.return_value.update_port.assert_called_once_with(port_id,
body)
@mock.patch.object(neutron, 'LOG')
def test_unbind_neutron_port_failure(self, mock_log, mock_client):
mock_client.return_value.update_port.side_effect = (
@ -634,24 +620,22 @@ class TestUnbindPort(base.TestCase):
}
}
port_id = 'fake-port-id'
token = 'token'
self.assertRaises(exception.NetworkError, neutron.unbind_neutron_port,
port_id, token=token)
mock_client.assert_called_once_with(token)
port_id)
mock_client.assert_called_once_with()
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):
port_id = 'fake-port-id'
token = 'token'
body = {
'port': {
'binding:host_id': '',
'binding:profile': {}
}
}
neutron.unbind_neutron_port(port_id, token=token)
mock_client.assert_called_once_with(token)
neutron.unbind_neutron_port(port_id)
mock_client.assert_called_once_with()
mock_client.return_value.update_port.assert_called_once_with(port_id,
body)

15
ironic/tests/unit/dhcp/test_neutron.py
View File

@ -117,8 +117,7 @@ class TestNeutron(db_base.DbTestCase):
opts = pxe_utils.dhcp_options_for_instance(task)
api = dhcp_factory.DHCPFactory()
api.update_dhcp(task, opts)
mock_updo.assert_called_once_with('vif-uuid', opts,
token=self.context.auth_token)
mock_updo.assert_called_once_with('vif-uuid', opts)
@mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts')
@mock.patch('ironic.common.network.get_node_vif_ids')
@ -179,8 +178,7 @@ class TestNeutron(db_base.DbTestCase):
api = dhcp_factory.DHCPFactory()
api.update_dhcp(task, opts)
mock_ts.assert_called_with(30)
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts,
token=self.context.auth_token)
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts)
@mock.patch.object(neutron, 'LOG', autospec=True)
@mock.patch('time.sleep', autospec=True)
@ -201,8 +199,7 @@ class TestNeutron(db_base.DbTestCase):
self.assertIn('Setting the port delay to 15 for SSH',
mock_log.warning.call_args[0][0])
mock_ts.assert_called_with(15)
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts,
token=self.context.auth_token)
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts)
@mock.patch.object(neutron, 'LOG', autospec=True)
@mock.patch('time.sleep', autospec=True)
@ -223,8 +220,7 @@ class TestNeutron(db_base.DbTestCase):
"Waiting %d seconds for Neutron.", 30)
mock_log.warning.assert_not_called()
mock_ts.assert_called_with(30)
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts,
token=self.context.auth_token)
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts)
@mock.patch.object(neutron, 'LOG', autospec=True)
@mock.patch.object(neutron.NeutronDHCPApi, 'update_port_dhcp_opts',
@ -241,8 +237,7 @@ class TestNeutron(db_base.DbTestCase):
api.update_dhcp(task, opts)
mock_log.debug.assert_not_called()
mock_log.warning.assert_not_called()
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts,
token=self.context.auth_token)
mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts)
def test__get_fixed_ip_address(self):
port_id = 'fake-port-id'

18
ironic/tests/unit/drivers/modules/network/test_common.py
View File

@ -257,7 +257,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
self.port.refresh()
self.assertEqual("fake_vif_id", self.port.internal_info.get(
common.TENANT_VIF_KEY))
mock_client.assert_called_once_with(None)
mock_client.assert_called_once_with()
mock_upa.assert_called_once_with("fake_vif_id", self.port.address)
@mock.patch.object(common, 'get_free_port_like_object', autospec=True)
@ -292,7 +292,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
self.assertRaisesRegexp(
exception.NetworkError, "can not update Neutron port",
self.interface.vif_attach, task, vif)
mock_client.assert_called_once_with(None)
mock_client.assert_called_once_with()
def test_vif_detach_in_extra(self):
with task_manager.acquire(self.context, self.node.id) as task:
@ -402,8 +402,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.port_changed(task, self.port)
mac_update_mock.assert_called_once_with(
self.port.extra['vif_port_id'],
new_address, token=task.context.auth_token)
self.port.extra['vif_port_id'], new_address)
self.assertFalse(mock_warn.called)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
@ -417,8 +416,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
self.interface.port_changed,
task, self.port)
mac_update_mock.assert_called_once_with(
self.port.extra['vif_port_id'],
new_address, token=task.context.auth_token)
self.port.extra['vif_port_id'], new_address)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_port_changed_address_no_vif_id(self, mac_update_mock):
@ -437,7 +435,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.port_changed(task, self.port)
dhcp_update_mock.assert_called_once_with(
'fake-id', expected_dhcp_opts, token=self.context.auth_token)
'fake-id', expected_dhcp_opts)
@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)
@ -656,8 +654,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
pg.address = new_address
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.portgroup_changed(task, pg)
mac_update_mock.assert_called_once_with('fake-id', new_address,
token=self.context.auth_token)
mac_update_mock.assert_called_once_with('fake-id', new_address)
@mock.patch.object(neutron_common, 'update_port_address', autospec=True)
def test_update_portgroup_remove_address(self, mac_update_mock):
@ -724,8 +721,7 @@ class TestVifPortIDMixin(db_base.DbTestCase):
self.assertRaises(exception.FailedToUpdateMacOnPort,
self.interface.portgroup_changed,
task, pg)
mac_update_mock.assert_called_once_with('fake-id', new_address,
token=self.context.auth_token)
mac_update_mock.assert_called_once_with('fake-id', new_address)
@mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id',
autospec=True)

12
ironic/tests/unit/drivers/modules/network/test_neutron.py
View File

@ -220,7 +220,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.unconfigure_tenant_networks(task)
mock_unbind_port.assert_called_once_with(
self.port.extra['vif_port_id'], token=None)
self.port.extra['vif_port_id'])
def test_configure_tenant_networks_no_ports_for_node(self):
n = utils.create_test_node(self.context, network_interface='neutron',
@ -243,7 +243,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
'associated with node',
self.interface.configure_tenant_networks,
task)
client_mock.assert_called_once_with(task.context.auth_token)
client_mock.assert_called_once_with()
upd_mock.assert_not_called()
self.assertIn('No neutron ports or portgroups are associated with',
log_mock.error.call_args[0][0])
@ -270,7 +270,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
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(task.context.auth_token)
client_mock.assert_called_once_with()
upd_mock.assert_called_once_with(self.port.extra['vif_port_id'],
expected_body)
@ -283,7 +283,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
self.assertRaisesRegexp(
exception.NetworkError, 'Could not add',
self.interface.configure_tenant_networks, task)
client_mock.assert_called_once_with(task.context.auth_token)
client_mock.assert_called_once_with()
@mock.patch.object(neutron_common, 'get_client')
def _test_configure_tenant_networks(self, client_mock, is_client_id=False,
@ -339,7 +339,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
[{'opt_name': 'client-id', 'opt_value': client_ids[1]}])
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.configure_tenant_networks(task)
client_mock.assert_called_once_with(task.context.auth_token)
client_mock.assert_called_once_with()
if vif_int_info:
portid1 = self.port.internal_info['tenant_vif_port_id']
portid2 = second_port.internal_info['tenant_vif_port_id']
@ -413,7 +413,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
}
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.configure_tenant_networks(task)
client_mock.assert_called_once_with(task.context.auth_token)
client_mock.assert_called_once_with()
upd_mock.assert_has_calls(
[mock.call(self.port.extra['vif_port_id'], call1_body),
mock.call(pg.extra['vif_port_id'], call2_body)]

5
releasenotes/notes/fix-baremetal-admin-user-not-neutron-admin-f163df90ab520dad.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- An issue when baremetal admin user doesn't have enough rights (admin)
in Neutron by always picking neutron user from ironic config
and avoiding passing client token.