diff --git a/cli/src/pcluster/aws/ec2.py b/cli/src/pcluster/aws/ec2.py index 57f576ee8f..30783e645a 100644 --- a/cli/src/pcluster/aws/ec2.py +++ b/cli/src/pcluster/aws/ec2.py @@ -572,6 +572,18 @@ def is_subnet_public(self, subnet_id): if not route_tables: raise Exception("No route tables found. The subnet or VPC configuration may be incorrect.") + # Find the main route table (subnets without explicit association use the main route table) + main_route_table = next( + (rt for rt in route_tables if any(assoc.get("Main") for assoc in rt.get("Associations", []))), + None, + ) + if main_route_table is None: + raise Exception( + f"No main route table found for VPC {vpc_id}. " + "Please explicitly associate the subnet with a route table." + ) + route_tables = [main_route_table] + # Check if any route contains an internet gateway (igw) for route in route_tables[0].get("Routes", []): if "GatewayId" in route and route["GatewayId"].startswith("igw-"): diff --git a/cli/tests/pcluster/aws/test_ec2.py b/cli/tests/pcluster/aws/test_ec2.py index 719e6d4e37..9dcc86385f 100644 --- a/cli/tests/pcluster/aws/test_ec2.py +++ b/cli/tests/pcluster/aws/test_ec2.py @@ -679,6 +679,30 @@ def get_describe_route_tables_mocked_request(subnet_id, gateway_id): ) +def get_describe_route_tables_empty_mocked_request(subnet_id): + return MockedBoto3Request( + method="describe_route_tables", + response={"RouteTables": []}, + expected_params={"Filters": [{"Name": "association.subnet-id", "Values": [subnet_id]}]}, + ) + + +def get_describe_route_tables_by_vpc_mocked_request(vpc_id, route_tables): + return MockedBoto3Request( + method="describe_route_tables", + response={"RouteTables": route_tables}, + expected_params={"Filters": [{"Name": "vpc-id", "Values": [vpc_id]}]}, + ) + + +def get_describe_subnets_for_vpc_mocked_request(subnet_id, vpc_id): + return MockedBoto3Request( + method="describe_subnets", + response={"Subnets": [{"SubnetId": subnet_id, "VpcId": vpc_id}]}, + expected_params={"SubnetIds": [subnet_id]}, + ) + + def test_is_subnet_public(boto3_stubber): # First boto3 call. The subnet should be private subnet_id = "subnet-12345678" @@ -698,3 +722,61 @@ def test_is_subnet_public(boto3_stubber): # Third boto3 call. The result should be from the latest response even if the gateway id of the subnet is different assert AWSApi.instance().ec2.is_subnet_public(subnet_id) is True + + +def test_is_subnet_public_with_main_route_table(boto3_stubber): + # Test when subnet has no explicit route table association (uses main route table) + # This tests the bug fix: should use main route table, not route_tables[0] + subnet_id = "subnet-no-explicit-assoc" + vpc_id = "vpc-12345678" + + route_tables = [ + # First route table (non-main, no IGW) - bug would incorrectly use this + { + "RouteTableId": "rtb-private", + "Associations": [{"Main": False, "SubnetId": "subnet-other"}], + "Routes": [{"DestinationCidrBlock": "10.0.0.0/16", "GatewayId": "local"}], + }, + # Main route table with IGW - correct one to use + { + "RouteTableId": "rtb-main", + "Associations": [{"Main": True}], + "Routes": [ + {"DestinationCidrBlock": "10.0.0.0/16", "GatewayId": "local"}, + {"DestinationCidrBlock": "0.0.0.0/0", "GatewayId": "igw-12345678"}, + ], + }, + ] + + mocked_requests = [ + get_describe_route_tables_empty_mocked_request(subnet_id), + get_describe_subnets_for_vpc_mocked_request(subnet_id, vpc_id), + get_describe_route_tables_by_vpc_mocked_request(vpc_id, route_tables), + ] + boto3_stubber("ec2", mocked_requests) + + # Should return True because main route table has IGW + assert AWSApi.instance().ec2.is_subnet_public(subnet_id) is True + + +def test_is_subnet_public_main_route_table_no_igw(boto3_stubber): + # Test when main route table has no IGW (private subnet) + subnet_id = "subnet-private" + vpc_id = "vpc-12345678" + + route_tables = [ + { + "RouteTableId": "rtb-main", + "Associations": [{"Main": True}], + "Routes": [{"DestinationCidrBlock": "10.0.0.0/16", "GatewayId": "local"}], + }, + ] + + mocked_requests = [ + get_describe_route_tables_empty_mocked_request(subnet_id), + get_describe_subnets_for_vpc_mocked_request(subnet_id, vpc_id), + get_describe_route_tables_by_vpc_mocked_request(vpc_id, route_tables), + ] + boto3_stubber("ec2", mocked_requests) + + assert AWSApi.instance().ec2.is_subnet_public(subnet_id) is False