From 3197e44c04de064bc3d8af09af7e0d2d9511af6d Mon Sep 17 00:00:00 2001 From: Sukhdev Kapur Date: Fri, 26 Aug 2016 13:12:57 -0700 Subject: [PATCH] Add support for Security Groups for baremetal servers This patch adds support for Neutron Security Groups to the baremetal severs when neutron network interface is used for deployments. Specifically, this patch adds support so that security groups could be specified (and applied) for provisioning and cleaning networks. Change-Id: I0cf652bdd220480b104e478f2096bf89a9ba8bdf Partial-bug: #1594242 --- etc/ironic/ironic.conf.sample | 14 ++++ ironic/common/neutron.py | 32 +++++++- ironic/conf/neutron.py | 16 ++++ ironic/drivers/modules/network/neutron.py | 7 +- ironic/tests/unit/common/test_neutron.py | 77 ++++++++++++++++++- .../drivers/modules/network/test_neutron.py | 44 +++++++++++ .../security_groups-b57a5d6c30c2fae4.yaml | 10 +++ 7 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/security_groups-b57a5d6c30c2fae4.yaml diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 6614a172e7..d1af2073f1 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1933,6 +1933,13 @@ # interface or "neutron" DHCP provider. (string value) #cleaning_network_uuid = +# List of Neutron Security Group UUIDs to be applied during +# cleaning of the nodes. Optional for the "neutron" network +# interface and not used for the "flat" or "noop" network +# interfaces. If not specified, default security +# group is used. (list value) +#cleaning_network_security_groups = + # Optional domain ID to use with v3 and v2 parameters. It will # be used for both the user and project domain in v3 and # ignored in v2 authentication. (string value) @@ -1982,6 +1989,13 @@ # interface. (string value) #provisioning_network_uuid = +# List of Neutron Security Group UUIDs to be applied during +# provisioning of the nodes. Optional for the "neutron" +# network interface and not used for the "flat" or "noop" +# network interfaces. If not specified, default +# security group is used. (list value) +#provisioning_network_security_groups = + # Client retries in the case of a failed request. (integer # value) #retries = 3 diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 4290fd31a7..915fdcc39e 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -68,7 +68,30 @@ def get_client(token=None): return clientv20.Client(**params) -def add_ports_to_network(task, network_uuid, is_flat=False): +def _verify_security_groups(security_groups, client): + if not security_groups: + return + try: + neutron_sec_groups = ( + client.list_security_groups().get('security_groups') or []) + except neutron_exceptions.NeutronClientException as e: + msg = (_("Could not retrieve neutron security groups %(exc)s") % + {'exc': e}) + LOG.exception(msg) + raise exception.NetworkError(msg) + + existing_sec_groups = [sec_group['id'] for sec_group in neutron_sec_groups] + missing_sec_groups = set(security_groups) - set(existing_sec_groups) + if missing_sec_groups: + msg = (_('Security Groups specified in Ironic config ' + '%(ir-sg)s are not found') % + {'ir-sg': list(missing_sec_groups)}) + LOG.exception(msg) + raise exception.NetworkError(msg) + + +def add_ports_to_network(task, network_uuid, is_flat=False, + security_groups=None): """Create neutron ports to boot the ramdisk. Create neutron ports for each pxe_enabled port on task.node to boot @@ -78,12 +101,17 @@ def add_ports_to_network(task, network_uuid, is_flat=False): :param network_uuid: UUID of a neutron network where ports will be created. :param is_flat: Indicates whether it is a flat network or not. + :param security_groups: List of Security Groups UUIDs to be used for + network. :raises: NetworkError :returns: a dictionary in the form {port.uuid: neutron_port['id']} """ client = get_client(task.context.auth_token) node = task.node + # If Security Groups are specified, verify that they exist + _verify_security_groups(security_groups, client) + LOG.debug('For node %(node)s, creating neutron ports on network ' '%(network_uuid)s using %(net_iface)s network interface.', {'net_iface': task.driver.network.__class__.__name__, @@ -96,6 +124,8 @@ def add_ports_to_network(task, network_uuid, is_flat=False): 'device_owner': 'baremetal:none', } } + if security_groups: + body['port']['security_groups'] = security_groups if not is_flat: # NOTE(vdrok): It seems that change diff --git a/ironic/conf/neutron.py b/ironic/conf/neutron.py index 4e02f4725d..44fdf4c762 100644 --- a/ironic/conf/neutron.py +++ b/ironic/conf/neutron.py @@ -54,6 +54,22 @@ opts = [ help=_('Neutron network UUID for the ramdisk to be booted ' 'into for provisioning nodes. Required for "neutron" ' 'network interface.')), + cfg.ListOpt('provisioning_network_security_groups', + default=[], + help=_('List of Neutron Security Group UUIDs to be ' + 'applied during provisioning of the nodes. ' + 'Optional for the "neutron" network interface and not ' + 'used for the "flat" or "noop" network interfaces. ' + 'If not specified, default security group ' + 'is used.')), + cfg.ListOpt('cleaning_network_security_groups', + default=[], + help=_('List of Neutron Security Group UUIDs to be ' + 'applied during cleaning of the nodes. ' + 'Optional for the "neutron" network interface and not ' + 'used for the "flat" or "noop" network interfaces. ' + 'If not specified, default security group ' + 'is used.')), ] diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 372e5afbae..301537e720 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -62,7 +62,8 @@ class NeutronNetwork(base.NetworkInterface): LOG.info(_LI('Adding provisioning network to node %s'), task.node.uuid) vifs = neutron.add_ports_to_network( - task, CONF.neutron.provisioning_network_uuid) + task, CONF.neutron.provisioning_network_uuid, + security_groups=CONF.neutron.provisioning_network_security_groups) for port in task.ports: if port.uuid in vifs: internal_info = port.internal_info @@ -97,8 +98,10 @@ class NeutronNetwork(base.NetworkInterface): # If we have left over ports from a previous cleaning, remove them neutron.rollback_ports(task, CONF.neutron.cleaning_network_uuid) LOG.info(_LI('Adding cleaning network to node %s'), task.node.uuid) + security_groups = CONF.neutron.cleaning_network_security_groups vifs = neutron.add_ports_to_network(task, - CONF.neutron.cleaning_network_uuid) + CONF.neutron.cleaning_network_uuid, + security_groups=security_groups) for port in task.ports: if port.uuid in vifs: internal_info = port.internal_info diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index bc9c032691..370896398d 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -136,7 +136,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase): patcher.start() self.addCleanup(patcher.stop) - def _test_add_ports_to_vlan_network(self, is_client_id): + def _test_add_ports_to_vlan_network(self, is_client_id, + security_groups=None): # Ports will be created only if pxe_enabled is True object_utils.create_test_port( self.context, node_id=self.node.id, @@ -164,6 +165,9 @@ class TestNeutronNetworkActions(db_base.DbTestCase): } } } + if security_groups: + expected_body['port']['security_groups'] = security_groups + if is_client_id: expected_body['port']['extra_dhcp_opts'] = ( [{'opt_name': 'client-id', 'opt_value': self._CLIENT_ID}]) @@ -172,13 +176,80 @@ class TestNeutronNetworkActions(db_base.DbTestCase): 'port': self.neutron_port} expected = {port.uuid: self.neutron_port['id']} with task_manager.acquire(self.context, self.node.uuid) as task: - ports = neutron.add_ports_to_network(task, self.network_uuid) + ports = neutron.add_ports_to_network( + task, self.network_uuid, security_groups=security_groups) self.assertEqual(expected, ports) self.client_mock.create_port.assert_called_once_with( expected_body) def test_add_ports_to_vlan_network(self): - self._test_add_ports_to_vlan_network(is_client_id=False) + self._test_add_ports_to_vlan_network(is_client_id=False, + security_groups=None) + + @mock.patch.object(neutron, '_verify_security_groups') + def test_add_ports_to_vlan_network_with_sg(self, verify_mock): + sg_ids = [] + for i in range(2): + sg_ids.append(uuidutils.generate_uuid()) + self._test_add_ports_to_vlan_network(is_client_id=False, + security_groups=sg_ids) + + def test_verify_sec_groups(self): + sg_ids = [] + for i in range(2): + sg_ids.append(uuidutils.generate_uuid()) + + expected_vals = {'security_groups': []} + for sg in sg_ids: + expected_vals['security_groups'].append({'id': sg}) + + client = mock.MagicMock() + client.list_security_groups.return_value = expected_vals + + self.assertIsNone( + neutron._verify_security_groups(sg_ids, client)) + + def test_verify_sec_groups_less_than_configured(self): + sg_ids = [] + for i in range(2): + sg_ids.append(uuidutils.generate_uuid()) + + expected_vals = {'security_groups': []} + for sg in sg_ids: + expected_vals['security_groups'].append({'id': sg}) + + client = mock.MagicMock() + client.list_security_groups.return_value = expected_vals + + self.assertIsNone( + neutron._verify_security_groups(sg_ids[:1], client)) + + def test_verify_sec_groups_more_than_configured(self): + sg_ids = [] + for i in range(1): + sg_ids.append(uuidutils.generate_uuid()) + + client = mock.MagicMock() + expected_vals = {'security_groups': []} + client.list_security_groups.return_value = expected_vals + + self.assertRaises( + exception.NetworkError, + neutron._verify_security_groups, sg_ids, client) + + def test_verify_sec_groups_exception_by_neutronclient(self): + sg_ids = [] + for i in range(2): + sg_ids.append(uuidutils.generate_uuid()) + + client = mock.MagicMock() + client.list_security_groups.side_effect = \ + neutron_client_exc.NeutronClientException + + self.assertRaisesRegex( + exception.NetworkError, + "Could not retrieve neutron security groups", + neutron._verify_security_groups, sg_ids, client) def test_add_ports_with_client_id_to_vlan_network(self): self._test_add_ports_to_vlan_network(is_client_id=True) diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index ebcd3f4289..33b15030d3 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -65,7 +65,31 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): rollback_mock.assert_called_once_with( task, CONF.neutron.provisioning_network_uuid) add_ports_mock.assert_called_once_with( + task, CONF.neutron.provisioning_network_uuid, + security_groups=[]) + self.port.refresh() + self.assertEqual(self.neutron_port['id'], + self.port.internal_info['provisioning_vif_port_id']) + + @mock.patch.object(neutron_common, 'rollback_ports') + @mock.patch.object(neutron_common, 'add_ports_to_network') + def test_add_provisioning_network_with_sg(self, add_ports_mock, + rollback_mock): + sg_ids = [] + for i in range(2): + sg_ids.append(uuidutils.generate_uuid()) + + self.config(provisioning_network_security_groups=sg_ids, + group='neutron') + add_ports_mock.return_value = {self.port.uuid: self.neutron_port['id']} + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.add_provisioning_network(task) + rollback_mock.assert_called_once_with( task, CONF.neutron.provisioning_network_uuid) + add_ports_mock.assert_called_once_with( + task, CONF.neutron.provisioning_network_uuid, + security_groups=( + CONF.neutron.provisioning_network_security_groups)) self.port.refresh() self.assertEqual(self.neutron_port['id'], self.port.internal_info['provisioning_vif_port_id']) @@ -94,6 +118,26 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.assertEqual(self.neutron_port['id'], self.port.internal_info['cleaning_vif_port_id']) + @mock.patch.object(neutron_common, 'rollback_ports') + @mock.patch.object(neutron_common, 'add_ports_to_network') + def test_add_cleaning_network_with_sg(self, add_ports_mock, rollback_mock): + add_ports_mock.return_value = {self.port.uuid: self.neutron_port['id']} + sg_ids = [] + for i in range(2): + sg_ids.append(uuidutils.generate_uuid()) + self.config(cleaning_network_security_groups=sg_ids, group='neutron') + with task_manager.acquire(self.context, self.node.id) as task: + res = self.interface.add_cleaning_network(task) + add_ports_mock.assert_called_once_with( + task, CONF.neutron.cleaning_network_uuid, + security_groups=CONF.neutron.cleaning_network_security_groups) + rollback_mock.assert_called_once_with( + task, CONF.neutron.cleaning_network_uuid) + self.assertEqual(res, add_ports_mock.return_value) + self.port.refresh() + self.assertEqual(self.neutron_port['id'], + self.port.internal_info['cleaning_vif_port_id']) + @mock.patch.object(neutron_common, 'remove_ports_from_network') def test_remove_cleaning_network(self, remove_ports_mock): self.port.internal_info = {'cleaning_vif_port_id': 'vif-port-id'} diff --git a/releasenotes/notes/security_groups-b57a5d6c30c2fae4.yaml b/releasenotes/notes/security_groups-b57a5d6c30c2fae4.yaml new file mode 100644 index 0000000000..996a4c4e27 --- /dev/null +++ b/releasenotes/notes/security_groups-b57a5d6c30c2fae4.yaml @@ -0,0 +1,10 @@ +--- +features: + - Adds support for security groups for the provisioning and cleaning + network. These are optionally specified by the configuration options + ``[neutron]/provisioning_network_security_groups`` and + ``[neutron]/cleaning_network_security_groups``, respectively. + If not specified, + the default security group for the network is used. These options are only + applicable for nodes using the "neutron" network interface. These options + are ignored for nodes using the "flat" and "noop" network interfaces.