From 21917c4b936113e09133f21a988ff60d4d8e832d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20=C3=81cs?= <30595431+acsbendi@users.noreply.github.com> Date: Sun, 26 May 2019 03:02:14 +0200 Subject: [PATCH] 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 | 9 ++++++++ moto/ec2/models.py | 13 +++++++---- tests/test_ec2/test_network_acls.py | 36 +++++++++++++++++++++++++++++ wait_for.py | 3 ++- 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/moto/ec2/exceptions.py b/moto/ec2/exceptions.py index d93ced16..1357d49e 100644 --- a/moto/ec2/exceptions.py +++ b/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): diff --git a/moto/ec2/models.py b/moto/ec2/models.py index c2e5970b..b894853d 100644 --- a/moto/ec2/models.py +++ b/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': 32767, 'rule_action': 'deny', 'egress': 'true'}, - {'rule_number': 100, 'rule_action': 'allow', 'egress': 'false'}, - {'rule_number': 32767, 'rule_action': 'deny', 'egress': 'false'} + {'rule_number': "100", 'rule_action': 'allow', 'egress': 'true'}, + {'rule_number': "32767", 'rule_action': 'deny', 'egress': 'true'}, + {'rule_number': "100", 'rule_action': 'allow', '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 diff --git a/tests/test_ec2/test_network_acls.py b/tests/test_ec2/test_network_acls.py index 5b7f107d..d4c330f0 100644 --- a/tests/test_ec2/test_network_acls.py +++ b/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)) + + diff --git a/wait_for.py b/wait_for.py index d313ea5a..1f291c16 100755 --- a/wait_for.py +++ b/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()