Infer nova-net security groups better
If a cloud does not have neutron, it's entirely possible to infer that security groups should go through nova. While it's currently possible to set secgroup_source: nova in clouds.yaml or in a vendor file, it's a little overkill for folks who otherwise do not need to express that level of complexity. Change-Id: I8e4dec2cf993a47c8a0d5cb55bbf80b0fdc4dd46
This commit is contained in:
Monty Taylor
parent
e5ac401ce7
commit
bd3f07140d
|
|
@ -0,0 +1,9 @@
|
|||
---
|
||||
features:
|
||||
- If a cloud does not have a neutron service, it is now
|
||||
assumed that Nova will be the source of security groups.
|
||||
To handle clouds that have nova-network and do not have
|
||||
the security group extension, setting secgroup_source to
|
||||
None will prevent attempting to use them at all. If the
|
||||
cloud has neutron but it is not a functional source of
|
||||
security groups, set secgroup_source to nova.
|
||||
|
|
@ -1414,7 +1414,7 @@ class OpenStackCloud(object):
|
|||
"""
|
||||
|
||||
# Don't even try if we're a cloud that doesn't have them
|
||||
if self.secgroup_source not in ('nova', 'neutron'):
|
||||
if not self._has_secgroups():
|
||||
return []
|
||||
|
||||
with _utils.shade_exceptions():
|
||||
|
|
@ -1429,8 +1429,14 @@ class OpenStackCloud(object):
|
|||
:returns: A list of security group ``munch.Munch``.
|
||||
|
||||
"""
|
||||
# Security groups not supported
|
||||
if not self._has_secgroups():
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
# Handle neutron security groups
|
||||
if self.secgroup_source == 'neutron':
|
||||
if self._use_neutron_secgroups():
|
||||
# Neutron returns dicts, so no need to convert objects here.
|
||||
with _utils.neutron_exceptions(
|
||||
"Error fetching security group list"):
|
||||
|
|
@ -1438,18 +1444,12 @@ class OpenStackCloud(object):
|
|||
_tasks.NeutronSecurityGroupList())['security_groups']
|
||||
|
||||
# Handle nova security groups
|
||||
elif self.secgroup_source == 'nova':
|
||||
else:
|
||||
with _utils.shade_exceptions("Error fetching security group list"):
|
||||
groups = self.manager.submitTask(
|
||||
_tasks.NovaSecurityGroupList())
|
||||
return _utils.normalize_nova_secgroups(groups)
|
||||
|
||||
# Security groups not supported
|
||||
else:
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
def list_servers(self, detailed=False):
|
||||
"""List all available servers.
|
||||
|
||||
|
|
@ -1776,6 +1776,16 @@ class OpenStackCloud(object):
|
|||
return (self.has_service('network')
|
||||
and self._floating_ip_source == 'neutron')
|
||||
|
||||
def _has_secgroups(self):
|
||||
if not self.secgroup_source:
|
||||
return False
|
||||
else:
|
||||
return self.secgroup_source.lower() in ('nova', 'neutron')
|
||||
|
||||
def _use_neutron_secgroups(self):
|
||||
return (self.has_service('network')
|
||||
and self.secgroup_source == 'neutron')
|
||||
|
||||
def get_keypair(self, name_or_id, filters=None):
|
||||
"""Get a keypair by name or ID.
|
||||
|
||||
|
|
@ -5244,7 +5254,14 @@ class OpenStackCloud(object):
|
|||
:raises: OpenStackCloudUnavailableFeature if security groups are
|
||||
not supported on this cloud.
|
||||
"""
|
||||
if self.secgroup_source == 'neutron':
|
||||
|
||||
# Security groups not supported
|
||||
if not self._has_secgroups():
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
if self._use_neutron_secgroups():
|
||||
with _utils.neutron_exceptions(
|
||||
"Error creating security group {0}".format(name)):
|
||||
group = self.manager.submitTask(
|
||||
|
|
@ -5255,7 +5272,7 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return group['security_group']
|
||||
|
||||
elif self.secgroup_source == 'nova':
|
||||
else:
|
||||
with _utils.shade_exceptions(
|
||||
"Failed to create security group '{name}'".format(
|
||||
name=name)):
|
||||
|
|
@ -5266,12 +5283,6 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return _utils.normalize_nova_secgroups([group])[0]
|
||||
|
||||
# Security groups not supported
|
||||
else:
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
def delete_security_group(self, name_or_id):
|
||||
"""Delete a security group
|
||||
|
||||
|
|
@ -5283,13 +5294,19 @@ class OpenStackCloud(object):
|
|||
:raises: OpenStackCloudUnavailableFeature if security groups are
|
||||
not supported on this cloud.
|
||||
"""
|
||||
# Security groups not supported
|
||||
if not self._has_secgroups():
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
secgroup = self.get_security_group(name_or_id)
|
||||
if secgroup is None:
|
||||
self.log.debug('Security group %s not found for deleting' %
|
||||
name_or_id)
|
||||
return False
|
||||
|
||||
if self.secgroup_source == 'neutron':
|
||||
if self._use_neutron_secgroups():
|
||||
with _utils.neutron_exceptions(
|
||||
"Error deleting security group {0}".format(name_or_id)):
|
||||
self.manager.submitTask(
|
||||
|
|
@ -5299,7 +5316,7 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return True
|
||||
|
||||
elif self.secgroup_source == 'nova':
|
||||
else:
|
||||
with _utils.shade_exceptions(
|
||||
"Failed to delete security group '{group}'".format(
|
||||
group=name_or_id)):
|
||||
|
|
@ -5308,12 +5325,6 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return True
|
||||
|
||||
# Security groups not supported
|
||||
else:
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
@_utils.valid_kwargs('name', 'description')
|
||||
def update_security_group(self, name_or_id, **kwargs):
|
||||
"""Update a security group
|
||||
|
|
@ -5326,13 +5337,19 @@ class OpenStackCloud(object):
|
|||
|
||||
:raises: OpenStackCloudException on operation error.
|
||||
"""
|
||||
# Security groups not supported
|
||||
if not self._has_secgroups():
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
secgroup = self.get_security_group(name_or_id)
|
||||
|
||||
if secgroup is None:
|
||||
raise OpenStackCloudException(
|
||||
"Security group %s not found." % name_or_id)
|
||||
|
||||
if self.secgroup_source == 'neutron':
|
||||
if self._use_neutron_secgroups():
|
||||
with _utils.neutron_exceptions(
|
||||
"Error updating security group {0}".format(name_or_id)):
|
||||
group = self.manager.submitTask(
|
||||
|
|
@ -5342,7 +5359,7 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return group['security_group']
|
||||
|
||||
elif self.secgroup_source == 'nova':
|
||||
else:
|
||||
with _utils.shade_exceptions(
|
||||
"Failed to update security group '{group}'".format(
|
||||
group=name_or_id)):
|
||||
|
|
@ -5352,12 +5369,6 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return _utils.normalize_nova_secgroups([group])[0]
|
||||
|
||||
# Security groups not supported
|
||||
else:
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
def create_security_group_rule(self,
|
||||
secgroup_name_or_id,
|
||||
port_range_min=None,
|
||||
|
|
@ -5409,13 +5420,18 @@ class OpenStackCloud(object):
|
|||
|
||||
:raises: OpenStackCloudException on operation error.
|
||||
"""
|
||||
# Security groups not supported
|
||||
if not self._has_secgroups():
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
secgroup = self.get_security_group(secgroup_name_or_id)
|
||||
if not secgroup:
|
||||
raise OpenStackCloudException(
|
||||
"Security group %s not found." % secgroup_name_or_id)
|
||||
|
||||
if self.secgroup_source == 'neutron':
|
||||
if self._use_neutron_secgroups():
|
||||
# NOTE: Nova accepts -1 port numbers, but Neutron accepts None
|
||||
# as the equivalent value.
|
||||
rule_def = {
|
||||
|
|
@ -5439,7 +5455,7 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return rule['security_group_rule']
|
||||
|
||||
elif self.secgroup_source == 'nova':
|
||||
else:
|
||||
# NOTE: Neutron accepts None for protocol. Nova does not.
|
||||
if protocol is None:
|
||||
raise OpenStackCloudException('Protocol must be specified')
|
||||
|
|
@ -5482,12 +5498,6 @@ class OpenStackCloud(object):
|
|||
)
|
||||
return _utils.normalize_nova_secgroup_rules([rule])[0]
|
||||
|
||||
# Security groups not supported
|
||||
else:
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
def delete_security_group_rule(self, rule_id):
|
||||
"""Delete a security group rule
|
||||
|
||||
|
|
@ -5499,8 +5509,13 @@ class OpenStackCloud(object):
|
|||
:raises: OpenStackCloudUnavailableFeature if security groups are
|
||||
not supported on this cloud.
|
||||
"""
|
||||
# Security groups not supported
|
||||
if not self._has_secgroups():
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
if self.secgroup_source == 'neutron':
|
||||
if self._use_neutron_secgroups():
|
||||
try:
|
||||
with _utils.neutron_exceptions(
|
||||
"Error deleting security group rule "
|
||||
|
|
@ -5513,7 +5528,7 @@ class OpenStackCloud(object):
|
|||
return False
|
||||
return True
|
||||
|
||||
elif self.secgroup_source == 'nova':
|
||||
else:
|
||||
try:
|
||||
self.manager.submitTask(
|
||||
_tasks.NovaSecurityGroupRuleDelete(rule=rule_id)
|
||||
|
|
@ -5528,12 +5543,6 @@ class OpenStackCloud(object):
|
|||
id=rule_id, msg=str(e)))
|
||||
return True
|
||||
|
||||
# Security groups not supported
|
||||
else:
|
||||
raise OpenStackCloudUnavailableFeature(
|
||||
"Unavailable feature: security groups"
|
||||
)
|
||||
|
||||
def list_zones(self):
|
||||
"""List all available zones.
|
||||
|
||||
|
|
|
|||
|
|
@ -55,6 +55,14 @@ nova_grp_dict = meta.obj_to_dict(nova_grp_obj)
|
|||
|
||||
class TestSecurityGroups(base.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestSecurityGroups, self).setUp()
|
||||
self.has_neutron = True
|
||||
|
||||
def fake_has_service(*args, **kwargs):
|
||||
return self.has_neutron
|
||||
self.cloud.has_service = fake_has_service
|
||||
|
||||
@mock.patch.object(shade.OpenStackCloud, 'neutron_client')
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_list_security_groups_neutron(self, mock_nova, mock_neutron):
|
||||
|
|
@ -67,6 +75,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_list_security_groups_nova(self, mock_nova, mock_neutron):
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
self.has_neutron = False
|
||||
self.cloud.list_security_groups()
|
||||
self.assertFalse(mock_neutron.list_security_groups.called)
|
||||
self.assertTrue(mock_nova.security_groups.list.called)
|
||||
|
|
@ -75,6 +84,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_list_security_groups_none(self, mock_nova, mock_neutron):
|
||||
self.cloud.secgroup_source = None
|
||||
self.has_neutron = False
|
||||
self.assertRaises(shade.OpenStackCloudUnavailableFeature,
|
||||
self.cloud.list_security_groups)
|
||||
self.assertFalse(mock_neutron.list_security_groups.called)
|
||||
|
|
@ -93,6 +103,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_delete_security_group_nova(self, mock_nova):
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
self.has_neutron = False
|
||||
nova_return = [nova_grp_obj]
|
||||
mock_nova.security_groups.list.return_value = nova_return
|
||||
self.cloud.delete_security_group('2')
|
||||
|
|
@ -111,6 +122,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_delete_security_group_nova_not_found(self, mock_nova):
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
self.has_neutron = False
|
||||
nova_return = [nova_grp_obj]
|
||||
mock_nova.security_groups.list.return_value = nova_return
|
||||
self.cloud.delete_security_group('doesNotExist')
|
||||
|
|
@ -140,6 +152,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_create_security_group_nova(self, mock_nova):
|
||||
group_name = self.getUniqueString()
|
||||
self.has_neutron = False
|
||||
group_desc = 'security group from test_create_security_group_neutron'
|
||||
new_group = fakes.FakeSecgroup(id='2',
|
||||
name=group_name,
|
||||
|
|
@ -159,6 +172,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_create_security_group_none(self, mock_nova, mock_neutron):
|
||||
self.cloud.secgroup_source = None
|
||||
self.has_neutron = False
|
||||
self.assertRaises(shade.OpenStackCloudUnavailableFeature,
|
||||
self.cloud.create_security_group,
|
||||
'', '')
|
||||
|
|
@ -178,6 +192,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_update_security_group_nova(self, mock_nova):
|
||||
self.has_neutron = False
|
||||
new_name = self.getUniqueString()
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
nova_return = [nova_grp_obj]
|
||||
|
|
@ -228,6 +243,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'get_security_group')
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_create_security_group_rule_nova(self, mock_nova, mock_get):
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
|
||||
new_rule = fakes.FakeNovaSecgroupRule(
|
||||
|
|
@ -249,6 +265,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_create_security_group_rule_nova_no_ports(self,
|
||||
mock_nova, mock_get):
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
|
||||
new_rule = fakes.FakeNovaSecgroupRule(
|
||||
|
|
@ -269,6 +286,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'neutron_client')
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_create_security_group_rule_none(self, mock_nova, mock_neutron):
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = None
|
||||
self.assertRaises(shade.OpenStackCloudUnavailableFeature,
|
||||
self.cloud.create_security_group_rule,
|
||||
|
|
@ -286,6 +304,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_delete_security_group_rule_nova(self, mock_nova):
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
r = self.cloud.delete_security_group_rule('xyz')
|
||||
mock_nova.security_group_rules.delete.assert_called_once_with(
|
||||
|
|
@ -295,6 +314,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade.OpenStackCloud, 'neutron_client')
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_delete_security_group_rule_none(self, mock_nova, mock_neutron):
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = None
|
||||
self.assertRaises(shade.OpenStackCloudUnavailableFeature,
|
||||
self.cloud.delete_security_group_rule,
|
||||
|
|
@ -314,6 +334,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
r = self.cloud.delete_security_group('doesNotExist')
|
||||
self.assertFalse(r)
|
||||
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
mock_neutron.security_group_rules.delete.side_effect = (
|
||||
nova_exc.NotFound("uh oh")
|
||||
|
|
@ -323,6 +344,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_nova_egress_security_group_rule(self, mock_nova):
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = 'nova'
|
||||
mock_nova.security_groups.list.return_value = [nova_grp_obj]
|
||||
self.assertRaises(shade.OpenStackCloudException,
|
||||
|
|
@ -333,6 +355,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
@mock.patch.object(shade._utils, 'normalize_nova_secgroups')
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_list_server_security_groups(self, mock_nova, mock_norm):
|
||||
self.has_neutron = False
|
||||
server = dict(id='server_id')
|
||||
self.cloud.list_server_security_groups(server)
|
||||
mock_nova.servers.list_security_group.assert_called_once_with(
|
||||
|
|
@ -342,6 +365,7 @@ class TestSecurityGroups(base.TestCase):
|
|||
|
||||
@mock.patch.object(shade.OpenStackCloud, 'nova_client')
|
||||
def test_list_server_security_groups_bad_source(self, mock_nova):
|
||||
self.has_neutron = False
|
||||
self.cloud.secgroup_source = 'invalid'
|
||||
server = dict(id='server_id')
|
||||
ret = self.cloud.list_server_security_groups(server)
|
||||
|
|
|
|||
Loading…
Reference in New Issue