From 3679521d76c54f0ca5fe635956bea2d8fc3192a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Piliszek?= Date: Sat, 1 Oct 2022 00:36:55 +0200 Subject: [PATCH] EC2 - Fix 3 issues with route table associations (#5509) --- moto/ec2/models/route_tables.py | 34 ++++++-- moto/ec2/responses/route_tables.py | 7 +- tests/test_ec2/test_route_tables.py | 125 ++++++++++++++++++---------- 3 files changed, 112 insertions(+), 54 deletions(-) diff --git a/moto/ec2/models/route_tables.py b/moto/ec2/models/route_tables.py index c2f7bc751..4df0164bc 100644 --- a/moto/ec2/models/route_tables.py +++ b/moto/ec2/models/route_tables.py @@ -25,8 +25,10 @@ class RouteTable(TaggedEC2Resource, CloudFormationModel): self.ec2_backend = ec2_backend self.id = route_table_id self.vpc_id = vpc_id - self.main = main - self.main_association = random_subnet_association_id() + if main: + self.main_association_id = random_subnet_association_id() + else: + self.main_association_id = None self.associations = {} self.routes = {} @@ -64,7 +66,7 @@ class RouteTable(TaggedEC2Resource, CloudFormationModel): if filter_name == "association.main": # Note: Boto only supports 'true'. # https://github.com/boto/boto/issues/1742 - if self.main: + if self.main_association_id is not None: return "true" else: return "false" @@ -75,7 +77,7 @@ class RouteTable(TaggedEC2Resource, CloudFormationModel): elif filter_name == "association.route-table-id": return self.id elif filter_name == "association.route-table-association-id": - return self.associations.keys() + return self.all_associations_ids elif filter_name == "association.subnet-id": return self.associations.values() elif filter_name == "route.gateway-id": @@ -87,6 +89,14 @@ class RouteTable(TaggedEC2Resource, CloudFormationModel): else: return super().get_filter_value(filter_name, "DescribeRouteTables") + @property + def all_associations_ids(self): + # NOTE(yoctozepto): Doing an explicit copy to not touch the original. + all_associations = set(self.associations) + if self.main_association_id is not None: + all_associations.add(self.main_association_id) + return all_associations + class RouteTableBackend: def __init__(self): @@ -186,7 +196,7 @@ class RouteTableBackend: def replace_route_table_association(self, association_id, route_table_id): # Idempotent if association already exists. new_route_table = self.get_route_table(route_table_id) - if association_id in new_route_table.associations: + if association_id in new_route_table.all_associations_ids: return association_id # Find route table which currently has the association, error if none. @@ -195,11 +205,19 @@ class RouteTableBackend: ) if not route_tables_by_association_id: raise InvalidAssociationIdError(association_id) + previous_route_table = route_tables_by_association_id[0] # Remove existing association, create new one. - previous_route_table = route_tables_by_association_id[0] - subnet_id = previous_route_table.associations.pop(association_id, None) - return self.associate_route_table(route_table_id, subnet_id) + new_association_id = random_subnet_association_id() + if previous_route_table.main_association_id == association_id: + previous_route_table.main_association_id = None + new_route_table.main_association_id = new_association_id + else: + association_target_id = previous_route_table.associations.pop( + association_id + ) + new_route_table.associations[new_association_id] = association_target_id + return new_association_id # TODO: refractor to isloate class methods from backend logic diff --git a/moto/ec2/responses/route_tables.py b/moto/ec2/responses/route_tables.py index 428dfbab6..1703310e6 100644 --- a/moto/ec2/responses/route_tables.py +++ b/moto/ec2/responses/route_tables.py @@ -251,14 +251,16 @@ DESCRIBE_ROUTE_TABLES_RESPONSE = """ {% endfor %} + {% if route_table.main_association_id is not none %} - {{ route_table.main_association }} + {{ route_table.main_association_id }} {{ route_table.id }}
true
associated + {% endif %} {% for association_id,subnet_id in route_table.associations.items() %} {{ association_id }} @@ -324,5 +326,8 @@ REPLACE_ROUTE_TABLE_ASSOCIATION_RESPONSE = """ 59dbff89-35bd-4eac-99ed-be587EXAMPLE {{ association_id }} + + associated + """ diff --git a/tests/test_ec2/test_route_tables.py b/tests/test_ec2/test_route_tables.py index 2f579b2c2..89fbe82b5 100644 --- a/tests/test_ec2/test_route_tables.py +++ b/tests/test_ec2/test_route_tables.py @@ -185,7 +185,7 @@ def test_route_tables_filters_associations(): )["RouteTables"] association1_route_tables.should.have.length_of(1) association1_route_tables[0]["RouteTableId"].should.equal(route_table1.id) - association1_route_tables[0]["Associations"].should.have.length_of(3) + association1_route_tables[0]["Associations"].should.have.length_of(2) # Filter by route table ID route_table2_route_tables = client.describe_route_tables( @@ -193,7 +193,7 @@ def test_route_tables_filters_associations(): )["RouteTables"] route_table2_route_tables.should.have.length_of(1) route_table2_route_tables[0]["RouteTableId"].should.equal(route_table2.id) - route_table2_route_tables[0]["Associations"].should.have.length_of(2) + route_table2_route_tables[0]["Associations"].should.have.length_of(1) # Filter by subnet ID subnet_route_tables = client.describe_route_tables( @@ -201,7 +201,7 @@ def test_route_tables_filters_associations(): )["RouteTables"] subnet_route_tables.should.have.length_of(1) subnet_route_tables[0]["RouteTableId"].should.equal(route_table1.id) - subnet_route_tables[0]["Associations"].should.have.length_of(3) + subnet_route_tables[0]["Associations"].should.have.length_of(2) @mock_ec2 @@ -215,7 +215,7 @@ def test_route_table_associations(): # Refresh r = client.describe_route_tables(RouteTableIds=[route_table.id])["RouteTables"][0] - r["Associations"].should.have.length_of(1) + r["Associations"].should.have.length_of(0) # Associate association_id = client.associate_route_table( @@ -224,12 +224,12 @@ def test_route_table_associations(): # Refresh r = client.describe_route_tables(RouteTableIds=[route_table.id])["RouteTables"][0] - r["Associations"].should.have.length_of(2) + r["Associations"].should.have.length_of(1) - r["Associations"][1]["RouteTableAssociationId"].should.equal(association_id) - r["Associations"][1]["Main"].should.equal(False) - r["Associations"][1]["RouteTableId"].should.equal(route_table.id) - r["Associations"][1]["SubnetId"].should.equal(subnet.id) + r["Associations"][0]["RouteTableAssociationId"].should.equal(association_id) + r["Associations"][0]["Main"].should.equal(False) + r["Associations"][0]["RouteTableId"].should.equal(route_table.id) + r["Associations"][0]["SubnetId"].should.equal(subnet.id) # Associate is idempotent association_id_idempotent = client.associate_route_table( @@ -247,16 +247,9 @@ def test_route_table_associations(): # Disassociate client.disassociate_route_table(AssociationId=association_id) - # Refresh - The default (main) route should be there + # Validate r = client.describe_route_tables(RouteTableIds=[route_table.id])["RouteTables"][0] - r["Associations"].should.have.length_of(1) - r["Associations"][0].should.have.key("Main").equal(True) - r["Associations"][0].should.have.key("RouteTableAssociationId") - r["Associations"][0].should.have.key("RouteTableId").equals(route_table.id) - r["Associations"][0].should.have.key("AssociationState").equals( - {"State": "associated"} - ) - r["Associations"][0].shouldnt.have.key("SubnetId") + r["Associations"].should.have.length_of(0) # Error: Disassociate with invalid association ID with pytest.raises(ClientError) as ex: @@ -301,7 +294,7 @@ def test_route_table_replace_route_table_association(): route_table1 = client.describe_route_tables(RouteTableIds=[route_table1_id])[ "RouteTables" ][0] - route_table1["Associations"].should.have.length_of(1) + route_table1["Associations"].should.have.length_of(0) # Associate association_id1 = client.associate_route_table( @@ -317,15 +310,15 @@ def test_route_table_replace_route_table_association(): ][0] # Validate - route_table1["Associations"].should.have.length_of(2) - route_table2["Associations"].should.have.length_of(1) + route_table1["Associations"].should.have.length_of(1) + route_table2["Associations"].should.have.length_of(0) - route_table1["Associations"][1]["RouteTableAssociationId"].should.equal( + route_table1["Associations"][0]["RouteTableAssociationId"].should.equal( association_id1 ) - route_table1["Associations"][1]["Main"].should.equal(False) - route_table1["Associations"][1]["RouteTableId"].should.equal(route_table1_id) - route_table1["Associations"][1]["SubnetId"].should.equal(subnet.id) + route_table1["Associations"][0]["Main"].should.equal(False) + route_table1["Associations"][0]["RouteTableId"].should.equal(route_table1_id) + route_table1["Associations"][0]["SubnetId"].should.equal(subnet.id) # Replace Association association_id2 = client.replace_route_table_association( @@ -341,15 +334,15 @@ def test_route_table_replace_route_table_association(): ][0] # Validate - route_table1["Associations"].should.have.length_of(1) - route_table2["Associations"].should.have.length_of(2) + route_table1["Associations"].should.have.length_of(0) + route_table2["Associations"].should.have.length_of(1) - route_table2["Associations"][1]["RouteTableAssociationId"].should.equal( + route_table2["Associations"][0]["RouteTableAssociationId"].should.equal( association_id2 ) - route_table2["Associations"][1]["Main"].should.equal(False) - route_table2["Associations"][1]["RouteTableId"].should.equal(route_table2_id) - route_table2["Associations"][1]["SubnetId"].should.equal(subnet.id) + route_table2["Associations"][0]["Main"].should.equal(False) + route_table2["Associations"][0]["RouteTableId"].should.equal(route_table2_id) + route_table2["Associations"][0]["SubnetId"].should.equal(subnet.id) # Replace Association is idempotent association_id_idempotent = client.replace_route_table_association( @@ -376,6 +369,52 @@ def test_route_table_replace_route_table_association(): ex.value.response["Error"]["Code"].should.equal("InvalidRouteTableID.NotFound") +@mock_ec2 +def test_route_table_replace_route_table_association_for_main(): + client = boto3.client("ec2", region_name="us-east-1") + ec2 = boto3.resource("ec2", region_name="us-east-1") + + vpc = ec2.create_vpc(CidrBlock="10.0.0.0/16") + new_route_table_id = ec2.create_route_table(VpcId=vpc.id).id + + # Get main route table details + main_route_table = client.describe_route_tables( + Filters=[ + {"Name": "vpc-id", "Values": [vpc.id]}, + {"Name": "association.main", "Values": ["true"]}, + ] + )["RouteTables"][0] + main_route_table_id = main_route_table["RouteTableId"] + main_route_table_association_id = main_route_table["Associations"][0][ + "RouteTableAssociationId" + ] + + # Replace Association + new_association = client.replace_route_table_association( + AssociationId=main_route_table_association_id, RouteTableId=new_route_table_id + ) + new_association_id = new_association["NewAssociationId"] + + # Validate the format + new_association["AssociationState"]["State"].should.equal("associated") + + # Refresh + main_route_table = client.describe_route_tables( + RouteTableIds=[main_route_table_id] + )["RouteTables"][0] + new_route_table = client.describe_route_tables(RouteTableIds=[new_route_table_id])[ + "RouteTables" + ][0] + + # Validate + main_route_table["Associations"].should.have.length_of(0) + new_route_table["Associations"].should.have.length_of(1) + new_route_table["Associations"][0]["RouteTableAssociationId"].should.equal( + new_association_id + ) + new_route_table["Associations"][0]["Main"].should.equal(True) + + @mock_ec2 def test_route_table_get_by_tag(): ec2 = boto3.resource("ec2", region_name="eu-central-1") @@ -950,14 +989,12 @@ def test_associate_route_table_by_gateway(): ] )["RouteTables"] - # First assocation is the main - verify[0]["Associations"][0]["Main"].should.equal(True) + verify[0]["Associations"].should.have.length_of(1) - # Second is our Gateway - verify[0]["Associations"][1]["Main"].should.equal(False) - verify[0]["Associations"][1]["GatewayId"].should.equal(igw_id) - verify[0]["Associations"][1]["RouteTableAssociationId"].should.equal(assoc_id) - verify[0]["Associations"][1].doesnt.have.key("SubnetId") + verify[0]["Associations"][0]["Main"].should.equal(False) + verify[0]["Associations"][0]["GatewayId"].should.equal(igw_id) + verify[0]["Associations"][0]["RouteTableAssociationId"].should.equal(assoc_id) + verify[0]["Associations"][0].doesnt.have.key("SubnetId") @mock_ec2 @@ -977,11 +1014,9 @@ def test_associate_route_table_by_subnet(): ] )["RouteTables"] - # First assocation is the main - verify[0]["Associations"][0].should.have.key("Main").equals(True) + verify[0]["Associations"].should.have.length_of(1) - # Second is our Gateway - verify[0]["Associations"][1]["Main"].should.equal(False) - verify[0]["Associations"][1]["SubnetId"].should.equals(subnet_id) - verify[0]["Associations"][1]["RouteTableAssociationId"].should.equal(assoc_id) - verify[0]["Associations"][1].doesnt.have.key("GatewayId") + verify[0]["Associations"][0]["Main"].should.equal(False) + verify[0]["Associations"][0]["SubnetId"].should.equals(subnet_id) + verify[0]["Associations"][0]["RouteTableAssociationId"].should.equal(assoc_id) + verify[0]["Associations"][0].doesnt.have.key("GatewayId")