Try to be more verbose on sec group error

Try to identify if the error is due to the security rule
already existing and return the proper message in that case to
not confuse the user.

Change-Id: I13346611b9d7309f84a5bfba8b69ea5e65d0a02a
Closes-Bug: 1298460

openstack_dashboard/api/neutron.py

@@ -30,6 +30,7 @@ from neutronclient.common import exceptions as neutron_exc
 from neutronclient.v2_0 import client as neutron_client
 import six
 
+from horizon import exceptions
 from horizon import messages
 from horizon.utils.memoized import memoized  # noqa
 from openstack_dashboard.api import base
@@ -297,7 +298,10 @@ class SecurityGroupManager(network_base.SecurityGroupManager):
                  'port_range_max': to_port,
                  'remote_ip_prefix': cidr,
                  'remote_group_id': group_id}}
-        rule = self.client.create_security_group_rule(body)
+        try:
+            rule = self.client.create_security_group_rule(body)
+        except neutron_exc.Conflict:
+            raise exceptions.Conflict(_('Security group rule already exists.'))
         rule = rule.get('security_group_rule')
         sg_dict = self._sg_name_dict(parent_group_id, [rule])
         return SecurityGroupRule(rule, sg_dict)

openstack_dashboard/api/nova.py

@@ -286,12 +286,16 @@ class SecurityGroupManager(network_base.SecurityGroupManager):
                     ip_protocol=None, from_port=None, to_port=None,
                     cidr=None, group_id=None):
         # Nova Security Group API does not use direction and ethertype fields.
-        sg = self.client.security_group_rules.create(parent_group_id,
+        try:
-                                                     ip_protocol,
+            sg = self.client.security_group_rules.create(parent_group_id,
-                                                     from_port,
+                                                         ip_protocol,
-                                                     to_port,
+                                                         from_port,
-                                                     cidr,
+                                                         to_port,
-                                                     group_id)
+                                                         cidr,
+                                                         group_id)
+        except nova_exceptions.BadRequest:
+            raise horizon_exceptions.Conflict(
+                _('Security group rule already exists.'))
         return SecurityGroupRule(sg)
 
     def rule_delete(self, security_group_rule_id):

openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py

@@ -390,6 +390,8 @@ class AddRule(forms.SelfHandlingForm):
         return cleaned_data
 
     def handle(self, request, data):
+        redirect = reverse("horizon:project:access_and_security:"
+                           "security_groups:detail", args=[data['id']])
         try:
             rule = api.network.security_group_rule_create(
                 request,
@@ -405,9 +407,9 @@ class AddRule(forms.SelfHandlingForm):
                              _('Successfully added rule: %s')
                              % six.text_type(rule))
             return rule
+        except exceptions.Conflict as error:
+            exceptions.handle(request, error, redirect=redirect)
         except Exception:
-            redirect = reverse("horizon:project:access_and_security:"
-                               "security_groups:detail", args=[data['id']])
             exceptions.handle(request,
                               _('Unable to add rule to security group.'),
                               redirect=redirect)

openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py

@@ -24,6 +24,8 @@ from django import http
 
 from mox3.mox import IsA  # noqa
 
+from horizon import exceptions
+
 from openstack_dashboard import api
 from openstack_dashboard.test import helpers as test
 
@@ -570,6 +572,39 @@ class SecurityGroupsViewTests(test.TestCase):
         res = self.client.post(self.edit_url, formData)
         self.assertRedirectsNoFollow(res, self.detail_url)
 
+    @test.create_stubs({api.network: ('security_group_rule_create',
+                                      'security_group_list',
+                                      'security_group_backend')})
+    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()
+
+        api.network.security_group_backend(
+            IsA(http.HttpRequest)).AndReturn(self.secgroup_backend)
+        api.network.security_group_rule_create(
+            IsA(http.HttpRequest),
+            sec_group.id, 'ingress', 'IPv4',
+            rule.ip_protocol,
+            int(rule.from_port),
+            int(rule.to_port),
+            rule.ip_range['cidr'],
+            None).AndRaise(exceptions.Conflict)
+        api.network.security_group_list(
+            IsA(http.HttpRequest)).AndReturn(sec_group_list)
+        self.mox.ReplayAll()
+
+        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.assertNoFormErrors(res)
+        self.assertRedirectsNoFollow(res, self.detail_url)
+
     @test.create_stubs({api.network: ('security_group_rule_delete',)})
     def test_detail_delete_rule(self):
         sec_group = self.security_groups.first()