From 1480f8b44ae5f9b9af7e7bee376b628d593c67ee Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 20 Mar 2014 16:22:37 -0700 Subject: [PATCH 1/5] test which verifies authorizing rules within a VPC is broken see issue #108 --- tests/test_ec2/test_security_groups.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index 9b1f34964..24e25f8b8 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -20,6 +20,7 @@ def test_create_and_describe_security_group(): all_groups.should.have.length_of(1) all_groups[0].name.should.equal('test security group') + @mock_ec2 def test_create_and_describe_vpc_security_group(): conn = boto.connect_ec2('the_key', 'the_secret') @@ -130,3 +131,16 @@ def test_authorize_other_group_and_revoke(): security_group = [group for group in conn.get_all_security_groups() if group.name == 'test'][0] security_group.rules.should.have.length_of(0) + +@mock_ec2 +def test_authorize_ip_in_vpc(): + conn = boto.connect_ec2('the_key', 'the_secret') + vpc_id = "vpc-12345" + + # create 2 groups in a vpc + security_group1 = conn.create_security_group('test1', 'test1', vpc_id) + security_group2 = conn.create_security_group('test2', 'test2', vpc_id) + + success = security_group1.authorize(ip_protocol="tcp", from_port="22", to_port="2222", src_group=security_group2) + + From cbdc8ba183359b67d140059959dc399e3acf89b5 Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 20 Mar 2014 17:26:08 -0700 Subject: [PATCH 2/5] We're getting back the correct group from get_security_group_from_id, but hitting another issue with the source_group_name also using an id rather than a name --- moto/ec2/models.py | 22 +++++++++++++++++++--- moto/ec2/responses/security_groups.py | 12 ++++++++++-- tests/test_ec2/test_security_groups.py | 3 ++- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 3eaa1a4d1..a1165efa9 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -373,6 +373,16 @@ class SecurityGroupBackend(object): if group: return self.groups[None].pop(group.id) + def get_security_group_from_id(self, group_id): + # 2 levels of chaining necessary since it's a complex structure + all_groups = itertools.chain.from_iterable([x.values() for x in self.groups.values()]) + + for group in itertools.chain(all_groups): + if group.id == group_id: + return group + + + def get_security_group_from_name(self, name, vpc_id): for group_id, group in self.groups[vpc_id].iteritems(): if group.name == name: @@ -383,8 +393,14 @@ class SecurityGroupBackend(object): default_group = ec2_backend.create_security_group("default", "The default security group", force=True) return default_group - def authorize_security_group_ingress(self, group_name, ip_protocol, from_port, to_port, ip_ranges=None, source_group_names=None, vpc_id=None): - group = self.get_security_group_from_name(group_name, vpc_id) + def authorize_security_group_ingress(self, group_name, group_id, ip_protocol, from_port, to_port, ip_ranges=None, source_group_names=None, vpc_id=None): + # to auth a group in a VPC you need the group_id the name isn't enough + + if group_name: + group = self.get_security_group_from_name(group_name, vpc_id) + elif group_id: + group = self.get_security_group_from_id(group_id) + source_groups = [] for source_group_name in source_group_names: source_group = self.get_security_group_from_name(source_group_name, vpc_id) @@ -394,7 +410,7 @@ class SecurityGroupBackend(object): security_rule = SecurityRule(ip_protocol, from_port, to_port, ip_ranges, source_groups) group.ingress_rules.append(security_rule) - def revoke_security_group_ingress(self, group_name, ip_protocol, from_port, to_port, ip_ranges=None, source_group_names=None, vpc_id=None): + def revoke_security_group_ingress(self, group_name, group_id, ip_protocol, from_port, to_port, ip_ranges=None, source_group_names=None, vpc_id=None): group = self.get_security_group_from_name(group_name, vpc_id) source_groups = [] for source_group_name in source_group_names: diff --git a/moto/ec2/responses/security_groups.py b/moto/ec2/responses/security_groups.py index 163faa1d9..b92235322 100644 --- a/moto/ec2/responses/security_groups.py +++ b/moto/ec2/responses/security_groups.py @@ -5,7 +5,15 @@ from moto.ec2.models import ec2_backend def process_rules_from_querystring(querystring): - name = querystring.get('GroupName')[0] + + name = None + group_id = None + + try: + name = querystring.get('GroupName')[0] + except: + group_id = querystring.get('GroupId')[0] + ip_protocol = querystring.get('IpPermissions.1.IpProtocol')[0] from_port = querystring.get('IpPermissions.1.FromPort')[0] to_port = querystring.get('IpPermissions.1.ToPort')[0] @@ -18,7 +26,7 @@ def process_rules_from_querystring(querystring): for key, value in querystring.iteritems(): if 'IpPermissions.1.Groups' in key: source_groups.append(value[0]) - return (name, ip_protocol, from_port, to_port, ip_ranges, source_groups) + return (name, group_id, ip_protocol, from_port, to_port, ip_ranges, source_groups) class SecurityGroups(BaseResponse): diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index 24e25f8b8..fe330e17f 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -133,7 +133,7 @@ def test_authorize_other_group_and_revoke(): security_group.rules.should.have.length_of(0) @mock_ec2 -def test_authorize_ip_in_vpc(): +def test_authorize_group_in_vpc(): conn = boto.connect_ec2('the_key', 'the_secret') vpc_id = "vpc-12345" @@ -142,5 +142,6 @@ def test_authorize_ip_in_vpc(): security_group2 = conn.create_security_group('test2', 'test2', vpc_id) success = security_group1.authorize(ip_protocol="tcp", from_port="22", to_port="2222", src_group=security_group2) + success = security_group1.revoke(ip_protocol="tcp", from_port="22", to_port="2222", src_group=security_group2) From 2b4fe552d1ec13249c8a330f589e4a4477cbc91f Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 20 Mar 2014 17:52:49 -0700 Subject: [PATCH 3/5] VPC support for adding rules improving --- moto/ec2/models.py | 38 ++++++++++++++++++++++++--- moto/ec2/responses/security_groups.py | 10 +++++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index a1165efa9..05aca8519 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -393,25 +393,55 @@ class SecurityGroupBackend(object): default_group = ec2_backend.create_security_group("default", "The default security group", force=True) return default_group - def authorize_security_group_ingress(self, group_name, group_id, ip_protocol, from_port, to_port, ip_ranges=None, source_group_names=None, vpc_id=None): + def authorize_security_group_ingress(self, + group_name, + group_id, + ip_protocol, + from_port, + to_port, + ip_ranges=None, + source_group_names=None, + source_group_ids=None, + vpc_id=None): # to auth a group in a VPC you need the group_id the name isn't enough if group_name: group = self.get_security_group_from_name(group_name, vpc_id) elif group_id: group = self.get_security_group_from_id(group_id) - + source_groups = [] for source_group_name in source_group_names: source_group = self.get_security_group_from_name(source_group_name, vpc_id) if source_group: source_groups.append(source_group) + # for VPCs + for source_group_id in source_group_ids: + source_group = self.get_security_group_from_id(source_group_id) + if source_group: + source_groups.append(source_group) + security_rule = SecurityRule(ip_protocol, from_port, to_port, ip_ranges, source_groups) group.ingress_rules.append(security_rule) - def revoke_security_group_ingress(self, group_name, group_id, ip_protocol, from_port, to_port, ip_ranges=None, source_group_names=None, vpc_id=None): - group = self.get_security_group_from_name(group_name, vpc_id) + def revoke_security_group_ingress(self, + group_name, + group_id, + ip_protocol, + from_port, + to_port, + ip_ranges=None, + source_group_names=None, + source_group_ids=None, + vpc_id=None): + + if group_name: + group = self.get_security_group_from_name(group_name, vpc_id) + elif group_id: + group = self.get_security_group_from_id(group_id) + + source_groups = [] for source_group_name in source_group_names: source_group = self.get_security_group_from_name(source_group_name, vpc_id) diff --git a/moto/ec2/responses/security_groups.py b/moto/ec2/responses/security_groups.py index b92235322..ab5d3b1eb 100644 --- a/moto/ec2/responses/security_groups.py +++ b/moto/ec2/responses/security_groups.py @@ -22,11 +22,17 @@ def process_rules_from_querystring(querystring): if 'IpPermissions.1.IpRanges' in key: ip_ranges.append(value[0]) + source_groups = [] + source_group_ids = [] + for key, value in querystring.iteritems(): - if 'IpPermissions.1.Groups' in key: + if 'IpPermissions.1.Groups.1.GroupId' in key: + source_group_ids.append(value[0]) + elif 'IpPermissions.1.Groups' in key: source_groups.append(value[0]) - return (name, group_id, ip_protocol, from_port, to_port, ip_ranges, source_groups) + + return (name, group_id, ip_protocol, from_port, to_port, ip_ranges, source_groups, source_group_ids) class SecurityGroups(BaseResponse): From ecaf53fd2852849a9c0bc589db270e4bdbcc87bf Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 20 Mar 2014 17:53:39 -0700 Subject: [PATCH 4/5] fix for VPC revoking of rules related to groups --- moto/ec2/models.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 05aca8519..73b16be3d 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -435,7 +435,7 @@ class SecurityGroupBackend(object): source_group_names=None, source_group_ids=None, vpc_id=None): - + if group_name: group = self.get_security_group_from_name(group_name, vpc_id) elif group_id: @@ -448,6 +448,11 @@ class SecurityGroupBackend(object): if source_group: source_groups.append(source_group) + for source_group_id in source_group_ids: + source_group = self.get_security_group_from_id(source_group_id) + if source_group: + source_groups.append(source_group) + security_rule = SecurityRule(ip_protocol, from_port, to_port, ip_ranges, source_groups) if security_rule in group.ingress_rules: group.ingress_rules.remove(security_rule) From 77ab6d20225ff531f84792fe0fd7d8bfd36e92d1 Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Fri, 21 Mar 2014 13:31:00 -0700 Subject: [PATCH 5/5] removed unnecessary itertools chain. added success test case around authorize & revoke --- moto/ec2/models.py | 4 ++-- tests/test_ec2/test_security_groups.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 73b16be3d..5b1f5a1d6 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -377,7 +377,7 @@ class SecurityGroupBackend(object): # 2 levels of chaining necessary since it's a complex structure all_groups = itertools.chain.from_iterable([x.values() for x in self.groups.values()]) - for group in itertools.chain(all_groups): + for group in all_groups: if group.id == group_id: return group @@ -452,7 +452,7 @@ class SecurityGroupBackend(object): source_group = self.get_security_group_from_id(source_group_id) if source_group: source_groups.append(source_group) - + security_rule = SecurityRule(ip_protocol, from_port, to_port, ip_ranges, source_groups) if security_rule in group.ingress_rules: group.ingress_rules.remove(security_rule) diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index fe330e17f..27d523e41 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -142,6 +142,8 @@ def test_authorize_group_in_vpc(): security_group2 = conn.create_security_group('test2', 'test2', vpc_id) success = security_group1.authorize(ip_protocol="tcp", from_port="22", to_port="2222", src_group=security_group2) + success.should.be.true success = security_group1.revoke(ip_protocol="tcp", from_port="22", to_port="2222", src_group=security_group2) + success.should.be.true