Bug fix for default network ACL entries (#2056)

* Fixed a bug where default network ACL entries could not be deleted.

* Implemented throwing error when a network entry with the same rule number and egress value already exists.

* Fixed syntax errors.

* Added socket.timeout to possibly raised exceptions in wait_for for Python 3.

moto/ec2/exceptions.py

@@ -430,6 +430,15 @@ class OperationNotPermitted(EC2ClientError):
         )
 
 
+class NetworkAclEntryAlreadyExistsError(EC2ClientError):
+
+    def __init__(self, rule_number):
+        super(NetworkAclEntryAlreadyExistsError, self).__init__(
+            "NetworkAclEntryAlreadyExists",
+            "The network acl entry identified by {} already exists.".format(rule_number)
+        )
+
+
 class InvalidSubnetRangeError(EC2ClientError):
 
     def __init__(self, cidr_block):

moto/ec2/models.py

@@ -76,6 +76,7 @@ from .exceptions import (
     MalformedDHCPOptionsIdError,
     MissingParameterError,
     MotoNotImplementedError,
+    NetworkAclEntryAlreadyExistsError,
     OperationNotPermitted,
     OperationNotPermitted2,
     OperationNotPermitted3,
@@ -3664,10 +3665,10 @@ class NetworkAclBackend(object):
 
     def add_default_entries(self, network_acl_id):
         default_acl_entries = [
-            {'rule_number': 100, 'rule_action': 'allow', 'egress': 'true'},
+            {'rule_number': "100", 'rule_action': 'allow', 'egress': 'true'},
-            {'rule_number': 32767, 'rule_action': 'deny', 'egress': 'true'},
+            {'rule_number': "32767", 'rule_action': 'deny', 'egress': 'true'},
-            {'rule_number': 100, 'rule_action': 'allow', 'egress': 'false'},
+            {'rule_number': "100", 'rule_action': 'allow', 'egress': 'false'},
-            {'rule_number': 32767, 'rule_action': 'deny', 'egress': 'false'}
+            {'rule_number': "32767", 'rule_action': 'deny', 'egress': 'false'}
         ]
         for entry in default_acl_entries:
             self.create_network_acl_entry(network_acl_id=network_acl_id, rule_number=entry['rule_number'], protocol='-1',
@@ -3698,12 +3699,14 @@ class NetworkAclBackend(object):
                                  icmp_code, icmp_type, port_range_from,
                                  port_range_to):
 
+        network_acl = self.get_network_acl(network_acl_id)
+        if any(entry.egress == egress and entry.rule_number == rule_number for entry in network_acl.network_acl_entries):
+            raise NetworkAclEntryAlreadyExistsError(rule_number)
         network_acl_entry = NetworkAclEntry(self, network_acl_id, rule_number,
                                             protocol, rule_action, egress,
                                             cidr_block, icmp_code, icmp_type,
                                             port_range_from, port_range_to)
 
-        network_acl = self.get_network_acl(network_acl_id)
         network_acl.network_acl_entries.append(network_acl_entry)
         return network_acl_entry
 

tests/test_ec2/test_network_acls.py

@@ -2,6 +2,8 @@ from __future__ import unicode_literals
 import boto
 import boto3
 import sure  # noqa
+from nose.tools import assert_raises
+from botocore.exceptions import ClientError
 
 from moto import mock_ec2_deprecated, mock_ec2
 
@@ -214,3 +216,37 @@ def test_default_network_acl_default_entries():
             unique_entries.append(entry)
 
     unique_entries.should.have.length_of(4)
+
+
+@mock_ec2
+def test_delete_default_network_acl_default_entry():
+    ec2 = boto3.resource('ec2', region_name='us-west-1')
+    default_network_acl = next(iter(ec2.network_acls.all()), None)
+    default_network_acl.is_default.should.be.ok
+
+    default_network_acl.entries.should.have.length_of(4)
+    first_default_network_acl_entry = default_network_acl.entries[0]
+
+    default_network_acl.delete_entry(Egress=first_default_network_acl_entry['Egress'],
+                                     RuleNumber=first_default_network_acl_entry['RuleNumber'])
+
+    default_network_acl.entries.should.have.length_of(3)
+
+
+@mock_ec2
+def test_duplicate_network_acl_entry():
+    ec2 = boto3.resource('ec2', region_name='us-west-1')
+    default_network_acl = next(iter(ec2.network_acls.all()), None)
+    default_network_acl.is_default.should.be.ok
+
+    rule_number = 200
+    egress = True
+    default_network_acl.create_entry(CidrBlock="0.0.0.0/0", Egress=egress, Protocol="-1", RuleAction="allow", RuleNumber=rule_number)
+
+    with assert_raises(ClientError) as ex:
+        default_network_acl.create_entry(CidrBlock="10.0.0.0/0", Egress=egress, Protocol="-1", RuleAction="deny", RuleNumber=rule_number)
+    str(ex.exception).should.equal(
+        "An error occurred (NetworkAclEntryAlreadyExists) when calling the CreateNetworkAclEntry "
+        "operation: The network acl entry identified by {} already exists.".format(rule_number))
+
+

wait_for.py

@@ -12,8 +12,9 @@ except ImportError:
     # py3
     import urllib.request as urllib
     from urllib.error import URLError
+    import socket
 
-    EXCEPTIONS = (URLError, ConnectionResetError)
+    EXCEPTIONS = (URLError, socket.timeout, ConnectionResetError)
 
 
 start_ts = time.time()