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

fix: prevent expanding dynamic blocks injecting more than once #2291

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

hugorut
Copy link
Contributor

@hugorut hugorut commented Feb 14, 2023

Resolves issue where dynamic blocks add duplicate expanded blocks into the childBlocks of the parent module. For example:

Take the following root module definition:

data "bad_state" "bad" {}

module "reload" {
  source         = "./baz"
  input = [
    {
      "ip"        = "10.0.0.0"
      "mock" = data.bad_state.bad.my_bad["10.0.0.0/24"].id
    },
    {
      "ip"        = "10.0.1.0"
      "mock" = data.bad_state.bad.my_bad["10.0.0.0/24"].id
    }
  ]
}

which calls out to a module with the following terraform:

variable "input" {}

resource "dynamic" "resource" {
  dynamic "child_block" {
    for_each = {
    	for i in var.input : i.ip => i
    }

    content {
      bar = child_block.value.mock
      foo = child_block.value.ip
    }
  }
}

Prior to this change the length of child_block would be 4 instead of the expected 2. This is because on first evaluation data.bad_state.bad.my_bad["10.0.0.0/24"].id is null and then on second evaluation we mock it. This means that we "reload" the underlying module baz which expands the child_block again. The second expansion simply injects two more blocks into childBlocks without clearing the prior expanded blocks.

We now RemoveBlocks of all type before InjectBlock so any prior expanded blocks will be overwritten.

@hugorut hugorut self-assigned this Feb 14, 2023
Resolves issue where dynamic blocks add duplicate expanded blocks into the `childBlocks` of the parent module. For example:

Take the following root module definition:

```
data "bad_state" "bad" {}

module "reload" {
  source         = "./baz"
  input = [
    {
      "ip"        = "10.0.0.0"
      "mock" = data.bad_state.bad.my_bad["10.0.0.0/24"].id
    },
    {
      "ip"        = "10.0.1.0"
      "mock" = data.bad_state.bad.my_bad["10.0.0.0/24"].id
    }
  ]
}
```

which calls out to a module with the following terraform:

````
variable "input" {}

resource "dynamic" "resource" {
  dynamic "child_block" {
    for_each = {
    	for i in var.input : i.ip => i
    }

    content {
      bar = child_block.value.mock
      foo = child_block.value.ip
    }
  }
}
```

Prior to this change the `length` of `child_block` would be 4 instead of the expected 2. This is because on first evaluation `data.bad_state.bad.my_bad["10.0.0.0/24"].id` is `null` and then on second evaluation we mock it. This means that we "reload" the underlying module `baz` which expands the `child_block` again. The second expansion simply injects two more blocks into `childBlocks` without clearing the prior expanded blocks.

We now `RemoveBlocks` of all type before `InjectBlock` so any prior expanded blocks will be overwritten.
@hugorut
Copy link
Contributor Author

hugorut commented Feb 14, 2023

Ignore google test errors, they are unrelated and due to a regression introduced in the terraform-google-provider: hashicorp/terraform-provider-google#13731

@hugorut hugorut merged commit 2d4a16f into master Feb 15, 2023
@hugorut hugorut deleted the fix/dynamic-blocks-appending branch February 15, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants