Skip to content

Commit f08eb02

Browse files
[Bug] Fix is_subnet_public to use main route table for subnets without explicit association (#7174)
* [Bug] Fix is_subnet_public to use main route table for subnets without explicit association When a subnet has no explicit route table association, it uses the VPC's main route table. The previous implementation incorrectly used route_tables[0] which may not be the main route table, causing public subnets to be incorrectly identified as private. This fix explicitly finds the main route table by checking the "Main" flag in route table associations. If no main route table is found (which should not happen in a valid VPC), an exception is raised with a helpful message. Fixes #7173 * Use API filter association.main instead of Python filtering Refactor is_subnet_public to use AWS API filter 'association.main=true' to fetch only the main route table, instead of fetching all route tables and filtering in Python. This is more efficient and cleaner. Update test mocks to match the new API filter parameters. * Refactor main route table tests to use pytest.mark.parametrize Combine test_is_subnet_public_with_main_route_table and test_is_subnet_public_main_route_table_no_igw into a single parameterized test for better maintainability. --------- Co-authored-by: Himani Anil Deshpande <[email protected]>
1 parent 92357d9 commit f08eb02

File tree

2 files changed

+74
-1
lines changed

2 files changed

+74
-1
lines changed

cli/src/pcluster/aws/ec2.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,7 +568,9 @@ def is_subnet_public(self, subnet_id):
568568
raise Exception(f"No subnet found with ID {subnet_id}")
569569
vpc_id = subnets[0].get("VpcId")
570570

571-
route_tables = self.describe_route_tables(filters=[{"Name": "vpc-id", "Values": [vpc_id]}])
571+
route_tables = self.describe_route_tables(
572+
filters=[{"Name": "vpc-id", "Values": [vpc_id]}, {"Name": "association.main", "Values": ["true"]}]
573+
)
572574
if not route_tables:
573575
raise Exception("No route tables found. The subnet or VPC configuration may be incorrect.")
574576

cli/tests/pcluster/aws/test_ec2.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,35 @@ def get_describe_route_tables_mocked_request(subnet_id, gateway_id):
679679
)
680680

681681

682+
def get_describe_route_tables_empty_mocked_request(subnet_id):
683+
return MockedBoto3Request(
684+
method="describe_route_tables",
685+
response={"RouteTables": []},
686+
expected_params={"Filters": [{"Name": "association.subnet-id", "Values": [subnet_id]}]},
687+
)
688+
689+
690+
def get_describe_route_tables_by_vpc_mocked_request(vpc_id, route_tables):
691+
return MockedBoto3Request(
692+
method="describe_route_tables",
693+
response={"RouteTables": route_tables},
694+
expected_params={
695+
"Filters": [
696+
{"Name": "vpc-id", "Values": [vpc_id]},
697+
{"Name": "association.main", "Values": ["true"]},
698+
]
699+
},
700+
)
701+
702+
703+
def get_describe_subnets_for_vpc_mocked_request(subnet_id, vpc_id):
704+
return MockedBoto3Request(
705+
method="describe_subnets",
706+
response={"Subnets": [{"SubnetId": subnet_id, "VpcId": vpc_id}]},
707+
expected_params={"SubnetIds": [subnet_id]},
708+
)
709+
710+
682711
def test_is_subnet_public(boto3_stubber):
683712
# First boto3 call. The subnet should be private
684713
subnet_id = "subnet-12345678"
@@ -698,3 +727,45 @@ def test_is_subnet_public(boto3_stubber):
698727

699728
# Third boto3 call. The result should be from the latest response even if the gateway id of the subnet is different
700729
assert AWSApi.instance().ec2.is_subnet_public(subnet_id) is True
730+
731+
732+
@pytest.mark.parametrize(
733+
"subnet_id, routes, expected_result",
734+
[
735+
pytest.param(
736+
"subnet-no-explicit-assoc",
737+
[
738+
{"DestinationCidrBlock": "10.0.0.0/16", "GatewayId": "local"},
739+
{"DestinationCidrBlock": "0.0.0.0/0", "GatewayId": "igw-12345678"},
740+
],
741+
True,
742+
id="main route table with igw",
743+
),
744+
pytest.param(
745+
"subnet-private",
746+
[{"DestinationCidrBlock": "10.0.0.0/16", "GatewayId": "local"}],
747+
False,
748+
id="main route table without igw",
749+
),
750+
],
751+
)
752+
def test_is_subnet_public_with_main_route_table(boto3_stubber, subnet_id, routes, expected_result):
753+
# Test when subnet has no explicit route table association (uses main route table)
754+
vpc_id = "vpc-12345678"
755+
756+
route_tables = [
757+
{
758+
"RouteTableId": "rtb-main",
759+
"Associations": [{"Main": True}],
760+
"Routes": routes,
761+
},
762+
]
763+
764+
mocked_requests = [
765+
get_describe_route_tables_empty_mocked_request(subnet_id),
766+
get_describe_subnets_for_vpc_mocked_request(subnet_id, vpc_id),
767+
get_describe_route_tables_by_vpc_mocked_request(vpc_id, route_tables),
768+
]
769+
boto3_stubber("ec2", mocked_requests)
770+
771+
assert AWSApi.instance().ec2.is_subnet_public(subnet_id) is expected_result

0 commit comments

Comments
 (0)