From 6f37943af9634ef6a86475402e69c33bc162893e Mon Sep 17 00:00:00 2001 From: Samuel12321 Date: Thu, 18 Jan 2018 16:14:02 +1300 Subject: [PATCH] A description can now be added to a security rule This patch fixes Bug #1742332 (Security Rule Description neither editable nor shown). I have added the option for a description to be added to a security rule which will show the description on the related table. Co-Authored-By: Akihiro Motoki Change-Id: Ie723deb412977ae460c1e897f5d71fc8dbb7a853 Closes-Bug: #1742332 --- openstack_dashboard/api/neutron.py | 27 +-- .../project/security_groups/forms.py | 23 ++- .../project/security_groups/tables.py | 23 +++ .../project/security_groups/tests.py | 182 +++++++++++++++--- .../test/test_data/neutron_data.py | 25 ++- .../test/unit/api/test_neutron.py | 16 +- ...-description-support-37ea7580d3b7c7a9.yaml | 4 + 7 files changed, 256 insertions(+), 44 deletions(-) create mode 100644 releasenotes/notes/sg-rule-description-support-37ea7580d3b7c7a9.yaml diff --git a/openstack_dashboard/api/neutron.py b/openstack_dashboard/api/neutron.py index 22a05c5fe0..fd04562441 100644 --- a/openstack_dashboard/api/neutron.py +++ b/openstack_dashboard/api/neutron.py @@ -248,6 +248,7 @@ class SecurityGroupRule(NeutronAPIDictWrapper): 'ip_protocol': sgr['protocol'], 'from_port': sgr['port_range_min'], 'to_port': sgr['port_range_max'], + 'description': sgr.get('description', '') } cidr = sgr['remote_ip_prefix'] rule['ip_range'] = {'cidr': cidr} if cidr else {} @@ -387,7 +388,7 @@ class SecurityGroupManager(object): def rule_create(self, parent_group_id, direction=None, ethertype=None, ip_protocol=None, from_port=None, to_port=None, - cidr=None, group_id=None): + cidr=None, group_id=None, description=None): """Create a new security group rule. :param parent_group_id: security group id a rule is created to @@ -409,15 +410,17 @@ class SecurityGroupManager(object): if isinstance(ip_protocol, int) and ip_protocol < 0: ip_protocol = None - body = {'security_group_rule': - {'security_group_id': parent_group_id, - 'direction': direction, - 'ethertype': ethertype, - 'protocol': ip_protocol, - 'port_range_min': from_port, - 'port_range_max': to_port, - 'remote_ip_prefix': cidr, - 'remote_group_id': group_id}} + params = {'security_group_id': parent_group_id, + 'direction': direction, + 'ethertype': ethertype, + 'protocol': ip_protocol, + 'port_range_min': from_port, + 'port_range_max': to_port, + 'remote_ip_prefix': cidr, + 'remote_group_id': group_id} + if description is not None: + params['description'] = description + body = {'security_group_rule': params} try: rule = self.client.create_security_group_rule(body) except neutron_exc.OverQuotaClient: @@ -1590,10 +1593,10 @@ def security_group_update(request, sg_id, name, desc): def security_group_rule_create(request, parent_group_id, direction, ethertype, ip_protocol, from_port, to_port, - cidr, group_id): + cidr, group_id, description=None): return SecurityGroupManager(request).rule_create( parent_group_id, direction, ethertype, ip_protocol, - from_port, to_port, cidr, group_id) + from_port, to_port, cidr, group_id, description) def security_group_rule_delete(request, sgr_id): diff --git a/openstack_dashboard/dashboards/project/security_groups/forms.py b/openstack_dashboard/dashboards/project/security_groups/forms.py index 8524138437..58dd2dc4d2 100644 --- a/openstack_dashboard/dashboards/project/security_groups/forms.py +++ b/openstack_dashboard/dashboards/project/security_groups/forms.py @@ -102,6 +102,12 @@ class AddRule(forms.SelfHandlingForm): widget=forms.ThemableSelectWidget(attrs={ 'class': 'switchable', 'data-slug': 'rule_menu'})) + description = forms.CharField( + label=_('Description'), + required=False, max_length=255, + widget=forms.Textarea(attrs={'rows': 2}), + help_text=_('A brief description of the security group rule ' + 'you are adding')) # "direction" field is enabled only when custom mode. # It is because most common rules in local_settings.py is meaningful @@ -296,6 +302,17 @@ class AddRule(forms.SelfHandlingForm): attrs={'readonly': 'readonly'}) self.fields['ethertype'].initial = 'IPv4' + try: + is_desc_supported = api.neutron.is_extension_supported( + self.request, 'standard-attr-description') + except Exception: + exceptions.handle( + self.request, + _('Failed to check if description field is supported.')) + is_desc_supported = False + if not is_desc_supported: + del self.fields['description'] + def _update_and_pop_error(self, cleaned_data, key, value): cleaned_data[key] = value self.errors.pop(key, None) @@ -419,6 +436,9 @@ class AddRule(forms.SelfHandlingForm): def handle(self, request, data): redirect = reverse("horizon:project:security_groups:detail", args=[data['id']]) + params = {} + if 'description' in data: + params['description'] = data['description'] try: rule = api.neutron.security_group_rule_create( request, @@ -429,7 +449,8 @@ class AddRule(forms.SelfHandlingForm): data['from_port'], data['to_port'], data['cidr'], - data['security_group']) + data['security_group'], + **params) messages.success(request, _('Successfully added rule: %s') % six.text_type(rule)) diff --git a/openstack_dashboard/dashboards/project/security_groups/tables.py b/openstack_dashboard/dashboards/project/security_groups/tables.py index 3872116efc..c4386c0b01 100644 --- a/openstack_dashboard/dashboards/project/security_groups/tables.py +++ b/openstack_dashboard/dashboards/project/security_groups/tables.py @@ -12,12 +12,16 @@ # License for the specific language governing permissions and limitations # under the License. +import functools + from django.conf import settings +from django.template import defaultfilters from django.urls import reverse from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ungettext_lazy import six +from horizon import exceptions from horizon import tables from openstack_dashboard import api @@ -228,6 +232,25 @@ class RulesTable(tables.DataTable): remote_security_group = tables.Column(get_remote_security_group, verbose_name=_("Remote Security" " Group")) + description = tables.Column( + "description", + verbose_name=("Description"), + # 'default' filter is to hide the difference between empty string + # and None (null) in description. Both will be displayed as '-'. + filters=(functools.partial(defaultfilters.default, arg=_("-")),)) + + def __init__(self, request, *args, **kwargs): + super(RulesTable, self).__init__(request, *args, **kwargs) + try: + is_desc_supported = api.neutron.is_extension_supported( + self.request, 'standard-attr-description') + except Exception: + exceptions.handle( + self.request, + _('Failed to check if description field is supported.')) + is_desc_supported = False + if not is_desc_supported: + del self.columns['description'] def sanitize_id(self, obj_id): return filters.get_int_or_uuid(obj_id) diff --git a/openstack_dashboard/dashboards/project/security_groups/tests.py b/openstack_dashboard/dashboards/project/security_groups/tests.py index 74b751df8b..e12cdaa3f0 100644 --- a/openstack_dashboard/dashboards/project/security_groups/tests.py +++ b/openstack_dashboard/dashboards/project/security_groups/tests.py @@ -160,11 +160,12 @@ class SecurityGroupsViewTests(test.TestCase): def test_create_button_disabled_when_quota_exceeded_neutron_enabled(self): self._test_create_button_disabled_when_quota_exceeded(True) - def _add_security_group_rule_fixture(self, **kwargs): + def _add_security_group_rule_fixture(self, is_desc_support=True, **kwargs): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = is_desc_support self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -174,6 +175,13 @@ class SecurityGroupsViewTests(test.TestCase): sec_group = self.security_groups.first() rule = self.security_group_rules.first() + extra_params = {} + if 'description' in kwargs: + if kwargs['description'] is not None: + extra_params['description'] = kwargs['description'] + else: + extra_params['description'] = rule.description + self.mock_security_group_rule_create.assert_called_once_with( test.IsHttpRequest(), kwargs.get('sec_group', sec_group.id), @@ -183,9 +191,12 @@ class SecurityGroupsViewTests(test.TestCase): kwargs.get('from_port', int(rule.from_port)), kwargs.get('to_port', int(rule.to_port)), kwargs.get('cidr', rule.ip_range['cidr']), - kwargs.get('security_group', u'%s' % sec_group.id)) + kwargs.get('security_group', u'%s' % sec_group.id), + **extra_params) self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_get',)}) def test_update_security_groups_get(self): @@ -274,25 +285,34 @@ class SecurityGroupsViewTests(test.TestCase): sec_group.name, sec_group.description) - @test.create_mocks({api.neutron: ('security_group_get',)}) + @test.create_mocks({api.neutron: ('security_group_get', + 'is_extension_supported')}) def test_detail_get(self): sec_group = self.security_groups.first() self.mock_security_group_get.return_value = sec_group + self.mock_is_extension_supported.return_value = True res = self.client.get(self.detail_url) self.assertTemplateUsed(res, SG_DETAIL_TEMPLATE) self.mock_security_group_get.assert_called_once_with( test.IsHttpRequest(), sec_group.id) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') - @test.create_mocks({api.neutron: ('security_group_get',)}) + @test.create_mocks({api.neutron: ('security_group_get', + 'is_extension_supported')}) def test_detail_get_exception(self): sec_group = self.security_groups.first() self.mock_security_group_get.side_effect = self.exceptions.nova + self.mock_is_extension_supported.return_value = True res = self.client.get(self.detail_url) self.assertRedirectsNoFollow(res, INDEX_URL) self.mock_security_group_get.assert_called_once_with( test.IsHttpRequest(), sec_group.id) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_cidr(self): sec_group, rule = self._add_security_group_rule_fixture( @@ -300,6 +320,7 @@ class SecurityGroupsViewTests(test.TestCase): formData = {'method': 'AddRule', 'id': sec_group.id, + 'description': rule.description, 'port_or_range': 'port', 'port': rule.from_port, 'rule_menu': rule.ip_protocol, @@ -311,6 +332,7 @@ class SecurityGroupsViewTests(test.TestCase): security_group=None) @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_cidr_with_invalid_unused_fields(self): sec_group, rule = self._add_security_group_rule_fixture( @@ -318,6 +340,7 @@ class SecurityGroupsViewTests(test.TestCase): formData = {'method': 'AddRule', 'id': sec_group.id, + 'description': rule.description, 'port_or_range': 'port', 'port': rule.from_port, 'to_port': 'INVALID', @@ -336,6 +359,7 @@ class SecurityGroupsViewTests(test.TestCase): security_group=None) @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_securitygroup_with_invalid_unused_fields(self): sec_group, rule = self._add_security_group_rule_fixture( @@ -343,6 +367,7 @@ class SecurityGroupsViewTests(test.TestCase): formData = {'method': 'AddRule', 'id': sec_group.id, + 'description': rule.description, 'port_or_range': 'port', 'port': rule.from_port, 'to_port': 'INVALID', @@ -361,6 +386,7 @@ class SecurityGroupsViewTests(test.TestCase): cidr=None, ethertype='') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_icmp_with_invalid_unused_fields(self): sec_group, rule = self._add_security_group_rule_fixture( @@ -368,6 +394,7 @@ class SecurityGroupsViewTests(test.TestCase): formData = {'method': 'AddRule', 'id': sec_group.id, + 'description': rule.description, 'port_or_range': 'port', 'port': 'INVALID', 'to_port': 'INVALID', @@ -386,12 +413,33 @@ class SecurityGroupsViewTests(test.TestCase): ip_protocol='icmp', security_group=None) @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', + 'security_group_list')}) + def test_detail_add_rule_without_description_support(self): + sec_group, rule = self._add_security_group_rule_fixture( + security_group=None, is_desc_support=False) + + formData = {'method': 'AddRule', + 'id': sec_group.id, + 'port_or_range': 'port', + 'port': rule.from_port, + 'rule_menu': rule.ip_protocol, + 'cidr': rule.ip_range['cidr'], + 'remote': 'cidr'} + res = self.client.post(self.edit_url, formData) + self.assertRedirectsNoFollow(res, self.detail_url) + self._check_add_security_group_rule( + security_group=None, description=None) + + @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_cidr_with_template(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -412,9 +460,12 @@ class SecurityGroupsViewTests(test.TestCase): int(rule.from_port), int(rule.to_port), rule.ip_range['cidr'], - None) + None, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') def _get_source_group_rule(self): for rule in self.security_group_rules.list(): @@ -423,12 +474,14 @@ class SecurityGroupsViewTests(test.TestCase): raise Exception("No matches found.") @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_self_as_source_group(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self._get_source_group_rule() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -453,17 +506,22 @@ class SecurityGroupsViewTests(test.TestCase): int(rule.from_port), int(rule.to_port), None, - u'%s' % sec_group.id) + u'%s' % sec_group.id, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_self_as_source_group_with_template(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self._get_source_group_rule() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -487,17 +545,22 @@ class SecurityGroupsViewTests(test.TestCase): int(rule.from_port), int(rule.to_port), None, - u'%s' % sec_group.id) + u'%s' % sec_group.id, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') - @test.create_mocks({api.neutron: ('security_group_list',)}) + @test.create_mocks({api.neutron: ('security_group_list', + 'is_extension_supported')}) def test_detail_invalid_port(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() self.mock_security_group_list.return_value = sec_group_list + self.mock_is_extension_supported.return_value = True formData = {'method': 'AddRule', 'id': sec_group.id, @@ -513,13 +576,18 @@ class SecurityGroupsViewTests(test.TestCase): self.assert_mock_multiple_calls_with_same_arguments( self.mock_security_group_list, 2, mock.call(test.IsHttpRequest())) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_is_extension_supported, 2, + mock.call(test.IsHttpRequest(), 'standard-attr-description')) - @test.create_mocks({api.neutron: ('security_group_list',)}) + @test.create_mocks({api.neutron: ('security_group_list', + 'is_extension_supported')}) def test_detail_invalid_port_range(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = True self.mock_security_group_list.return_value = sec_group_list formData = {'method': 'AddRule', @@ -563,14 +631,19 @@ class SecurityGroupsViewTests(test.TestCase): self.assert_mock_multiple_calls_with_same_arguments( self.mock_security_group_list, 6, mock.call(test.IsHttpRequest())) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_is_extension_supported, 6, + mock.call(test.IsHttpRequest(), 'standard-attr-description')) - @test.create_mocks({api.neutron: ('security_group_list',)}) + @test.create_mocks({api.neutron: ('security_group_list', + 'is_extension_supported')}) def test_detail_invalid_icmp_rule(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() icmp_rule = self.security_group_rules.list()[1] self.mock_security_group_list.return_value = sec_group_list + self.mock_is_extension_supported.return_value = True formData = {'method': 'AddRule', 'id': sec_group.id, @@ -636,14 +709,19 @@ class SecurityGroupsViewTests(test.TestCase): self.assert_mock_multiple_calls_with_same_arguments( self.mock_security_group_list, 10, mock.call(test.IsHttpRequest())) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_is_extension_supported, 10, + mock.call(test.IsHttpRequest(), 'standard-attr-description')) @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_exception(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.side_effect = self.exceptions.nova self.mock_security_group_list.return_value = sec_group_list @@ -664,17 +742,22 @@ class SecurityGroupsViewTests(test.TestCase): int(rule.from_port), int(rule.to_port), rule.ip_range['cidr'], - None) + None, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_duplicated(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.side_effect = exceptions.Conflict self.mock_security_group_list.return_value = sec_group_list @@ -696,15 +779,20 @@ class SecurityGroupsViewTests(test.TestCase): int(rule.from_port), int(rule.to_port), rule.ip_range['cidr'], - None) + None, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') - @test.create_mocks({api.neutron: ('security_group_rule_delete',)}) + @test.create_mocks({api.neutron: ('security_group_rule_delete', + 'is_extension_supported')}) def test_detail_delete_rule(self): sec_group = self.security_groups.first() rule = self.security_group_rules.first() self.mock_security_group_rule_delete.return_value = None + self.mock_is_extension_supported.return_value = True form_data = {"action": "rules__delete__%s" % rule.id} req = self.factory.post(self.edit_url, form_data) @@ -715,12 +803,16 @@ class SecurityGroupsViewTests(test.TestCase): self.detail_url) self.mock_security_group_rule_delete.assert_called_once_with( test.IsHttpRequest(), rule.id) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') - @test.create_mocks({api.neutron: ('security_group_rule_delete',)}) + @test.create_mocks({api.neutron: ('security_group_rule_delete', + 'is_extension_supported')}) def test_detail_delete_rule_exception(self): sec_group = self.security_groups.first() rule = self.security_group_rules.first() self.mock_security_group_rule_delete.side_effect = self.exceptions.nova + self.mock_is_extension_supported.return_value = True form_data = {"action": "rules__delete__%s" % rule.id} req = self.factory.post(self.edit_url, form_data) @@ -732,6 +824,8 @@ class SecurityGroupsViewTests(test.TestCase): self.detail_url) self.mock_security_group_rule_delete.assert_called_once_with( test.IsHttpRequest(), rule.id) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_delete',)}) def test_delete_group(self): @@ -763,12 +857,14 @@ class SecurityGroupsViewTests(test.TestCase): test.IsHttpRequest(), sec_group.id) @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_custom_protocol(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -787,17 +883,21 @@ class SecurityGroupsViewTests(test.TestCase): test.IsHttpRequest(), sec_group.id, 'ingress', 'IPv6', 37, None, None, 'fe80::/48', - None) + None, description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_egress(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -816,17 +916,21 @@ class SecurityGroupsViewTests(test.TestCase): test.IsHttpRequest(), sec_group.id, 'egress', 'IPv4', 'udp', 80, 80, '10.1.1.0/24', - None) + None, description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_egress_with_all_tcp(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.list()[3] + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -847,17 +951,22 @@ class SecurityGroupsViewTests(test.TestCase): int(rule.from_port), int(rule.to_port), rule.ip_range['cidr'], - None) + None, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_source_group_with_direction_ethertype(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self._get_source_group_rule() + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -884,16 +993,21 @@ class SecurityGroupsViewTests(test.TestCase): int(rule.from_port), int(rule.to_port), None, - u'%s' % sec_group.id) + u'%s' % sec_group.id, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.update_settings( OPENSTACK_NEUTRON_NETWORK={'enable_ipv6': False}) - @test.create_mocks({api.neutron: ('security_group_list',)}) + @test.create_mocks({api.neutron: ('security_group_list', + 'is_extension_supported')}) def test_add_rule_ethertype_with_ipv6_disabled(self): self.mock_security_group_list.return_value = \ self.security_groups.list() + self.mock_is_extension_supported.return_value = True res = self.client.get(self.edit_url) @@ -911,14 +1025,18 @@ class SecurityGroupsViewTests(test.TestCase): ) self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.update_settings( OPENSTACK_NEUTRON_NETWORK={'enable_ipv6': False}) - @test.create_mocks({api.neutron: ('security_group_list',)}) + @test.create_mocks({api.neutron: ('security_group_list', + 'is_extension_supported')}) def test_add_rule_cidr_with_ipv6_disabled(self): sec_group = self.security_groups.first() self.mock_security_group_list.return_value = \ self.security_groups.list() + self.mock_is_extension_supported.return_value = True formData = {'method': 'AddRule', 'id': sec_group.id, @@ -935,13 +1053,18 @@ class SecurityGroupsViewTests(test.TestCase): self.assert_mock_multiple_calls_with_same_arguments( self.mock_security_group_list, 2, mock.call(test.IsHttpRequest())) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_is_extension_supported, 2, + mock.call(test.IsHttpRequest(), 'standard-attr-description')) - @test.create_mocks({api.neutron: ('security_group_list',)}) + @test.create_mocks({api.neutron: ('security_group_list', + 'is_extension_supported')}) def test_detail_add_rule_invalid_port(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.first() + self.mock_is_extension_supported.return_value = True self.mock_security_group_list.return_value = sec_group_list formData = {'method': 'AddRule', @@ -958,14 +1081,19 @@ class SecurityGroupsViewTests(test.TestCase): self.assert_mock_multiple_calls_with_same_arguments( self.mock_security_group_list, 2, mock.call(test.IsHttpRequest())) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_is_extension_supported, 2, + mock.call(test.IsHttpRequest(), 'standard-attr-description')) @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_ingress_tcp_without_port(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.list()[3] + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -985,17 +1113,22 @@ class SecurityGroupsViewTests(test.TestCase): None, None, rule.ip_range['cidr'], - None) + None, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') @test.create_mocks({api.neutron: ('security_group_rule_create', + 'is_extension_supported', 'security_group_list')}) def test_detail_add_rule_custom_without_protocol(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() rule = self.security_group_rules.list()[3] + self.mock_is_extension_supported.return_value = True self.mock_security_group_rule_create.return_value = rule self.mock_security_group_list.return_value = sec_group_list @@ -1016,6 +1149,9 @@ class SecurityGroupsViewTests(test.TestCase): None, None, rule.ip_range['cidr'], - None) + None, + description='') self.mock_security_group_list.assert_called_once_with( test.IsHttpRequest()) + self.mock_is_extension_supported.assert_called_once_with( + test.IsHttpRequest(), 'standard-attr-description') diff --git a/openstack_dashboard/test/test_data/neutron_data.py b/openstack_dashboard/test/test_data/neutron_data.py index 7297716740..a0a9db62b3 100644 --- a/openstack_dashboard/test/test_data/neutron_data.py +++ b/openstack_dashboard/test/test_data/neutron_data.py @@ -505,7 +505,9 @@ def data(TEST): 'protocol': None, 'remote_group_id': None, 'remote_ip_prefix': None, 'security_group_id': secgroup['id'], - 'tenant_id': secgroup['tenant_id']} + 'tenant_id': secgroup['tenant_id'], + 'description': "Egrress IPv4 rule", + } rule_egress_ipv6 = { 'id': uuidutils.generate_uuid(), 'direction': u'egress', 'ethertype': u'IPv6', @@ -513,8 +515,9 @@ def data(TEST): 'protocol': None, 'remote_group_id': None, 'remote_ip_prefix': None, 'security_group_id': secgroup['id'], - 'tenant_id': secgroup['tenant_id']} - + 'tenant_id': secgroup['tenant_id'], + 'description': 'Egress IPv6 rule', + } rule_tcp_80 = { 'id': uuidutils.generate_uuid(), 'direction': u'ingress', 'ethertype': u'IPv4', @@ -522,7 +525,9 @@ def data(TEST): 'protocol': u'tcp', 'remote_group_id': None, 'remote_ip_prefix': u'0.0.0.0/0', 'security_group_id': secgroup['id'], - 'tenant_id': secgroup['tenant_id']} + 'tenant_id': secgroup['tenant_id'], + 'description': 'Ingress HTTP', + } rule_icmp = { 'id': uuidutils.generate_uuid(), 'direction': u'ingress', 'ethertype': u'IPv4', @@ -530,7 +535,9 @@ def data(TEST): 'protocol': u'icmp', 'remote_group_id': None, 'remote_ip_prefix': u'0.0.0.0/0', 'security_group_id': secgroup['id'], - 'tenant_id': secgroup['tenant_id']} + 'tenant_id': secgroup['tenant_id'], + 'description': 'Ingress IPv4 ICMP', + } rule_group = { 'id': uuidutils.generate_uuid(), 'direction': u'ingress', 'ethertype': u'IPv4', @@ -538,7 +545,9 @@ def data(TEST): 'protocol': u'tcp', 'remote_group_id': sec_group_1['id'], 'remote_ip_prefix': None, 'security_group_id': secgroup['id'], - 'tenant_id': secgroup['tenant_id']} + 'tenant_id': secgroup['tenant_id'], + 'description': 'Ingress HTTP from SG #1', + } rule_all_tcp = { 'id': uuidutils.generate_uuid(), 'direction': u'egress', 'ethertype': u'IPv4', @@ -546,7 +555,9 @@ def data(TEST): 'protocol': u'tcp', 'remote_group_id': None, 'remote_ip_prefix': u'0.0.0.0/24', 'security_group_id': secgroup['id'], - 'tenant_id': secgroup['tenant_id']} + 'tenant_id': secgroup['tenant_id'], + 'description': 'Egress all TCP', + } rules = [] if not default_only: diff --git a/openstack_dashboard/test/unit/api/test_neutron.py b/openstack_dashboard/test/unit/api/test_neutron.py index 1b457fef79..eb88d2f6bd 100644 --- a/openstack_dashboard/test/unit/api/test_neutron.py +++ b/openstack_dashboard/test/unit/api/test_neutron.py @@ -1138,6 +1138,12 @@ class NeutronApiSecurityGroupTests(test.APIMockTestCase): secgroup['id']) def test_security_group_rule_create(self): + self._test_security_group_rule_create(with_desc=True) + + def test_security_group_rule_create_without_desc(self): + self._test_security_group_rule_create(with_desc=False) + + def _test_security_group_rule_create(self, with_desc): sg_rule = [r for r in self.api_security_group_rules.list() if r['protocol'] == 'tcp' and r['remote_ip_prefix']][0] sg_id = sg_rule['security_group_id'] @@ -1147,17 +1153,25 @@ class NeutronApiSecurityGroupTests(test.APIMockTestCase): post_rule = copy.deepcopy(sg_rule) del post_rule['id'] del post_rule['tenant_id'] + if not with_desc: + del post_rule['description'] post_body = {'security_group_rule': post_rule} self.qclient.create_security_group_rule.return_value = \ {'security_group_rule': copy.deepcopy(sg_rule)} self.qclient.list_security_groups.return_value = \ {'security_groups': [copy.deepcopy(secgroup)]} + if with_desc: + description = sg_rule['description'] + else: + description = None + ret = api.neutron.security_group_rule_create( self.request, sg_rule['security_group_id'], sg_rule['direction'], sg_rule['ethertype'], sg_rule['protocol'], sg_rule['port_range_min'], sg_rule['port_range_max'], - sg_rule['remote_ip_prefix'], sg_rule['remote_group_id']) + sg_rule['remote_ip_prefix'], sg_rule['remote_group_id'], + description) self._cmp_sg_rule(sg_rule, ret) self.qclient.create_security_group_rule.assert_called_once_with( diff --git a/releasenotes/notes/sg-rule-description-support-37ea7580d3b7c7a9.yaml b/releasenotes/notes/sg-rule-description-support-37ea7580d3b7c7a9.yaml new file mode 100644 index 0000000000..f8ecd5e95d --- /dev/null +++ b/releasenotes/notes/sg-rule-description-support-37ea7580d3b7c7a9.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + [:bug:`1742332`] Description for security group rule is supported.