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

Imported VPC Subnets show as Private (while they're public) #3706

Closed
javydekoning opened this issue Aug 19, 2019 · 6 comments · Fixed by #3784
Closed

Imported VPC Subnets show as Private (while they're public) #3706

javydekoning opened this issue Aug 19, 2019 · 6 comments · Fixed by #3784
Assignees
Labels
bug This issue is a bug.

Comments

@javydekoning
Copy link

🐛 Bug Report

What is the problem?

When I import a VPC with a single public subnet, CDK says there are no Public Subnets. Specifically: There are no 'Public' subnets in this VPC. Use a different VPC subnet selection.

I'm using this to deploy a public ALB.

Reproduction Steps

Here's my route-table:

{
    "RouteTables": [
        {
            "Associations": [
                {
                    "Main": true,
                    "RouteTableAssociationId": "rtbassoc-xxxxxx",
                    "RouteTableId": "rtb-xxxxxx"
                },
                {
                    "Main": false,
                    "RouteTableAssociationId": "rtbassoc-xxxxxx",
                    "RouteTableId": "rtb-xxxxxx",
                    "SubnetId": "subnet-xxxxxx"
                }
            ],
            "PropagatingVgws": [],
            "RouteTableId": "rtb-xxxxxx",
            "Routes": [
                {
                    "DestinationCidrBlock": "10.0.2.0/26",
                    "Origin": "CreateRoute",
                    "State": "active",
                    "VpcPeeringConnectionId": "pcx-xxxxxx"
                },
                {
                    "DestinationCidrBlock": "10.0.1.0/24",
                    "GatewayId": "local",
                    "Origin": "CreateRouteTable",
                    "State": "active"
                },
                {
                    "DestinationCidrBlock": "0.0.0.0/0",
                    "GatewayId": "igw-xxxxxx",
                    "Origin": "CreateRoute",
                    "State": "active"
                }
            ],
            "Tags": [],
            "VpcId": "vpc-xxxxxx",
            "OwnerId": "xxxxxx"
        }
    ]
}

Stack:

export class SampleStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, { env: { account: "xxxxxx", region: "eu-west-1" } });

    const vpcpub = ec2.Vpc.fromLookup(this, "pub", {
      vpcId: "vpc-xxxxx"
    });

    const lb = new elb.ApplicationLoadBalancer(this, "alb", {
      vpc: vpcpub,
      internetFacing: true
    });

    const listener = lb.addListener("pub", { port: 80, open: true });

    listener.addTargets("ecs", {
      port: 8000,
      targets: []
    });

    new cdk.CfnOutput(this, "LoadBalancerDNS", {
      value: lb.loadBalancerDnsName
    });
  }
}

Verbose Log

CDK_CONTEXT_JSON: 
{
   "vpc-provider:account=xxxxxx:filter.vpc-id=vpc-xxxxxx:region=eu-west-1":{
      "vpcId":"vpc-xxxxxx",
      "availabilityZones":[
         "eu-west-1a"
      ],
      "privateSubnetIds":[
         "subnet-xxxxxx"
      ],
      "privateSubnetNames":[
         "Private"
      ],
      "privateSubnetRouteTableIds":[
         "rtb-xxxxxx"
      ]
   }
   "aws:cdk:enable-path-metadata":true,
   "aws:cdk:enable-asset-metadata":true
}

Environment

  • CDK CLI Version: 1.4.0
  • Module Version: 1.4.0
  • OS: MacOS 10.14.6
  • Language: TypeScript
@javydekoning javydekoning added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 19, 2019
@rix0rrr rix0rrr self-assigned this Aug 21, 2019
@reeseyc
Copy link

reeseyc commented Aug 23, 2019

Have you tagged your subnets? CDK requires your subnets are tagged when you import them

aws-cdk:subnet-type - (Public/Private/Isolated)

@javydekoning
Copy link
Author

No I didn't, is there any documentation on this? My search skills seem to be letting me down.

@reeseyc
Copy link

reeseyc commented Aug 23, 2019

Hahahaa dude if you want documentation don't use CDK.
Your subnets need to be tagged to load them
Key name is aws-cdk
then the subnet type in the field. (Public/Private/Isolated)

there is some information about it in this bug report (which is how i figured it out).

#3407

rix0rrr added a commit that referenced this issue Aug 26, 2019
The VPC importer used to look at `MapPublicIpOnLaunch` as a shortcut
for determining whether a Subnet was public or private.

The correct way to do it is to look at the route table and see if
it includes an Internet Gateway.

Fixes #3706.
@rix0rrr rix0rrr added service/vpc and removed needs-triage This issue or PR still needs to be triaged. labels Aug 27, 2019
@fulghum
Copy link
Contributor

fulghum commented Aug 27, 2019

@reeseyc – sorry to hear you haven't had a good experience with CDK documentation. We want to fix that! Let us know if you have any specific feedback on what you liked, what you didn't, where you found gaps, etc. The more specific/concrete (i.e. in which API ref, which Construct, where in the Developer Guide, workshop, etc), the more likely we can get it fixed.

@reeseyc
Copy link

reeseyc commented Aug 28, 2019

@fulghum Hi Jason; we are facing 2 major challenges with CDK documentation.
Firstly (and most challenging) is the lack of examples (especially working examples).
Secondly, although the API documentation is actually pretty good, the nuances of using CDK are not documented. For instance, take the VPCfromlookup method - where does it say in the documentation that the subnets need to be tagged for this to work properly (as per above)?
Happy to be corrected if i am not looking in the right place

@vaneek
Copy link

vaneek commented Aug 28, 2019

I find the requirement to tag subnets to be an arbitrary fix and a big impediment to adopting CDK. Often times VPCs and network related items are owned and managed by others and if every subnet in every VPC in every account has to be retro-tagged in order for a CDK app to function properly, the response will be something like, "it works with terribleform" . A big anti-pattern in my view, but hey just my 2p.

rix0rrr added a commit that referenced this issue Aug 28, 2019
The VPC importer used to look at `MapPublicIpOnLaunch` as a shortcut
for determining whether a Subnet was public or private.

The correct way to do it is to look at the route table and see if
it includes an Internet Gateway.

Fixes #3706.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants