Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions cli/src/pcluster/aws/ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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-"):
Expand Down
82 changes: 82 additions & 0 deletions cli/tests/pcluster/aws/test_ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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