Mutual references between two modules produces "Error: Cycle" if they refer to each other's instances using `each.key`

If I iterate over a set using for_each when calling two modules, I can refer to the output of the other module instance as long as the module's values / resources / outputs don't cause a cycle error. So this can work just fine:

module "one" {
  for_each = toset(["item"])
  source = "../modules/test"
  input1 = module.two["item"].output
  input2 = "one"
}

module "two" {
  for_each = toset(["item"])
  source = "../modules/test"
  input1 = module.one["item"].output
  input2 = "two"
}

It continues to work if I modify either of the module instance input1 values to reference the other instance using [each.key]. However, it breaks if I modify both of the module instance input1 values to reference the other instance using [each.key]. The following example causes Terraform to exit with a cycle error:

module "one" {
  for_each = toset(["item"])
  source = "../modules/test"
  input1 = module.two[each.key].output
  input2 = "one"
}

module "two" {
  for_each = toset(["item"])
  source = "../modules/test"
  input1 = module.one[each.key].output
  input2 = "two"
}

Terraform Version

Terraform v1.1.9 on darwin_arm64

Terraform Configuration Files

https://github.com/raffraffraff/terraform_for_each_module_cycle

Debug Output

I have included the debug output for each terraform apply command in the above project. See the working1, working2 and notworking directories in the project.

Expected Behavior

Data-driven Terraform code should produce consistent results. If two modules iterate over the same data, they should continue to be able to refer to each other using each.key to identify the module instance output.

Actual Behavior

Referencing the output of the other module instance using each.key causes Terraform to exit with a cycle error.

Steps to Reproduce

git clone https://github.com/raffraffraff/terraform_for_each_module_cycle
cd terraform_for_each_module_cycle/notworking
terraform init
TF_LOG=TRACE terraform apply

Hi @raffraffraff! Thanks for reporting this.

From what you've described, it seems to be expected for Terraform to return a cycle error here, at least with Terraform's current intended design.

The reason this occurs is that you're now (in your second example) dynamically deciding the key for accessing an element of module.two and module.one, and so Terraform cannot statically determine which element you're depending on and must therefore conservatively depend on all of them.

Your first example worked because you gave Terraform enough statically-visible information to know exactly which element of module.two and module.one you're accessing, and therefore Terraform can narrow down to exactly one output value to depend on, rather than depending on the entire module. That more precise dependency graph avoids the dependency cycle.

I think that what you expected to happen here was for Terraform to notice that regardless of which element of module.two or module.one is ultimately selected, the subsequent .output statically selects a particular output value and so in principle Terraform could generate a less conservative dependency graph where the input1 value for each of the modules depends on all of the output "output" blocks across all of the module instances, rather than depending on the module blocks themselves.

That analysis is beyond the capabilities of Terraform's static analysis today, but it does seem like it could be possible in principle with a more sophisticated static analysis algorithm. For that reason, I'm going to reclassify this as an enhancement request so that we can use this issue to discuss possible designs that might achieve that result, and to collect use-cases for it that will help to gauge the impact of addressing it and thus help with prioritization.

The reproduction case you included here is useful to demonstrate the behavior, but because it's a contrived example it doesn't explain why you want to have two modules referring to one another in this way. To help with the prioritization aspect, it would be helpful if you could add another comment here saying more about specifically what your pair of modules represents in your real system, which will then help us to understand how common a problem that is and to imagine similar related problems that might all have a common solution. We typically use example use-cases to evaluate multiple potential solutions to a problem.

To anyone else that finds this issue in future and has a problem that seems to suggest a similar solution: please also leave a comment explaining the situation you are trying to model.

Thanks!

Hi @apparentlymart, you're right this was a contrived example designed to provide simple code that you could run without depending on provider authentication etc. There are real-world examples. One I encountered recently is shown in this module example: https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/examples/eks_managed_node_group/main.tf

Specifically:

module "eks" references module.vpc_cni_irsa.iam_role_arn
module "vpc_cni_irsa" references module.eks.oidc_provider_arn

There is no way to follow the example code and use a data-driven approach the can create zero-or-more EKS clusters by iterating over this type of data:

eks_clusters:
  production:
    enable_irsa: true
    key: val

...because it would fail as soon as I introduce for_each = var.eks_clusters and attempt to reference the other module using each.key (which should resolve to the same cluster name, but would fail as we have already seen).

I encountered a similar issue with S3 buckets a while back, and I think it has the same root cause. I can try to prove this out with a proper example, but for now, let's assume that I want to iterate over a map of s3 bucket configs to deploy them:

s3_buckets:
  bucketA:
    replication_destination: "bucketB"
  bucketB:
    replication_destination: "bucketA"

^ That extremely simplified example should be possible after AWS refactored their aws_s3_bucket resources, breaking out most configuration options into standalone resources (eg: versioning, acls, replication). But with this bug, there is no way to write an S3 module that can iterate over a map of s3_buckets, create the buckets, and create a replication configuration, ie: the following code would probably fail:

module "s3" {
  for_each = var.s3_buckets
  replication_destination = module.s3[each.value.replication_destination].id
}

...even though, on a purely resource level, there are no circular dependencies. The aws_s3_bucket resources can all be created, and then the aws_s3_bucket_replication_configuration resources can be created afterwards, referencing the target bucket ID.

It might seem like expected behavior to fail when each module refers to the other module's output using each.key, because in many languages, two variables that have identical values are still treated as different objects. So the idea that "my each.key isn't the same thing as your each.key" makes a certain sense.

But that's not what is happening here. In this case, Terraform encounters a situation that triggers a "give up!" rule, even if the modules do not contain a cycle error. The rule appears to be: "if both modules refer to each other's outputs using any non-hard-coded value, exit with cycle error". The module ref doesn't have to be each.key, it can be any non-hard-coded value, even a static string:

locals {
  static_string = "one"
  my_list = ["one"]
}

module "one" {
  for_each = toset(local.my_list)
  source = "../modules/test"
  input1 = module.two[local.static_string].output
}

module "two" {
  for_each = toset(local.my_list)
  source = "../modules/test"
  input1 = module.one[local.static_string].output
}

So I disagree with the statement that "this is expected behavior", unless you're only speaking on behalf of Terraform developers who are intimitely familiar the code. As a user, it's unexpected that one of these should work and the other should fail:

module.one["one"].output
module.one[local.static_string].output

Hi @raffraffraff,

In this context, "expected behavior" means that Terraform is working in the way it was designed to work. That doesn't mean that there isn't a better design to be found and implemented, but we use the enhancement request process to discuss changes to Terraform's design, and hence this is classified as an enhancement request to reflect that.

As you've seen, the current design is that Terraform's static analysis of references is conservative in any case where the full path to the target object isn't a constant. That includes indexing with any reference at all; only a constant key value is acceptable for Terraform to infer the precise dependency relationship.

This issue is where we can discuss both different designs to allow for less conservative analysis (including how it would work, not only what the outcome would be) and the use-cases that would be enabled by each possible design, to help prioritize.

In the meantime, you will need to find a different way to express the design you intended to create here, so that it either uses only constant instance keys or it avoids mutual dependencies between the two modules. For example, some dependency cycle situations (but certainly not all) can be resolved by introducing a third object that both of the existing objects will depends on.

hashicorp / terraform