From 9b12ce6809dc8015a09f4e6b51fb856a830c4295 Mon Sep 17 00:00:00 2001 From: Hans Date: Sat, 25 May 2019 17:21:57 +0800 Subject: [PATCH] Fix #1842 Create cross region VPC peering connection in both region (#2195) Add a class level store in models/VPCPeeringConnectionBackend of ec2 for saving vpc peering connection. Any instance can correctly save VPC peering connection info on both region when it create vpc peering connection. Update vpc_peering_connections in ec2/responses to meet new version: DESCRIBE_VPC_PEERING_CONNECTIONS_RESPONSE, ACCEPT_VPC_PEERING_CONNECTION_RESPONSE, Previous code only create one region VPC peering connection but doesn't create the other region VPC peering connection when create cross region VPC peering connection. Tested in real AWS environment at first and create unit test case according to real AWS environment response. Add 5 test cases VPC cross region delete case VPC cross region accept case VPC cross region accept wrong region case VPC cross region reject case VPC cross region reject wrong region case Related: #1842, #1830 --- moto/ec2/exceptions.py | 22 +++ moto/ec2/models.py | 38 ++++- moto/ec2/responses/vpc_peering_connections.py | 64 ++++--- tests/test_ec2/test_vpc_peering.py | 158 +++++++++++++++++- 4 files changed, 246 insertions(+), 36 deletions(-) diff --git a/moto/ec2/exceptions.py b/moto/ec2/exceptions.py index f747c9cd5..ec2c8542d 100644 --- a/moto/ec2/exceptions.py +++ b/moto/ec2/exceptions.py @@ -420,3 +420,25 @@ class OperationNotPermitted(EC2ClientError): "The vpc CIDR block with association ID {} may not be disassociated. " "It is the primary IPv4 CIDR block of the VPC".format(association_id) ) + + +# accept exception +class OperationNotPermitted2(EC2ClientError): + def __init__(self, client_region, pcx_id, acceptor_region): + super(OperationNotPermitted2, self).__init__( + "OperationNotPermitted", + "Incorrect region ({0}) specified for this request." + "VPC peering connection {1} must be accepted in region {2}".format(client_region, pcx_id, acceptor_region) + ) + + +# reject exception +class OperationNotPermitted3(EC2ClientError): + def __init__(self, client_region, pcx_id, acceptor_region): + super(OperationNotPermitted3, self).__init__( + "OperationNotPermitted", + "Incorrect region ({0}) specified for this request." + "VPC peering connection {1} must be accepted or rejected in region {2}".format(client_region, + pcx_id, + acceptor_region) + ) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index fa07841b2..685558c35 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -70,6 +70,8 @@ from .exceptions import ( MissingParameterError, MotoNotImplementedError, OperationNotPermitted, + OperationNotPermitted2, + OperationNotPermitted3, ResourceAlreadyAssociatedError, RulesPerSecurityGroupLimitExceededError, TagLimitExceeded) @@ -2120,16 +2122,16 @@ class VPC(TaggedEC2Resource): class VPCBackend(object): - __refs__ = defaultdict(list) + vpc_refs = defaultdict(set) def __init__(self): self.vpcs = {} - self.__refs__[self.__class__].append(weakref.ref(self)) + self.vpc_refs[self.__class__].add(weakref.ref(self)) super(VPCBackend, self).__init__() @classmethod - def get_instances(cls): - for inst_ref in cls.__refs__[cls]: + def get_vpc_refs(cls): + for inst_ref in cls.vpc_refs[cls]: inst = inst_ref() if inst is not None: yield inst @@ -2159,7 +2161,7 @@ class VPCBackend(object): # get vpc by vpc id and aws region def get_cross_vpc(self, vpc_id, peer_region): - for vpcs in self.get_instances(): + for vpcs in self.get_vpc_refs(): if vpcs.region_name == peer_region: match_vpc = vpcs.get_vpc(vpc_id) return match_vpc @@ -2280,15 +2282,31 @@ class VPCPeeringConnection(TaggedEC2Resource): class VPCPeeringConnectionBackend(object): + # for cross region vpc reference + vpc_pcx_refs = defaultdict(set) + def __init__(self): self.vpc_pcxs = {} + self.vpc_pcx_refs[self.__class__].add(weakref.ref(self)) super(VPCPeeringConnectionBackend, self).__init__() + @classmethod + def get_vpc_pcx_refs(cls): + for inst_ref in cls.vpc_pcx_refs[cls]: + inst = inst_ref() + if inst is not None: + yield inst + def create_vpc_peering_connection(self, vpc, peer_vpc): vpc_pcx_id = random_vpc_peering_connection_id() vpc_pcx = VPCPeeringConnection(vpc_pcx_id, vpc, peer_vpc) vpc_pcx._status.pending() self.vpc_pcxs[vpc_pcx_id] = vpc_pcx + # insert cross region peering info + if vpc.ec2_backend.region_name != peer_vpc.ec2_backend.region_name: + for vpc_pcx_cx in peer_vpc.ec2_backend.get_vpc_pcx_refs(): + if vpc_pcx_cx.region_name == peer_vpc.ec2_backend.region_name: + vpc_pcx_cx.vpc_pcxs[vpc_pcx_id] = vpc_pcx return vpc_pcx def get_all_vpc_peering_connections(self): @@ -2306,6 +2324,11 @@ class VPCPeeringConnectionBackend(object): def accept_vpc_peering_connection(self, vpc_pcx_id): vpc_pcx = self.get_vpc_peering_connection(vpc_pcx_id) + # if cross region need accepter from another region + pcx_req_region = vpc_pcx.vpc.ec2_backend.region_name + pcx_acp_region = vpc_pcx.peer_vpc.ec2_backend.region_name + if pcx_req_region != pcx_acp_region and self.region_name == pcx_req_region: + raise OperationNotPermitted2(self.region_name, vpc_pcx.id, pcx_acp_region) if vpc_pcx._status.code != 'pending-acceptance': raise InvalidVPCPeeringConnectionStateTransitionError(vpc_pcx.id) vpc_pcx._status.accept() @@ -2313,6 +2336,11 @@ class VPCPeeringConnectionBackend(object): def reject_vpc_peering_connection(self, vpc_pcx_id): vpc_pcx = self.get_vpc_peering_connection(vpc_pcx_id) + # if cross region need accepter from another region + pcx_req_region = vpc_pcx.vpc.ec2_backend.region_name + pcx_acp_region = vpc_pcx.peer_vpc.ec2_backend.region_name + if pcx_req_region != pcx_acp_region and self.region_name == pcx_req_region: + raise OperationNotPermitted3(self.region_name, vpc_pcx.id, pcx_acp_region) if vpc_pcx._status.code != 'pending-acceptance': raise InvalidVPCPeeringConnectionStateTransitionError(vpc_pcx.id) vpc_pcx._status.reject() diff --git a/moto/ec2/responses/vpc_peering_connections.py b/moto/ec2/responses/vpc_peering_connections.py index 49d752893..68bae72da 100644 --- a/moto/ec2/responses/vpc_peering_connections.py +++ b/moto/ec2/responses/vpc_peering_connections.py @@ -74,30 +74,35 @@ CREATE_VPC_PEERING_CONNECTION_RESPONSE = """ """ DESCRIBE_VPC_PEERING_CONNECTIONS_RESPONSE = """ - - 7a62c49f-347e-4fc4-9331-6e8eEXAMPLE - - {% for vpc_pcx in vpc_pcxs %} - - {{ vpc_pcx.id }} - - 777788889999 - {{ vpc_pcx.vpc.id }} - {{ vpc_pcx.vpc.cidr_block }} - - - 123456789012 - {{ vpc_pcx.peer_vpc.id }} - - - {{ vpc_pcx._status.code }} - {{ vpc_pcx._status.message }} - - 2014-02-17T16:00:50.000Z - - - {% endfor %} - + +7a62c49f-347e-4fc4-9331-6e8eEXAMPLE + + {% for vpc_pcx in vpc_pcxs %} + + {{ vpc_pcx.id }} + + 777788889999 + {{ vpc_pcx.vpc.id }} + {{ vpc_pcx.vpc.cidr_block }} + + + 123456789012 + {{ vpc_pcx.peer_vpc.id }} + {{ vpc_pcx.peer_vpc.cidr_block }} + + false + true + false + + + + {{ vpc_pcx._status.code }} + {{ vpc_pcx._status.message }} + + + + {% endfor %} + """ @@ -109,19 +114,24 @@ DELETE_VPC_PEERING_CONNECTION_RESPONSE = """ """ ACCEPT_VPC_PEERING_CONNECTION_RESPONSE = """ - + 7a62c49f-347e-4fc4-9331-6e8eEXAMPLE {{ vpc_pcx.id }} - 123456789012 + 777788889999 {{ vpc_pcx.vpc.id }} {{ vpc_pcx.vpc.cidr_block }} - 777788889999 + 123456789012 {{ vpc_pcx.peer_vpc.id }} {{ vpc_pcx.peer_vpc.cidr_block }} + + false + false + false + {{ vpc_pcx._status.code }} diff --git a/tests/test_ec2/test_vpc_peering.py b/tests/test_ec2/test_vpc_peering.py index 082499a72..edfbfb3c2 100644 --- a/tests/test_ec2/test_vpc_peering.py +++ b/tests/test_ec2/test_vpc_peering.py @@ -107,14 +107,19 @@ def test_vpc_peering_connections_cross_region(): ec2_apn1 = boto3.resource('ec2', region_name='ap-northeast-1') vpc_apn1 = ec2_apn1.create_vpc(CidrBlock='10.20.0.0/16') # create peering - vpc_pcx = ec2_usw1.create_vpc_peering_connection( + vpc_pcx_usw1 = ec2_usw1.create_vpc_peering_connection( VpcId=vpc_usw1.id, PeerVpcId=vpc_apn1.id, PeerRegion='ap-northeast-1', ) - vpc_pcx.status['Code'].should.equal('initiating-request') - vpc_pcx.requester_vpc.id.should.equal(vpc_usw1.id) - vpc_pcx.accepter_vpc.id.should.equal(vpc_apn1.id) + vpc_pcx_usw1.status['Code'].should.equal('initiating-request') + vpc_pcx_usw1.requester_vpc.id.should.equal(vpc_usw1.id) + vpc_pcx_usw1.accepter_vpc.id.should.equal(vpc_apn1.id) + # test cross region vpc peering connection exist + vpc_pcx_apn1 = ec2_apn1.VpcPeeringConnection(vpc_pcx_usw1.id) + vpc_pcx_apn1.id.should.equal(vpc_pcx_usw1.id) + vpc_pcx_apn1.requester_vpc.id.should.equal(vpc_usw1.id) + vpc_pcx_apn1.accepter_vpc.id.should.equal(vpc_apn1.id) @mock_ec2 @@ -131,3 +136,148 @@ def test_vpc_peering_connections_cross_region_fail(): PeerVpcId=vpc_apn1.id, PeerRegion='ap-northeast-2') cm.exception.response['Error']['Code'].should.equal('InvalidVpcID.NotFound') + + +@mock_ec2 +def test_vpc_peering_connections_cross_region_accept(): + # create vpc in us-west-1 and ap-northeast-1 + ec2_usw1 = boto3.resource('ec2', region_name='us-west-1') + vpc_usw1 = ec2_usw1.create_vpc(CidrBlock='10.90.0.0/16') + ec2_apn1 = boto3.resource('ec2', region_name='ap-northeast-1') + vpc_apn1 = ec2_apn1.create_vpc(CidrBlock='10.20.0.0/16') + # create peering + vpc_pcx_usw1 = ec2_usw1.create_vpc_peering_connection( + VpcId=vpc_usw1.id, + PeerVpcId=vpc_apn1.id, + PeerRegion='ap-northeast-1', + ) + # accept peering from ap-northeast-1 + ec2_apn1 = boto3.client('ec2', region_name='ap-northeast-1') + ec2_usw1 = boto3.client('ec2', region_name='us-west-1') + acp_pcx_apn1 = ec2_apn1.accept_vpc_peering_connection( + VpcPeeringConnectionId=vpc_pcx_usw1.id + ) + des_pcx_apn1 = ec2_usw1.describe_vpc_peering_connections( + VpcPeeringConnectionIds=[vpc_pcx_usw1.id] + ) + des_pcx_usw1 = ec2_usw1.describe_vpc_peering_connections( + VpcPeeringConnectionIds=[vpc_pcx_usw1.id] + ) + acp_pcx_apn1['VpcPeeringConnection']['Status']['Code'].should.equal('active') + des_pcx_apn1['VpcPeeringConnections'][0]['Status']['Code'].should.equal('active') + des_pcx_usw1['VpcPeeringConnections'][0]['Status']['Code'].should.equal('active') + + +@mock_ec2 +def test_vpc_peering_connections_cross_region_reject(): + # create vpc in us-west-1 and ap-northeast-1 + ec2_usw1 = boto3.resource('ec2', region_name='us-west-1') + vpc_usw1 = ec2_usw1.create_vpc(CidrBlock='10.90.0.0/16') + ec2_apn1 = boto3.resource('ec2', region_name='ap-northeast-1') + vpc_apn1 = ec2_apn1.create_vpc(CidrBlock='10.20.0.0/16') + # create peering + vpc_pcx_usw1 = ec2_usw1.create_vpc_peering_connection( + VpcId=vpc_usw1.id, + PeerVpcId=vpc_apn1.id, + PeerRegion='ap-northeast-1', + ) + # reject peering from ap-northeast-1 + ec2_apn1 = boto3.client('ec2', region_name='ap-northeast-1') + ec2_usw1 = boto3.client('ec2', region_name='us-west-1') + rej_pcx_apn1 = ec2_apn1.reject_vpc_peering_connection( + VpcPeeringConnectionId=vpc_pcx_usw1.id + ) + des_pcx_apn1 = ec2_usw1.describe_vpc_peering_connections( + VpcPeeringConnectionIds=[vpc_pcx_usw1.id] + ) + des_pcx_usw1 = ec2_usw1.describe_vpc_peering_connections( + VpcPeeringConnectionIds=[vpc_pcx_usw1.id] + ) + rej_pcx_apn1['Return'].should.equal(True) + des_pcx_apn1['VpcPeeringConnections'][0]['Status']['Code'].should.equal('rejected') + des_pcx_usw1['VpcPeeringConnections'][0]['Status']['Code'].should.equal('rejected') + + +@mock_ec2 +def test_vpc_peering_connections_cross_region_delete(): + # create vpc in us-west-1 and ap-northeast-1 + ec2_usw1 = boto3.resource('ec2', region_name='us-west-1') + vpc_usw1 = ec2_usw1.create_vpc(CidrBlock='10.90.0.0/16') + ec2_apn1 = boto3.resource('ec2', region_name='ap-northeast-1') + vpc_apn1 = ec2_apn1.create_vpc(CidrBlock='10.20.0.0/16') + # create peering + vpc_pcx_usw1 = ec2_usw1.create_vpc_peering_connection( + VpcId=vpc_usw1.id, + PeerVpcId=vpc_apn1.id, + PeerRegion='ap-northeast-1', + ) + # reject peering from ap-northeast-1 + ec2_apn1 = boto3.client('ec2', region_name='ap-northeast-1') + ec2_usw1 = boto3.client('ec2', region_name='us-west-1') + del_pcx_apn1 = ec2_apn1.delete_vpc_peering_connection( + VpcPeeringConnectionId=vpc_pcx_usw1.id + ) + des_pcx_apn1 = ec2_usw1.describe_vpc_peering_connections( + VpcPeeringConnectionIds=[vpc_pcx_usw1.id] + ) + des_pcx_usw1 = ec2_usw1.describe_vpc_peering_connections( + VpcPeeringConnectionIds=[vpc_pcx_usw1.id] + ) + del_pcx_apn1['Return'].should.equal(True) + des_pcx_apn1['VpcPeeringConnections'][0]['Status']['Code'].should.equal('deleted') + des_pcx_usw1['VpcPeeringConnections'][0]['Status']['Code'].should.equal('deleted') + + +@mock_ec2 +def test_vpc_peering_connections_cross_region_accept_wrong_region(): + # create vpc in us-west-1 and ap-northeast-1 + ec2_usw1 = boto3.resource('ec2', region_name='us-west-1') + vpc_usw1 = ec2_usw1.create_vpc(CidrBlock='10.90.0.0/16') + ec2_apn1 = boto3.resource('ec2', region_name='ap-northeast-1') + vpc_apn1 = ec2_apn1.create_vpc(CidrBlock='10.20.0.0/16') + # create peering + vpc_pcx_usw1 = ec2_usw1.create_vpc_peering_connection( + VpcId=vpc_usw1.id, + PeerVpcId=vpc_apn1.id, + PeerRegion='ap-northeast-1', + ) + + # accept wrong peering from us-west-1 which will raise error + ec2_apn1 = boto3.client('ec2', region_name='ap-northeast-1') + ec2_usw1 = boto3.client('ec2', region_name='us-west-1') + with assert_raises(ClientError) as cm: + ec2_usw1.accept_vpc_peering_connection( + VpcPeeringConnectionId=vpc_pcx_usw1.id + ) + cm.exception.response['Error']['Code'].should.equal('OperationNotPermitted') + exp_msg = 'Incorrect region ({0}) specified for this request.VPC ' \ + 'peering connection {1} must be ' \ + 'accepted in region {2}'.format('us-west-1', vpc_pcx_usw1.id, 'ap-northeast-1') + cm.exception.response['Error']['Message'].should.equal(exp_msg) + + +@mock_ec2 +def test_vpc_peering_connections_cross_region_reject_wrong_region(): + # create vpc in us-west-1 and ap-northeast-1 + ec2_usw1 = boto3.resource('ec2', region_name='us-west-1') + vpc_usw1 = ec2_usw1.create_vpc(CidrBlock='10.90.0.0/16') + ec2_apn1 = boto3.resource('ec2', region_name='ap-northeast-1') + vpc_apn1 = ec2_apn1.create_vpc(CidrBlock='10.20.0.0/16') + # create peering + vpc_pcx_usw1 = ec2_usw1.create_vpc_peering_connection( + VpcId=vpc_usw1.id, + PeerVpcId=vpc_apn1.id, + PeerRegion='ap-northeast-1', + ) + # reject wrong peering from us-west-1 which will raise error + ec2_apn1 = boto3.client('ec2', region_name='ap-northeast-1') + ec2_usw1 = boto3.client('ec2', region_name='us-west-1') + with assert_raises(ClientError) as cm: + ec2_usw1.reject_vpc_peering_connection( + VpcPeeringConnectionId=vpc_pcx_usw1.id + ) + cm.exception.response['Error']['Code'].should.equal('OperationNotPermitted') + exp_msg = 'Incorrect region ({0}) specified for this request.VPC ' \ + 'peering connection {1} must be accepted or ' \ + 'rejected in region {2}'.format('us-west-1', vpc_pcx_usw1.id, 'ap-northeast-1') + cm.exception.response['Error']['Message'].should.equal(exp_msg)