From c3e4b5401c526d9b826ab4404da4f718792bc80c Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Wed, 30 Oct 2013 17:55:13 -0700 Subject: [PATCH 01/12] added new vpc security group test. failing --- tests/test_ec2/test_security_groups.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index ce8de872..606faebc 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -20,6 +20,27 @@ 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') + vpc_id = 'vpc-5300000c' + security_group = conn.create_security_group('test security group', 'this is a test security group', vpc_id) + + security_group.vpc_id.should.equal(vpc_id) + + security_group.name.should.equal('test security group') + security_group.description.should.equal('this is a test security group') + + # Trying to create another group with the same name should throw an error + conn.create_security_group.when.called_with('test security group', 'this is a test security group').should.throw(EC2ResponseError) + + all_groups = conn.get_all_security_groups() + + all_groups[0].vpc_id.should.equal(vpc_id) + + all_groups.should.have.length_of(1) + all_groups[0].name.should.equal('test security group') + @mock_ec2 def test_deleting_security_groups(): From 9ae144bad8e1182e7ef88774ebd6b72658c0a74a Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Wed, 30 Oct 2013 18:50:42 -0700 Subject: [PATCH 02/12] correctly getting the vpc address back --- moto/ec2/models.py | 9 ++++++--- moto/ec2/responses/security_groups.py | 5 +++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 0a391c5d..95903b82 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -300,27 +300,30 @@ class SecurityRule(object): class SecurityGroup(object): - def __init__(self, group_id, name, description): + def __init__(self, group_id, name, description, vpc_id=None): self.id = group_id self.name = name self.description = description self.ingress_rules = [] self.egress_rules = [] + self.vpc_id = vpc_id class SecurityGroupBackend(object): def __init__(self): self.groups = {} + self.vpc_groups = {} + super(SecurityGroupBackend, self).__init__() - def create_security_group(self, name, description, force=False): + def create_security_group(self, name, description, vpc_id=None, force=False): group_id = random_security_group_id() if not force: existing_group = self.get_security_group_from_name(name) if existing_group: return None - group = SecurityGroup(group_id, name, description) + group = SecurityGroup(group_id, name, description, vpc_id=vpc_id) self.groups[group_id] = group return group diff --git a/moto/ec2/responses/security_groups.py b/moto/ec2/responses/security_groups.py index 69d32d5f..16905104 100644 --- a/moto/ec2/responses/security_groups.py +++ b/moto/ec2/responses/security_groups.py @@ -31,7 +31,8 @@ class SecurityGroups(object): def create_security_group(self): name = self.querystring.get('GroupName')[0] description = self.querystring.get('GroupDescription')[0] - group = ec2_backend.create_security_group(name, description) + vpc_id = self.querystring.get("VpcId", [None])[0] + group = ec2_backend.create_security_group(name, description, vpc_id=vpc_id) if not group: # There was an exisitng group return "There was an existing security group with name {0}".format(name), dict(status=409) @@ -83,7 +84,7 @@ DESCRIBE_SECURITY_GROUPS_RESPONSE = """ {% for rule in group.ingress_rules %} From 9cbac9bbf4253f87e3f657f9d828b650f7a86c39 Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Wed, 30 Oct 2013 20:11:15 -0700 Subject: [PATCH 04/12] reworked internals, groups is now a dict of dicts. need to fix errors coming back from revoking and deleting groups --- moto/ec2/models.py | 35 +++++++++++++------------- tests/test_ec2/test_security_groups.py | 19 ++++++++++++-- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 95903b82..20d5af1c 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -1,4 +1,5 @@ import copy +import itertools from collections import defaultdict from boto.ec2.instance import Instance as BotoInstance, Reservation @@ -312,36 +313,36 @@ class SecurityGroup(object): class SecurityGroupBackend(object): def __init__(self): - self.groups = {} - self.vpc_groups = {} - + # the key in the dict group is the vpc_id or None (non-vpc) + self.groups = defaultdict(dict) super(SecurityGroupBackend, self).__init__() def create_security_group(self, name, description, vpc_id=None, force=False): group_id = random_security_group_id() if not force: - existing_group = self.get_security_group_from_name(name) + existing_group = self.get_security_group_from_name(name, vpc_id) if existing_group: return None group = SecurityGroup(group_id, name, description, vpc_id=vpc_id) - self.groups[group_id] = group + + self.groups[vpc_id][group_id] = group return group def describe_security_groups(self): - return self.groups.values() + return itertools.chain(*[x.values() for x in self.groups.values()]) - def delete_security_group(self, name_or_group_id): - if name_or_group_id in self.groups: + def delete_security_group(self, name_or_group_id, vpc_id): + if name_or_group_id in self.groups[vpc_id]: # Group Id - return self.groups.pop(name_or_group_id) + return self.groups[vpc_id].pop(name_or_group_id) else: # Group Name - group = self.get_security_group_from_name(name_or_group_id) + group = self.get_security_group_from_name(name_or_group_id, vpc_id) if group: - return self.groups.pop(group.id) + return self.groups[vpc_id].pop(group.id) - def get_security_group_from_name(self, name): - for group_id, group in self.groups.iteritems(): + def get_security_group_from_name(self, name, vpc_id): + for group_id, group in self.groups[vpc_id].iteritems(): if group.name == name: return group @@ -350,16 +351,16 @@ 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): - group = self.get_security_group_from_name(group_name) + 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) source_groups = [] for source_group_name in source_group_names: - source_groups.append(self.get_security_group_from_name(source_group_name)) + source_groups.append(self.get_security_group_from_name(source_group_name, vpc_id)) 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): + def revoke_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) source_groups = [] for source_group_name in source_group_names: diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index 606faebc..6b33c360 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -31,8 +31,8 @@ def test_create_and_describe_vpc_security_group(): security_group.name.should.equal('test security group') security_group.description.should.equal('this is a test security group') - # Trying to create another group with the same name should throw an error - conn.create_security_group.when.called_with('test security group', 'this is a test security group').should.throw(EC2ResponseError) + # Trying to create another group with the same name in the same VPC should throw an error + conn.create_security_group.when.called_with('test security group', 'this is a test security group', vpc_id).should.throw(EC2ResponseError) all_groups = conn.get_all_security_groups() @@ -41,6 +41,21 @@ def test_create_and_describe_vpc_security_group(): all_groups.should.have.length_of(1) all_groups[0].name.should.equal('test security group') +@mock_ec2 +def test_create_two_security_groups_with_same_name_in_different_vpc(): + conn = boto.connect_ec2('the_key', 'the_secret') + vpc_id = 'vpc-5300000c' + vpc_id2 = 'vpc-5300000d' + + sg1 = conn.create_security_group('test security group', 'this is a test security group', vpc_id) + sg2 = conn.create_security_group('test security group', 'this is a test security group', vpc_id2) + + all_groups = conn.get_all_security_groups() + + all_groups.should.have.length_of(2) + all_groups[0].name.should.equal('test security group') + all_groups[1].name.should.equal('test security group') + @mock_ec2 def test_deleting_security_groups(): From c2dad5357fe892ff621ecb33b62e79a271f8b9dc Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Fri, 1 Nov 2013 12:56:53 -0700 Subject: [PATCH 05/12] correctly passing vpc_id through to revoke --- moto/ec2/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 20d5af1c..2a760dd7 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -361,10 +361,10 @@ class SecurityGroupBackend(object): 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): - group = self.get_security_group_from_name(group_name) + group = self.get_security_group_from_name(group_name, vpc_id) source_groups = [] for source_group_name in source_group_names: - source_groups.append(self.get_security_group_from_name(source_group_name)) + source_groups.append(self.get_security_group_from_name(source_group_name, vpc_id)) security_rule = SecurityRule(ip_protocol, from_port, to_port, ip_ranges, source_groups) if security_rule in group.ingress_rules: From 0712d40f71016d03f71f3348ebc0721a69927662 Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 5 Dec 2013 16:39:25 -0800 Subject: [PATCH 06/12] added vpc_id to deletion --- moto/ec2/responses/security_groups.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/moto/ec2/responses/security_groups.py b/moto/ec2/responses/security_groups.py index 9ec5eaa4..5e8eebaa 100644 --- a/moto/ec2/responses/security_groups.py +++ b/moto/ec2/responses/security_groups.py @@ -42,8 +42,10 @@ class SecurityGroups(object): def delete_security_group(self): # TODO this should raise an error if there are instances in the group. See http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-DeleteSecurityGroup.html name = self.querystring.get('GroupName')[0] - group = ec2_backend.delete_security_group(name) + vpc_id = self.querystring.get("VpcId", [None])[0] + # needs vpc now + group = ec2_backend.delete_security_group(name, vpc_id) if not group: # There was no such group return "There was no security group with name {0}".format(name), dict(status=404) From 0e316d8fc3ad93abac78378290bd9dfdc85e7a3c Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 5 Dec 2013 16:45:18 -0800 Subject: [PATCH 07/12] fixed spot instances creation --- moto/ec2/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 2a760dd7..6121c338 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -540,12 +540,12 @@ class SpotInstanceRequest(object): self.security_groups = [] if security_groups: for group_name in security_groups: - group = ec2_backend.get_security_group_from_name(group_name) + group = ec2_backend.get_security_group_from_name(group_name, None) if group: self.security_groups.append(group) else: # If not security groups, add the default - default_group = ec2_backend.get_security_group_from_name("default") + default_group = ec2_backend.get_security_group_from_name("default", None) self.security_groups.append(default_group) From 8781714f5cadeab86f96327906e1e8b68b97d78e Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 5 Dec 2013 17:00:35 -0800 Subject: [PATCH 08/12] security group deletion --- tests/test_ec2/test_security_groups.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index 6b33c360..07af3315 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -76,6 +76,21 @@ def test_deleting_security_groups(): conn.delete_security_group(security_group1.id) conn.get_all_security_groups().should.have.length_of(0) +@mock_ec2 +def test_delete_security_group_in_vpc(): + conn = boto.connect_ec2('the_key', 'the_secret') + vpc_id = "vpc-12345" + security_group1 = conn.create_security_group('test1', 'test1', vpc_id) + + # Deleting a group that doesn't exist in the VPC should throw an error + conn.delete_security_group.when.called_with('test1').should.throw(EC2ResponseError) + + conn.delete_security_group("test1", vpc_id=vpc_id) + + # Delete by group id + # conn.delete_security_group(security_group1.id) + # conn.get_all_security_groups().should.have.length_of(0) + @mock_ec2 def test_authorize_ip_range_and_revoke(): From 95acab1dcae38ecae8ce4879b15a0ffda4dbf5e4 Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 5 Dec 2013 17:11:10 -0800 Subject: [PATCH 09/12] added myself to contributors --- AUTHORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.md b/AUTHORS.md index 8a0831dc..1a21fcc3 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -12,3 +12,4 @@ Moto is written by Steve Pulec with contributions from: * [Konstantinos Koukopoulos](https://github.com/kouk) * [attili](https://github.com/attili) * [JJ Zeng](https://github.com/jjofseattle) +* [Jon Haddad](https://github.com/rustyrazorblade) From 1eac424bf52fb6fab27f74c3a68c8d8b675badec Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Thu, 5 Dec 2013 17:56:46 -0800 Subject: [PATCH 10/12] missed a commit --- tests/test_ec2/test_security_groups.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index 07af3315..56bf6a0a 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -24,7 +24,7 @@ def test_create_and_describe_security_group(): def test_create_and_describe_vpc_security_group(): conn = boto.connect_ec2('the_key', 'the_secret') vpc_id = 'vpc-5300000c' - security_group = conn.create_security_group('test security group', 'this is a test security group', vpc_id) + security_group = conn.create_security_group('test security group', 'this is a test security group', vpc_id=vpc_id) security_group.vpc_id.should.equal(vpc_id) @@ -85,6 +85,7 @@ def test_delete_security_group_in_vpc(): # Deleting a group that doesn't exist in the VPC should throw an error conn.delete_security_group.when.called_with('test1').should.throw(EC2ResponseError) + # this should not throw an exception conn.delete_security_group("test1", vpc_id=vpc_id) # Delete by group id From 48ee4b600b14d0220079b6d9356f68e79031e09e Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Fri, 6 Dec 2013 14:34:13 -0800 Subject: [PATCH 11/12] updated SC methods to work with a group_id, which must be used if it's a group in a VPC --- moto/ec2/models.py | 18 ++++++++++-------- moto/ec2/responses/security_groups.py | 13 +++++++++---- tests/test_ec2/test_security_groups.py | 11 ++--------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 6121c338..a8e29bf2 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -331,15 +331,17 @@ class SecurityGroupBackend(object): def describe_security_groups(self): return itertools.chain(*[x.values() for x in self.groups.values()]) - def delete_security_group(self, name_or_group_id, vpc_id): - if name_or_group_id in self.groups[vpc_id]: - # Group Id - return self.groups[vpc_id].pop(name_or_group_id) - else: - # Group Name - group = self.get_security_group_from_name(name_or_group_id, vpc_id) + def delete_security_group(self, name=None, group_id=None): + if group_id: + # loop over all the SGs, find the right one + for vpc in self.groups.values(): + if group_id in vpc: + return vpc.pop(group_id) + elif name: + # Group Name. Has to be in standard EC2, VPC needs to be identified by group_id + group = self.get_security_group_from_name(name, None) if group: - return self.groups[vpc_id].pop(group.id) + return self.groups[None].pop(group.id) def get_security_group_from_name(self, name, vpc_id): for group_id, group in self.groups[vpc_id].iteritems(): diff --git a/moto/ec2/responses/security_groups.py b/moto/ec2/responses/security_groups.py index 5e8eebaa..1abda915 100644 --- a/moto/ec2/responses/security_groups.py +++ b/moto/ec2/responses/security_groups.py @@ -41,11 +41,16 @@ class SecurityGroups(object): def delete_security_group(self): # TODO this should raise an error if there are instances in the group. See http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-DeleteSecurityGroup.html - name = self.querystring.get('GroupName')[0] - vpc_id = self.querystring.get("VpcId", [None])[0] - # needs vpc now - group = ec2_backend.delete_security_group(name, vpc_id) + name = self.querystring.get('GroupName') + sg_id = self.querystring.get('GroupId') + + if name: + group = ec2_backend.delete_security_group(name[0]) + elif sg_id: + group = ec2_backend.delete_security_group(group_id=sg_id[0]) + + # needs name or group now if not group: # There was no such group return "There was no security group with name {0}".format(name), dict(status=404) diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py index 56bf6a0a..9b1f3496 100644 --- a/tests/test_ec2/test_security_groups.py +++ b/tests/test_ec2/test_security_groups.py @@ -73,7 +73,7 @@ def test_deleting_security_groups(): conn.get_all_security_groups().should.have.length_of(1) # Delete by group id - conn.delete_security_group(security_group1.id) + conn.delete_security_group(group_id=security_group1.id) conn.get_all_security_groups().should.have.length_of(0) @mock_ec2 @@ -82,15 +82,8 @@ def test_delete_security_group_in_vpc(): vpc_id = "vpc-12345" security_group1 = conn.create_security_group('test1', 'test1', vpc_id) - # Deleting a group that doesn't exist in the VPC should throw an error - conn.delete_security_group.when.called_with('test1').should.throw(EC2ResponseError) - # this should not throw an exception - conn.delete_security_group("test1", vpc_id=vpc_id) - - # Delete by group id - # conn.delete_security_group(security_group1.id) - # conn.get_all_security_groups().should.have.length_of(0) + conn.delete_security_group(group_id=security_group1.id) @mock_ec2 From 95dec63a4b21a96a9e7bf571c9ba3d0ac69d63f8 Mon Sep 17 00:00:00 2001 From: Jon Haddad Date: Fri, 6 Dec 2013 14:40:00 -0800 Subject: [PATCH 12/12] added latest version of boto to travis matrix --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 037501a5..e7fe9847 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ python: env: matrix: #- BOTO_VERSION=2.13.3 + - BOTO_VERSION=2.19.0 - BOTO_VERSION=2.12.0 - BOTO_VERSION=2.11.0 - BOTO_VERSION=2.10.0