Problem using for_each and indexing

I have the following code:

data "aws_vpc" "main" {
  for_each = var.vpcs
  id    = each.key
}

resource "mongodbatlas_network_peering" "main" {
  for_each = var.vpcs
  project_id             = "${mongodbatlas_project.main.id}"
  container_id           = "${mongodbatlas_network_container.main.container_id}"
  provider_name          = "AWS"
  accepter_region_name   = "us-east-1"
  aws_account_id         = "${data.aws_caller_identity.current.account_id}"
  route_table_cidr_block = data.aws_vpc.main[each.key].cidr_block
  vpc_id                 = each.key
}

resource "aws_vpc_peering_connection_accepter" "peer" {
  for_each = var.vpcs
  vpc_peering_connection_id = "${mongodbatlas_network_peering.main[each.key].connection_id}"
  auto_accept               = true
}

But trying to access mongodbatlas_network_peering.main with each.key throws an error of:

74:   vpc_peering_connection_id = "${mongodbatlas_network_peering.main[each.key].connection_id}"
    |----------------
    | each.key is "vpc-xxxxxxxxx"
    | mongodbatlas_network_peering.main is tuple with 2 elements

The given key does not identify an element in this collection value: a number
is required

From my understanding since I was using a for_each with mongodbatlas_network_peering I should be able to access that via the var.vpcs map key. The data.aws_vpc.main object seems to work fine that way.

For reference the var.vpcs object is:

variable vpcs {
  description = "List of vpc and route table ids to pair mongo atlas with"
  type = map(object({
    route_table_ids = list(string)
  }))
}

Hi @nathankodilla!

I’m not sure exactly what’s causing that error, since indeed as you said it looks like it should be a map.

With that said, I have a suggestion for a different way to express what you wanted to express there which I think is a more direct description of your intent and will hopefully also avoid the problem you encountered:

resource "mongodbatlas_network_peering" "main" {
  for_each = data.aws_vpc.main

  project_id             = mongodbatlas_project.main.id
  container_id           = mongodbatlas_network_container.main.container_id
  provider_name          = "AWS"
  accepter_region_name   = "us-east-1"
  aws_account_id         = data.aws_caller_identity.current.account_id
  route_table_cidr_block = each.value.cidr_block
  vpc_id                 = each.key
}

resource "aws_vpc_peering_connection_accepter" "peer" {
  for_each = mongodbatlas_network_peering.main

  vpc_peering_connection_id = each.value.connection_id
  auto_accept               = true
}

The idea here is to use the resource values themselves as the for_each basis for the downstream resources. We’re saying “create one network peering per VPC” and then “create one peering connection accepter per network peering”, and then they can cascade by using each.value to access the upstream object corresponding to each key.

This should work because each of these resource values should be a map, as you noted. However, I’ll be interested to see if this different formulation produces a similar error for you, since it seems like for some reason Terraform has concluded that mongodbatlas_network_peering.main is a sequence (as if count was set) rather than a mapping. :thinking:

@apparentlymart thanks that seems to have solved this issue I was experiencing for this part of code. However, there is another part that uses the [each.key]:

locals {
  vpc_routetables = flatten([
    for vpc_key, vpc in var.vpcs : [
      for route_table_id in vpc.route_table_ids : {
        vpc_id = vpc_key
        route_table_id  = route_table_id
      }
    ]
  ])
}

resource "aws_route" "peer" {
  for_each = local.vpc_routetables
  route_table_id            = each.value.route_table_id
  destination_cidr_block    = var.mongo_cidr
  vpc_peering_connection_id = mongodbatlas_network_peering.main[each.value.vpc_id].connection_id
  depends_on                = ["aws_vpc_peering_connection_accepter.peer"]
}

First issue is the flatten is incorrect and gives me The given "for_each" argument value is unsuitable: the "for_each" argument must be a map, or set of strings, and you have provided a value of type tuple.. So I am unclear, what I am doing wrong with the flatten. Additionally, in the code you provided how should I access mongodbatlas_network_peering.main in the aws_route resource?

Thanks

When you use for_each you need to project the value into a map so that the keys of the map can be used as unique identifiers for the aws_route.peer instances. Since each route table belongs to only one VPC I think the route table ids alone should be sufficient unique identifiers, so we can project it to a map:

resource "aws_route" "peer" {
  for_each = { for rt in local.vpc_routetables : rt.route_table_id => rt }

  # (and all of the other arguments as before)
}

With that in place, your expression mongodbatlas_network_peering.main[each.value.vpc_id].connection_id will hopefully work as you wrote it, though since we didn’t yet manage to explain the original problem you were seeing it might be that it will crop up again. If so, hopefully we’ll get some more information from a new example that will help debug what’s going on.

@apparentlymart thanks again for the new info. And yep as expected, the same error pops up but with no new information. Just The given key does not identify an element in this collection value: a number is required. again.

I put terraform in debug log mode and did find these log lines:

019/11/06 17:18:25 [TRACE] Completed graph transform *terraform.RootTransformer with new graph:
module.kong.mongodbatlas_network_peering.main - *terraform.NodeRefreshableManagedResourceInstance
module.kong.mongodbatlas_network_peering.main["vpc-xxxxxxxx"] - *terraform.NodeRefreshableManagedResourceInstance
module.kong.mongodbatlas_network_peering.main["vpc-yyyyyyyy"] - *terraform.NodeRefreshableManagedResourceInstance
module.kong.mongodbatlas_network_peering.main[1] - *terraform.NodeRefreshableManagedResourceInstance
  module.kong.mongodbatlas_network_peering.main - *terraform.NodeRefreshableManagedResourceInstance
root - terraform.graphNodeRoot
  module.kong.mongodbatlas_network_peering.main["vpc-xxxxxxx"] - *terraform.NodeRefreshableManagedResourceInstance
  module.kong.mongodbatlas_network_peering.main["vpc-yyyyyyy"] - *terraform.NodeRefreshableManagedResourceInstance
  module.kong.mongodbatlas_network_peering.main[1] - *terraform.NodeRefreshableManagedResourceInstance
...
2019/11/06 17:18:25 [ERROR] module.kong: eval: *terraform.EvalSequence, err: Invalid index: The given key does not identify an element in this collection value: a number is required.
2019/11/06 17:18:25 [ERROR] module.kong: eval: *terraform.EvalDiff, err: Invalid index: The given key does not identify an element in this collection value: a number is required.

So given the beginning portion of those logs it does appear to be making the correct map for mongodbatlas_network_peering.main that is accessible via [“vpc-xxxxx”]. But why later on it cannot access it that way is the true question here.

I see there’s a module.kong.mongodbatlas_network_peering.main[1] instance here. Did you previously use count for this resource and have now switched to using for_each?

It looks like somehow an ambiguous situation has arisen where both numbered and stringed instances are present at once. Normally Terraform should react to that by treating the numeric one as removed and planning to remove it, but it seems like something is preventing Terraform from doing so in this case.

If that module.kong.mongodbatlas_network_peering.main[1] is redundant (that is, if the object it’s representing is also represented by one of the instances with a string key) then you may be able to address the ambiguity by asking Terraform to forget about that [1] instance entirely:

terraform state rm 'module.kong.mongodbatlas_network_peering.main[1]'

If that instance is representing a remote object that still exists and isn’t covered by one of the others, you could either delete it manually in the remote system after asking Terraform to forget it, or you could make a note of its id before forgetting it and then re-import it at a suitable string key so that Terraform will treat it as if it were created by for_each.

It is strange that Terraform isn’t proposing a solution to this automatically; I assume there’s something unusual going on here that isn’t visible to me. If the above helps you work around it then I’d be inclined to leave this as an unresolved mystery for now and wait to see if it arises again, in which case we can compare notes with what you’ve shared here and see what the two (or more) situations have in common.

@apparentlymart spot on! Yes it use to use count and then was transitioned to a for_each. I did perform the single state rm you suggested however that alone did not get me up and running. I had to be a bit more aggressive with it and remove all states for module.kong.mongodbatlas_network_peering and delete the underlying resources, which was fine to do in this case. After doing all that, it appears to be all up and running!