#terraform-aws-modules (2022-05)

I think it is fair to say we do not have that on our Roadmap. @Erik Osterman (Cloud Posse) ?

Not on our roadmap at this time

@Jeremy G (Cloud Posse) is doing some major rewrites on this maybe it can be factored in

I was wondering if it was something worth trying to refactor and submit. Whether it was being able to set the default route using either a gateway_id or transit_gateway_id or simply make something like enable_default_route and add to the count conditional statement

@Jeremy (UnderGrid Network Services) Look for a PR for dynamic-subnets later today. The general plan is for you to be able to suppress the creation of any resources it creates by default. In your case, you would disable the creation of public routes and create your own routes, route table(s), and associations to the public subnets. Does that sound like a reasonable path forward for you?

The latter being the simplest solution and just have the TGW handle adding the route along with VPC attachments

@Jeremy G (Cloud Posse) yeah that would fit the bill as it would be okay to create the VPC subnets and not have a default route but have the route table created

No, we’re not going to create the route table if we do not create the routes, because you may want to use the VPC default route table, you may want a single route table for all public subnets, or you may want a route table per subnet. The [variables.tf](http://variables.tf) file is already 400 lines long and I really want to trim down the number of options where I can.

Okay… I can get the default route table from the VPC module in that case. We use the separate private subnet route tables as is currently.

The only advantage of having the extra public route table is that it actually requires explicitly associating subnets to it so if the default were left alone it would have no routes if a rogue subnet were created and jot associated to a route table

I understand. This is why you might want to create your separate own route table and associate it with the public subnets. It’s not that hard, but the combination of options gets very hard to manage inside the module.

Oh yeah… I’m actually writing a local VPC module that we call that wraps the VPC and subnet modules so adding a route table and using it as needed wouldn’t be a problem. I’ve managed to convince team to use Cloudposse modules as much as possible and this is the one complexity that been left to me to resolve

At one point I was thinking I could accept a named default route, which would solve your use case, but then I realized I would have to accept one for all AZs as well as one per AZ and then you would have to dynamically generate the map of AZs to routes and by then it doesn’t seem like it’s helping enough to be worth the added complexity.

I was actually happy to see the VPC change to be able to support IPAM as I’d been trying to use it myself and not much out there supported it

We still do not have IPAM set up in our test environment, so none of that stuff has been tested. Please let us know how it goes.

I don’t see much of a use case for multiple public route tables. Private certainly but public subnets would got out a GW that’s usually already multi-AZ

The main use case for a public route table per AZ is IPv6 with NAT64, where you still need a NAT Gateway per AZ for reliability.

Ah yeah you’re right… I hadn’t considered NAT64 as I assume IPv6 is always routable and not NAT like RFC1918 IPv4 space

That’s partly why we have to ensure IPv6 is completely disabled

@Jeremy G (Cloud Posse) you’re not talking about changing how the private subnets are setup are you, just public?

OK. Our impetus for the dynamic-subnets update is adding IPv6 support, so I’ve been thinking a lot about it. With IPv6 and NAT64 you can bring up an IPv6-only public subnet and retain connectivity to IPv4 backend services. Or, more likely, use a dual-stack subnet but only assign IPv4 addresses to the load balancer and use only IPv6 internally.

The private subnets will have IPv6 support added, and some cleanup, but should mostly be the same. I will ask you to try out the PR version once it is ready and welcome your feedback.

Sure thing… I was just asking as you said you wouldn’t be creating route tables and resources if it was disabled as I’d need to, juat wanted to make sure private subnet resource creation would remain as is as I’m not disabling any of that other than just not creating NAT GW or Instances

If you do not create a NAT destination for the private subnets, then we are not going to create route tables or routes because we have nowhere to point them to.

Okay I can work with that… I was just adding the necessary routes to the private subnet route tables to accommodate not using NAT GW

I’m not that familiar with provisioning route tables, maybe you can help me with the design. My assumption was that each subnet can only be associated with a single route table, which is why I did not want to create empty route tables for the subnets. If that’s wrong, and subnets can be associated with multiple route tables, then maybe I will continue to create the for the private subnets.

Technically we are running a 3-tier subnetting scheme (public, private, data) but as we only have public in the security VPC we were making the dynamic-subnets module work and using the public subnets as our private and private subnets as our data.

Yeah… the subnet is only associated with a single route table. When you have NAT GW or NAT Instances you would typically have each private subnet use its own route table that had thr NAT GW in the same AZ for HA redundancy. IGW and TGW are not AZ specific so using a single route table works

Right, that’s why the current module creates a route table per AZ for private but only one for public.

Yeah the route table setup as is isn’t the issue I’m butting heads with. It’s just the aws_route for the 0.0.0.0/0 destination that requires the IGW be defined

Right. Hence the compromise I came up with to keep this manageable: limited options for configuring what is created, but the ability to suppress creation of specific resources and supply your own replacement.

If the igw_id variable were optional not required and that one resource only was created of it was non-null would accomplish the same result. I can work with however you go and will definitely review for you

Because Terraform has a plan phase, we cannot condition the creation of a resource based on the value of an input that might be generated in the same plan. Instead, there is a new input, public_network_route_enabled, that you set to false to inhibit the creation of any routing setup for the public subnets.

That sounds like it would work. We’ll handle our route table manipulation from the calling module that bonds the VPC and subnet modules

@Jeremy (UnderGrid Network Services) I opened a draft PR (159) for you to start looking at. Additional documentation and maybe some other cleanup to come. It should serve your use case if you leave public_network_route_enabled at its default value of true but do not supply igw_id. In this case it will create the route table but no routes.

@Jeremy G (Cloud Posse) I’ll run it through it’s paces with our build out I’m working on currently. I’ll get back with any findings on Monday. Enjoy the weekend

@Jeremy (UnderGrid Network Services) I made some changes which I hope address your concerns.

@Jeremy G (Cloud Posse) okay I’ll have a look in a little bit.

@Jeremy G (Cloud Posse) I tested with a new VPC creation and upgrading from an older version of the module and in both instances the outcome was as I’d expect. You technically would never have an “empty” route table as it will always have a local route for the VPC CIDR so not having an IGW or NAT GW or Instance just makes a VPC that only routes locally which is a valid use-case scenario. In fact one could be an HA cluster that needs a private network for heartbeat or sync communication only between instances in multiple AZs in the region. How it is operating now allows that functionality.

Now I may end up submitting a PR for the terraform-aws-vpc module as well as I’ve identified some missing functionality. I’m going to leave it for Monday though and try to enjoy the rest of my weekend. I also need to dig into the terraform-aws-transit-gateway module some to better understand it and how to utilize it. I have had some issue with the 0.9.0 version of it and have had to pin back to 0.6.1 for now.

@Jeremy (UnderGrid Network Services) please tag me in a new thread in #terraform-aws-modules that explains your concerns with the terraform-aws-vpc module as I am also working on it and might be willing to add the features you need without you opening a PR.

Thank you for the feedback on dynamic-subnets.

Sure thing @Jeremy G (Cloud Posse) glad I could help. Estimate on how long that branch will remain draft so I can plan on when to change current reference method?

I think the code is not going to change much if at all, what is missing are the release notes and migration document. Not sure how long it is going to take. For now, you can refer to it using registry syntax as version v2.0.0-rc1

@Jeremy G (Cloud Posse) what would your thoughts be on being able to make the attributes value for the private_label and public_label configurable with the defaults being as they are now?

Use case situation I have in mind is like we currently have VPC setup that is 2 tier “private” and “data” but our “private” tier is setup as if public while our “data” tier is setup as if private using the module. We set a tag that specifies as “private” or “data” but the Name tag is obviously using “public” and “private”. Further we have a VPC yet to build out that has 4 subnet tiers where 1 would be setup as if “public” with an IGW but the other would be setup as if “private” and with the attribute being configurable could label them accordingly.

Thoughts? I wouldn’t consider this as a breaking change provided the default was set to maintain current label names with the option to change it.

That sounds reasonable. I will add it.

the naming discrepancy was pointed out when I was in a huddle with teammates sharing progress and then when I started relooking at the module after you mentioned the ipv4_cidrs option considered I could make use of it for the 4 tier VPC if the label were configurable

I’m actually trying to programmatically generate the ipv4_cidrs value to pass in based on the desired VPC CIDR block within locals {} and if I output the local variable results it looks correct but when I try passing it through the plan responds the given value is not suitable for child module variable "ipv4_cidrs" ... : list of object required.

Are you passing in a list with 1 object?

I think I got it worked out… I had tried

ipv4_cidrs = {
  private = local.private_cidrs
  public = local.public_cidrs
}

but it seems I needed to do:

ipv4_cidrs = [{
  private = local.private_cidrs
  public = local.public_cidrs
}]

both local.private_cidrs and local.public_cidrs were lists of CIDR blocks

@Jeremy (UnderGrid Network Services) v2.0.0-rc2 is available with public_label and private_label inputs.

I see the documentation on the private_label and public_label now… looks good but I’ll roll it out and confirm

@Jeremy (UnderGrid Network Services) v2.0.0-rc4 released. Please upgrade to it and let us know if you find issues.

I’ll upgrade and test… what was identified between rc2 and rc4 to watch for?

@Jeremy G (Cloud Posse) if passing in public_route_table_ids with a single route table ID do I still need to set public_route_table_per_subnet_enabled to false or will leaving it as null behave as if I hadn’t passed the public_route_table_ids?

@Jeremy (UnderGrid Network Services) From the description of public_route_table_ids:

If provided, it overrides public_route_table_per_subnet_enabled.

From the description of public_route_table_enabled:

Ignored if public_route_table_ids is non-empty.

@Jeremy (UnderGrid Network Services) wrote:
what was identified between rc2 and rc4 to watch for? • Fix for max_subnet_count greater than provided number of availability zones • Add max_nats to enable creating fewer NAT Gateways/instances than there are subnets.

@Jeremy (UnderGrid Network Services) Release candidate 5 is out. https://github.com/cloudposse/terraform-aws-dynamic-subnets/compare/2.0.0-rc4...2.0.0-rc5

@Jeremy G (Cloud Posse) just got home and checked over the diff compare. I’ll update my environment I’m working in and confirm.

Thank you

No problem glad to help, and it’s helping me out. Just been up since 5am as I had to drive wife and MIL up to South GA to lay FIL to rest

I saw that v2.0.0 was blessed and out the door

Yes

I updated our code to use the 2.0.0 version this afternoon after seeing that

PR reviewers like seeing non “RC” version strings

Think I found a bug… public.tf line 65 … the aws_route.public has a count looking for local.private4_enabled and believe that should be local.public4_enabled

same thing on lines 107 and 121 for the aws_network_acl_rule for public4_ingress and public4_egress

this looks like it go introduced between rc4 and rc5

Yes, you are probably right. I will review in the morning when I’m not so tired

Have you found any other issues so far?

No that was the only thing i found when I yries to update from rc4 to 2.0.0 I noticed ot was wanting to destroy those resources.

@Jeremy (UnderGrid Network Services) Fixed in https://github.com/cloudposse/terraform-aws-dynamic-subnets/pull/162

I’m mobile right now at EPCOT but PR looks good giving it a cursory glance. Will look again when back on system.

@Jeremy (UnderGrid Network Services) Thank you. I am not familiar with how VPC Endpoints work, so I very much welcome your input on this.

As you can see in our example, we expect the user of the VPC module to perform the route table associations. It’s not clear to me that you would necessarily want all the endpoints attached to the same route table, so then you have to figure which associations to make.

Because we want to maintain backward compatibility, we cannot just add a route_table_id member to the input objects, so we would have to add new input objects that just take route table IDs and then use those to create the associations. At that point it seems just as onerous to create the inputs as it does to create the aws_vpc_endpoint_route_table_association resources themselves (which are just 2 lines), with the advantage of the latter being full control and transparency.

I agree that it would have been better to allow for an optional route table ID from the start, but at this point I do not think it is worth a breaking change or the hassle of the non-breaking change.

If you feel strongly about it and want to submit a PR anyway, I would accept (keeping in mind that I am far from the sole arbiter of such things) a PR that adds 2 new inputs: • gateway_vpc_endpoint_route_table_ids • interface_vpc_endpoint_route_table_ids which are both map(string) and take the same keys as the corresponding endpoint inputs and then create appropriate aws_vpc_endpoint_route_table_association resources. But as I said, I don’t think it is worth the trouble, just as I don’t think it is worth the trouble to create Security Groups in the module.

The route_table_ids is only valid for Gateway endpoints just as subnet_ids is only valid for Interface and GatewayLoadBalancer endpoints. The latter isn’t even supported by the vpc-endpoints sub module as far as I can tell. They are both optional and passing them to the aws_vpc_endpoint resource simply foregoes having to use the aws_vpc_endpoint_route_table_association and aws_vpc_endpoint_subnet_association respectively.

I was attempted to add the aws_vpc_endpoint_route_table_association using both count and for_each while passing the dynamic-subnets private_route_table_ids and in both cases ran into the issue with standing up a new VPC where it was saying the value was not known until apply and hadn’t gotten it worked around which was when I dug into the sub module more.

The roadmap for the AWS Terraform provider cals for moving away from inline resources (such as passing the route table ID directly to aws_vpc_endpoint) and toward managing as much as possible with distinct resources such as aws_vpc_endpoint_route_table_association. So we want to use that resource regardless.

Unless you supply a fixed list of AZs or AZ IDs to dynamic-subnets, the number of private subnets (and therefore route tables) created will not be known at plan time, because it will depend on the number of AZs in the region.

So I see your dilemma. You want to stand up all of this in one root module, and cannot create the associations because they have unknown cardinality. The only solution is the inline setting.

@Jeremy (UnderGrid Network Services) So I guess you think we should support the GatewayLoadBalancer as well. What about auto_accept? Should we add that as an input? Does it apply to all endpoint types?

I’ve not yet worked with the GatewayLoadBalancer endpoints yet myself. As I’ve read the docs on the auto_accept option it still only works with accepting services within the same AWS account not cross-account but that said I’ve not had to use it with any of the VPC endpoints I’ve used which are mostly Gateway or Interface service types. The VPC endpoints are really good for working with private only subnets and being able to reach AWS services without NAT or IGW routing available which has been my primary use case for them. It’s nice to use the S3 Gateway VPC endpoint and then be able to lock down an S3 bucket to only allow traffic from a specific VPC for DLP.

Do you have auto-accept turned on at the Org level via Resource Access Manager?

not currently… Actually our Org setup is something I’m meaning to bring up with manager and higher up as I noticed we currently only have it enabled for consolidated billing and not much else so not really utilizing the Org features. In my own AWS Org I have it enabled and generally just have RAM resources shared to entire Org not just specific accounts

So it seems to me we should allow you to set auto_accept , shouldn’t we?

If want the sub module to be as full featured and usable for anyone I’d agree and say yes. I may not use it but someone else might and if it’s available they would then have it. It’s kind of like the route_table_ids in that it’s optional don’t need to use as you could then add aws_vpc_endpoint_route_table_association resources but if it was an optional variable you could use it and just have it passed along to the aws_vpc_endpoint resource upon creation. Both work but currently only one is functional. Now if I were working with an Interface service the subnet_ids is already available but optional so I could use it or just create an aws_vpc_endpoint_subnet_association resource.

Honestly, the vpc-endpoint sub module could have a case made for it being it’s own module and become more full featured

There’s no real reason to make it a standalone module. You can use the submodule without using the root module.

I’d have to research when the auto_accept would actually be needed… I know with the Gateway endpoints s3 and dynamodb that it isn’t necessary

OK. This seems like something you are going to be better at than I would, so I will let you take the lead on making changes and opening a PR on the vpc module/submodule.

@Jeremy (UnderGrid Network Services) Any further comments on dynamic-subnets?

@Jeremy G (Cloud Posse) it’s been working fine for our general VPCs and for our special case VPC that needs multiple subnet tiers I’m being able to work with it creatively

I would like to combine any breaking changes to the VPC endpoints submodule into this release. Are you going to get to that soon?

This is a visual representation of what I’m attempting to setup for the TGW but will actually have 4-5 spokes not just 2 as depicted

@Erik Osterman (Cloud Posse) @Jeremy G (Cloud Posse) who would know more about the transit-gateway module design?

@Andriy Knysh (Cloud Posse) ?

I think I’ve found a major flaw/issue that is making this network topology not work with the transit-gateway module… seems like it is opinionated in that it expects both route table association and propagation and has no means to associate a TGW VPC attachment with a route table but not propagate it and instead propagate separate TGW VPC attachment routes.

As in the diagram, I need to associate the mgmt VPC with the mgmt TGW RT but it only needs to propagate the spoke 1 & 2 TGW VPC attachments. The Sec RT is the same in that it is associated with the Sec VPC but only propagates the spoke 1 & 2 TGW VPC attachments. The Spoke RT is associated with all the spoke VPCs but propagates the mgmt TGW VPC as well as a static route pointing to the Sec VPC

@Jeremy (UnderGrid Network Services) We build these modules for the use cases our customers have, which up until how has been the fairly simple task of connecting private subnets in different VPCs in different accounts. We also want to keep the modules relatively easy to understand and easy to use. Your use case is rarer and more complex.

If you want to enhance the module to handle your use case, before you go ahead and create a full-blown PR for it, I suggest you open a PR that proposes the new inputs you would add and how you would preserve backward compatibility, and how they would address your use case. Then we can review that and see if it makes sense for us to incorporate in the module.

yeah this is a use-case where all traffic has to flow throw a firewall between VPCs and to egress/ingress outside AWS. I’ve been looking into the module itself and I’m not sure how, or even if, separating association & propagation could be done in a backwards compatible way given its design of 1:1 relation between the two. A VPC attachment has a 1:1 association relationship but can have a 1:M propagation relationships

@Ben Smith (Cloud Posse) @Leo Przybylski Do you have any comments to add on TGW?

egress/ingress outside AWS Does this mean you have services and/or users outside AWS that need to access services on AWS behind a private VPC?

Hi jeremy,

(just a quick heads up, this channel is for discussions related to https://github.com/terraform-aws-modules - which is a different org than cloudposse, just use #terraform next time )

I don’t believe the context supports re-mapping the tags but you can override the tags using something like this

# ...

  context = module.this.context

  tags = {
    App = module.this.namespace
  }
}

Take a look at [package.tf](http://package.tf) of that module, but specifically https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/master/package.tf#L6-L29

All resources in that file have same condition:

count = var.create && var.create_package ? 1 : 0

and the ZIP archive gets created each time the content (that is source code) of the files you are passing in source_path var changes.

You need to ensure you are structuring your Lambda code well enough and feed it into source_path (as example you ref. is showing). When that data element data.external.archive_prepare in [package.tf](http://package.tf) is stacked, it has info about your ZIP file which gets used in this sequence then:

1. https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/master/main.tf#L6
2. https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/master/main.tf#L12
3. https://github.com/terraform-aws-modules/terraform-aws-lambda/blob/master/main.tf#L49 Line 49 in [main.tf](http://main.tf) ensures that Lambda resource (stacks Lambda environment) takes local ZIP file for deployment. Depending on what you do with var.publish , you may stamp new version of Lambda each time there is configuration or code change through terraform apply .