Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(@aws-cdk/aws-ec2-alpha): vpc.addInternetGateway cannot handle multiple subnets with shared routetable #33672

Open
1 task
domestic-fennec opened this issue Mar 2, 2025 · 1 comment
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@domestic-fennec
Copy link
Contributor

domestic-fennec commented Mar 2, 2025

Describe the bug

If a VPC has 2 public subnets that share a single routing table, calling vpc.addInternetGateway with no options will attempt to add a route to the IGW to the route table for each subnet. This causes the cloudformation update to fail with a message like:

rtb-0c507470f627d6415|0.0.0.0/0 already exists in stack
I presume this error is coming from ec2 itself because it doesn't like 2 routes with the destination in a single route table.

It is possible to work around this by using seperate route tables for each subnet and whilst it's generally good practice to use one route table per subnet, shared route tables are legal.

It'a also possible to manually supplying single subnet to vpc.addInternetGateway, but it looks confusing semantically.

    const publicRouteTable = new ec2a.RouteTable(this, "RouteTable", {
      vpc: this.vpc,
    });

    const publicSubnetA = new ec2a.SubnetV2(this, "PublicSubnetA", {
      // abridged 
      routeTable: publicRouteTable,
    });

    const publicSubnetB = new ec2a.SubnetV2(this, "PublicSubnetB", {
     // abridged
      routeTable: publicRouteTable,
    });

   this.vpc.addInternetGateway({
     subnets: [publicSubnetA]
   });

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

That the vpc.addInternetGateway should not try to add identical routes to the same routetable.

Current Behavior

That the vpc.addInternetGateway tries to add identical routes to the same routetable.

Reproduction Steps

import * as cdk from "aws-cdk-lib";
import { Construct } from "constructs";

import * as ec2a from "@aws-cdk/aws-ec2-alpha";
import * as ec2 from "aws-cdk-lib/aws-ec2";

export interface NetworkProps {
  primaryAz: string;
  secondaryAz: string;
}

export class Network extends Construct {
  public readonly vpc: ec2a.VpcV2;

  constructor(scope: Construct, id: string, props: NetworkProps) {
    super(scope, id);

    this.vpc = new ec2a.VpcV2(this, "Vpc", {
      primaryAddressBlock: ec2a.IpAddresses.ipv4("192.168.0.0/20"),
    });

    const publicRouteTable = new ec2a.RouteTable(this, "RouteTable", {
      vpc: this.vpc,
    });

    const publicSubnetA = new ec2a.SubnetV2(this, "PublicSubnetA", {
      vpc: this.vpc,
      availabilityZone: props.primaryAz,
      subnetType: ec2.SubnetType.PUBLIC,
      ipv4CidrBlock: new ec2a.IpCidr("192.168.0.0/25"),
      routeTable: publicRouteTable,
    });

    const publicSubnetB = new ec2a.SubnetV2(this, "PublicSubnetB", {
      vpc: this.vpc,
      availabilityZone: props.secondaryAz,
      subnetType: ec2.SubnetType.PUBLIC,
      ipv4CidrBlock: new ec2a.IpCidr("192.168.0.128/25"),
      routeTable: publicRouteTable,
    });

   this.vpc.addInternetGateway();
  }
};

Possible Solution

As vpc.addInternetGateway internally iterates over subnets, and the method it uses internally is vpc.addDefaultInternetRoute, I'm not sure where the cleanest place to fix this is.
Maybe addInternetGateway should iterate the subnets to produce a unique list of route tables, and addDefaultInternetRoute should operate on route tables not subnets, as semantically the route is added to the table, not the subnet. You would need some way of telling addDefaultInternetRoute if it needed to added optional IPv6 routes as well.

Here is very niave approach:

  /**
   * Adds a new Internet Gateway to this VPC
   *
   * @default - creates a new route for public subnets(with all outbound access) to the Internet Gateway.
   */
  public addInternetGateway(options?: InternetGatewayOptions): void {
    if (this._internetGatewayId) {
      throw new Error("The Internet Gateway has already been enabled.");
    }

    const igw = new InternetGateway(this, "InternetGateway", {
      vpc: this,
      internetGatewayName: options?.internetGatewayName,
    });

    this._internetConnectivityEstablished.add(igw);
    this._internetGatewayId = igw.routerTargetId;

    // Use Map to ensure unique RouteTables, value is "hasIpV6CidrBlock"
    const routeTables = new Map<RouteTable, boolean>();

    // Add routes for subnets defined as an input
    if (options?.subnets) {
      // Use Set to ensure unique subnets
      const processedSubnets = new Set<string>();
      const subnets = flatten(options.subnets.map((s) => this.selectSubnets(s).subnets));
      subnets.forEach((subnet) => {
        if (!processedSubnets.has(subnet.node.id)) {
          if (!this.publicSubnets.includes(subnet)) {
            Annotations.of(this).addWarningV2(
              "InternetGatewayWarning",
              `Subnet ${subnet.node.id} is not a public subnet. Internet Gateway should be added only to public subnets.`,
            );
          }
          // If any previous subnet on this routetable has ipv6 then keep the ipv6 flag
          routeTables.set(subnet.routeTable, !!routeTables.get(subnet.routeTable) || !!subnet.ipv6CidrBlock);
          processedSubnets.add(subnet.node.id);
        }
      }); // If there are no input subnets defined, default route will be added to all public subnets
    } else if (!options?.subnets && this.publicSubnets) {
      this.publicSubnets.forEach((publicSubnets) =>
        routeTables.set(publicSubnets.routeTable, !!publicSubnets.ipv6CidrBlock),
      );
    }

    routeTables.forEach((ipv6Required, routeTable) =>
      this.addDefaultInternetRoute(routeTable, ipv6Required, igw, options),
    );
  }

  /**
   * Adds default route for the internet gateway
   * @internal
   */
  private addDefaultInternetRoute(
    routeTable: RouteTable,
    ipv6Required: boolean,
    igw: InternetGateway,
    options?: InternetGatewayOptions,
  ): void {
    // Add default route to IGW for IPv6
    if (ipv6Required) {
      new Route(this, `${subnet.node.id}-DefaultIPv6Route`, {
        routeTable: subnet.routeTable,
        destination: options?.ipv6Destination ?? "::/0",
        target: { gateway: igw },
        routeName: "CDKDefaultIPv6Route",
      });
    }
    // Add default route to IGW for IPv4
    new Route(this, `${subnet.node.id}-DefaultRoute`, {
      routeTable: subnet.routeTable,
      destination: options?.ipv4Destination ?? "0.0.0.0/0",
      target: { gateway: igw },
      routeName: "CDKDefaultIPv4Route",
    });
  }

Additional Information/Context

No response

CDK CLI Version

2.1000.3

Framework Version

No response

Node.js Version

v20.13.1

OS

OSX 15.3.1

Language

TypeScript

Language Version

TypeScript 5.6.3

Other information

No response

@domestic-fennec domestic-fennec added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 2, 2025
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Mar 2, 2025
@pahud
Copy link
Contributor

pahud commented Mar 3, 2025

Hi @domestic-fennec

Thank you for the report.

The issue is occurring when a VPC has multiple public subnets that share the same route table. When vpc.addInternetGateway() is called:

  1. It iterates through each subnet and adds default routes (for IPv4 and IPv6) to the subnet's route table
  2. When multiple subnets share a single route table, it tries to create the same route multiple times
  3. This causes CloudFormation to fail with an error like: rtb-0c507470f627d6415|0.0.0.0/0 already exists in stack

Root Cause

The current implementation in vpc-v2-base.ts doesn't check if a route for a specific route table has already been created. It simply iterates over all subnets and adds routes to each subnet's route table, which causes duplication when multiple subnets share the same route table.

Possible Solution

Off the top of my head, one of the possible solutions is:

  • Change the implementation to first collect all unique route tables
  • Then add routes to each unique route table only once
  • Keep the original addDefaultInternetRoute method for backward compatibility but mark it as deprecated.

This solution addresses the issue by identifying unique route tables among the subnets and only adding routes to each route table once, preventing the duplication that was causing the CloudFormation error.

Making this a p1 and we welcome PRs as well.

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 3, 2025
@shikha372 shikha372 self-assigned this Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

No branches or pull requests

3 participants