From 2bae7e4e0dec352a6d7e2728d3c1d67c3b649b2c Mon Sep 17 00:00:00 2001
From: Brian Pandola <bpandola@gmail.com>
Date: Fri, 26 Mar 2021 04:23:07 -0700
Subject: [PATCH] Raise error when adding duplicate egress rule to
 ec2:SecurityGroup (#3801)

The `InvalidPermission.Duplicate` error was already implemented for inbound rules,
but AWS also returns this error for duplicate outbound rules.

Very minor changes were needed on existing tests that were adding duplicate
outbound rules (when testing the RulesPerSecurityGroupLimitExceeded error).
---
 moto/ec2/models.py                     |  5 +++--
 tests/test_ec2/test_security_groups.py | 27 ++++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/moto/ec2/models.py b/moto/ec2/models.py
index 78b66f44..3aba5f89 100644
--- a/moto/ec2/models.py
+++ b/moto/ec2/models.py
@@ -2151,10 +2151,11 @@ class SecurityGroup(TaggedEC2Resource, CloudFormationModel):
     def add_ingress_rule(self, rule):
         if rule in self.ingress_rules:
             raise InvalidPermissionDuplicateError()
-        else:
-            self.ingress_rules.append(rule)
+        self.ingress_rules.append(rule)
 
     def add_egress_rule(self, rule):
+        if rule in self.egress_rules:
+            raise InvalidPermissionDuplicateError()
         self.egress_rules.append(rule)
 
     def get_number_of_ingress_rules(self):
diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py
index 6558ca6d..2a7111f9 100644
--- a/tests/test_ec2/test_security_groups.py
+++ b/tests/test_ec2/test_security_groups.py
@@ -574,7 +574,7 @@ def test_sec_group_rule_limit():
     # fill the rules up the limit
     # remember that by default, when created a sec group contains 1 egress rule
     # so our other_sg rule + 98 CIDR IP rules + 1 by default == 100 the limit
-    for i in range(98):
+    for i in range(1, 99):
         ec2_conn.authorize_security_group_egress(
             group_id=sg.id, ip_protocol="-1", cidr_ip="{0}.0.0.0/0".format(i)
         )
@@ -645,7 +645,7 @@ def test_sec_group_rule_limit_vpc():
     # fill the rules up the limit
     # remember that by default, when created a sec group contains 1 egress rule
     # so our other_sg rule + 48 CIDR IP rules + 1 by default == 50 the limit
-    for i in range(48):
+    for i in range(1, 49):
         ec2_conn.authorize_security_group_egress(
             group_id=sg.id, ip_protocol="-1", cidr_ip="{0}.0.0.0/0".format(i)
         )
@@ -677,6 +677,7 @@ def test_add_same_rule_twice_throws_error():
         GroupName="sg1", Description="Test security group sg1", VpcId=vpc.id
     )
 
+    # Ingress
     ip_permissions = [
         {
             "IpProtocol": "tcp",
@@ -689,6 +690,28 @@ def test_add_same_rule_twice_throws_error():
 
     with pytest.raises(ClientError) as ex:
         sg.authorize_ingress(IpPermissions=ip_permissions)
+    ex.value.response["Error"]["Code"].should.equal("InvalidPermission.Duplicate")
+    ex.value.response["Error"]["Message"].should.match(
+        r"^.* specified rule.*already exists$"
+    )
+
+    # Egress
+    ip_permissions = [
+        {
+            "IpProtocol": "-1",
+            "IpRanges": [{"CidrIp": "0.0.0.0/0"}],
+            "Ipv6Ranges": [],
+            "PrefixListIds": [],
+            "UserIdGroupPairs": [],
+        }
+    ]
+
+    with pytest.raises(ClientError) as ex:
+        sg.authorize_egress(IpPermissions=ip_permissions)
+    ex.value.response["Error"]["Code"].should.equal("InvalidPermission.Duplicate")
+    ex.value.response["Error"]["Message"].should.match(
+        r"^.* specified rule.*already exists$"
+    )
 
 
 @mock_ec2