Create secondary cidr block on first pass?

[JasonCubic]

How can you make a secondary ipv4 cidr block for the vpc without needing to do a second pass and add it to an already created vpc? I want to create a vpc with two cidr blocks. A cidr block with a private IP block and an on-prem cidr block. I've tried it but it fails with this error:

│ Error: Invalid count argument
│
│ on .terraform\modules\secondary_vpc\data.tf line 132, in data "aws_vpc" "main":
│ 132: count = local.create_vpc ? 0 : 1
│
│ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created.
│ To work around this, use the -target argument to first apply only the resources that the count depends on.

It's possible that I am overthinking this. Should I just use the normal terraform aws_vpc_ipv4_cidr_block_association?

[drewmullen]

Hi thanks for opening this question. We actually use the resource _cidr_block_association.

The issue probably comes from the fact that the count in question is a complex local that considers whether or not vpc_id is passed in. In this circumstance youre passing the vpc_id in from another resource that has yet to be built which means its computed, which is causing a race condition.

Part of me wants to say this is a bug but im split on it without more research. I'll leave this open for discussion and consideration. This was a good find and a miss on our design. Thanks for opening this up!

[drewmullen]

A fix may honestly be as easy as creating a variable var.create_vpc and using that instead of inferring from vpc_id == null... i can do some testing

I could even just take the inverse of the var.vpc_secondary_cidr instead... same outcome 🤔

[JasonCubic]

This is the main.tf I am trying to use and getting that error. I am open to any ideas for improvement.

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "~> 5.31"
    }
  }
}

provider "aws" {
  region     = "us-west-2"
  access_key = var.access_key
  secret_key = var.secret_key
  token      = var.token
}

# https://github.com/aws-ia/terraform-aws-vpc
module "primary_vpc" {
  source     = "aws-ia/vpc/aws"
  version    = ">= 4.2.0"
  name       = "multi-az-vpc"
  cidr_block = "172.16.0.0/20"
  az_count   = 3
  subnets = {
    public = {
      cidrs                     = ["172.16.1.0/24", "172.16.2.0/24", "172.16.3.0/24"]
      nat_gateway_configuration = "all_azs" # options: "single_az", "none"
    }
    private = {
      name_prefix             = "private_workload"
      connect_to_public_natgw = true
      cidrs                   = ["172.16.4.0/22", "172.16.8.0/22", "172.16.12.0/22"]
    }
  }
}

# To add in the secondary cidr
module "secondary_vpc" {
  source     = "aws-ia/vpc/aws"
  version    = ">= 4.2.0"
  depends_on = [module.primary_vpc]
  name       = "secondary-cidr"
  cidr_block = "10.200.0.0/26" # pretend this is a private IP cidr range
  az_count   = 3

  vpc_secondary_cidr       = "true"
  vpc_id                   = module.primary_vpc.vpc_attributes.id
  vpc_secondary_cidr_natgw = module.primary_vpc.natgw_id_per_az

  subnets = {
    private = {
      name_prefix             = "private_internal"
      cidrs                   = ["10.200.0.0/28", "10.200.0.16/28", "10.200.0.32/28"]
      connect_to_public_natgw = true
    }
  }
}

[drewmullen]

Thanks for sharing your sample HCL. What you have is fine! Now that 4.4.2 is released i can test my fix idea... standby

[erezo9]

@drewmullen any updates on this? would really be helpful to assigne secondary cidr on first pass

[mperyer1]

Is there any update on this? it's a blocker on using this module with AFT Code Pipelines

[drewmullen]

I have sometime tomorrow I can try this out

v1.9 fixes this problem with inferencing, according to the alpha notes

[alexohima]

@drewmullen Looking forward for this update, it is indeed a blocker for AFT

[drewmullen]

hmm, my idea did not fix the problem... this needs to be dug in deeper to figure out...

I cloned the vpc module locally and used relative pathing to do a quick and dirty test. the code has been pushed: https://github.com/drewmullen/vpc-test-changes/blob/master/main.tf

new errors:

│ Error: Invalid for_each argument
│ 
│   on terraform-aws-vpc/main.tf line 232, in resource "aws_subnet" "private":
│  232:   for_each = toset(try(local.private_per_az, []))
│     ├────────────────
│     │ local.private_per_az will be known only after apply
│ 
│ The "for_each" set includes values derived from resource attributes that cannot be determined until apply, and so
│ Terraform cannot determine the full set of keys that will identify the instances of this resource.
│ 
│ When working with unknown values in for_each, it's better to use a map value where the keys are defined statically in
│ your configuration and where only the values contain apply-time results.
│ 
│ Alternatively, you could use the -target planning option to first apply only the resources that the for_each value
│ depends on, and then apply a second time to fully converge.
╵
╷
│ Error: Invalid for_each argument
│ 
│   on terraform-aws-vpc/main.tf line 256, in resource "aws_route_table" "private":
│  256:   for_each = toset(try(local.private_per_az, []))
│     ├────────────────
│     │ local.private_per_az will be known only after apply
│ 
│ The "for_each" set includes values derived from resource attributes that cannot be determined until apply, and so
│ Terraform cannot determine the full set of keys that will identify the instances of this resource.
│ 
│ When working with unknown values in for_each, it's better to use a map value where the keys are defined statically in
│ your configuration and where only the values contain apply-time results.
│ 
│ Alternatively, you could use the -target planning option to first apply only the resources that the for_each value
│ depends on, and then apply a second time to fully converge.

[mperyer1]

@drewmullen the following worked for me.

Added a create_vpc variable:

variable "create_vpc" {
  description = "VPC ID to use if not creating VPC."
  default     = true
  type        = bool
}

Updated local.create_vpc to use the value from the variable rather than the vpc_id:
create_vpc = var.create_vpc ? true : false

[drewmullen]

@mperyer1 interesting, that’s similar to what I did

feel free to send up a PR. Include a test payload you used to deploy a primary and secondary cidr pls

we will have to add a test too

[alexohima]

@drewmullen I have the same issue as you but only if I use transit_gateway_routes

Error: Invalid for_each argument
--
870 |  
871 | on .terraform/modules/secondary_cidr/main.tf line 232, in resource "aws_subnet" "private":
872 | 232:   for_each = toset(try(local.private_per_az, []))
873 | ├────────────────
874 | │ local.private_per_az will be known only after apply
875 |  
876 | The "for_each" set includes values derived from resource attributes that
877 | cannot be determined until apply, and so Terraform cannot determine the full
878 | set of keys that will identify the instances of this resource.
879 |  
880 | When working with unknown values in for_each, it's better to use a map value
881 | where the keys are defined statically in your configuration and where only
882 | the values contain apply-time results.
883 |  
884 | Alternatively, you could use the -target planning option to first apply only
885 | the resources that the for_each value depends on, and then apply a second
886 | time to fully converge.
887 |  
888 | Error: Invalid for_each argument
889 |  
890 | on .terraform/modules/secondary_cidr/main.tf line 256, in resource "aws_route_table" "private":
891 | 256:   for_each = toset(try(local.private_per_az, []))
892 | ├────────────────
893 | │ local.private_per_az will be known only after apply
894 |  
895 | The "for_each" set includes values derived from resource attributes that
896 | cannot be determined until apply, and so Terraform cannot determine the full
897 | set of keys that will identify the instances of this resource.
898 |  
899 | When working with unknown values in for_each, it's better to use a map value
900 | where the keys are defined statically in your configuration and where only
901 | the values contain apply-time results.
902 |  
903 | Alternatively, you could use the -target planning option to first apply only
904 | the resources that the for_each value depends on, and then apply a second
905 | time to fully converge.
906 |  
907 | Error: Invalid for_each argument
908 |  
909 | on .terraform/modules/secondary_cidr/main.tf line 364, in resource "aws_subnet" "tgw":
910 | 364:   for_each = contains(local.subnet_keys, "transit_gateway") ? toset(local.azs) : toset([])
911 | ├────────────────
912 | │ local.azs is a list of string, known only after apply
913 | │ local.subnet_keys is tuple with 1 element
914 |  
915 | The "for_each" set includes values derived from resource attributes that
916 | cannot be determined until apply, and so Terraform cannot determine the full
917 | set of keys that will identify the instances of this resource.
918 |  
919 | When working with unknown values in for_each, it's better to use a map value
920 | where the keys are defined statically in your configuration and where only
921 | the values contain apply-time results.
922 |  
923 | Alternatively, you could use the -target planning option to first apply only
924 | the resources that the for_each value depends on, and then apply a second
925 | time to fully converge.
926 |  
927 | Error: Invalid for_each argument
928 |  
929 | on .terraform/modules/secondary_cidr/main.tf line 381, in resource "aws_route_table" "tgw":
930 | 381:   for_each = contains(local.subnet_keys, "transit_gateway") ? toset(local.azs) : toset([])
931 | ├────────────────
932 | │ local.azs is a list of string, known only after apply
933 | │ local.subnet_keys is tuple with 1 element
934 |  
935 | The "for_each" set includes values derived from resource attributes that
936 | cannot be determined until apply, and so Terraform cannot determine the full
937 | set of keys that will identify the instances of this resource.
938 |  
939 | When working with unknown values in for_each, it's better to use a map value
940 | where the keys are defined statically in your configuration and where only
941 | the values contain apply-time results.
942 |  
943 | Alternatively, you could use the -target planning option to first apply only
944 | the resources that the for_each value depends on, and then apply a second
945 | time to fully converge.

[alexohima]

@drewmullen The problem is the calculation of local.azs within the module. By passing it as var.azs instead, both local.private_per_az and local.azs will be known. This fixed the issue for me. main...alexohima:terraform-aws-ia-vpc:fix-secondary-cidr-first-pass

[alexohima]

@JasonCubic Fix has been merged