nested dynamic block after running tf0.12upgrade isn't usable

[tbugfinder]

Terraform Version

Terraform v0.12.19
+ provider.azurerm v1.40.0
+ provider.local v1.4.0
+ provider.random v2.2.1

Terraform Configuration Files

resource "azurerm_virtual_machine" "vm-linux" {
  count = var.is_windows_image != "true" ? 1 : 0

  name                = local.unique_vm_name
  location            = var.location
  resource_group_name = var.resource_group_name

  delete_data_disks_on_termination = var.delete_data_disks_on_termination
  delete_os_disk_on_termination    = var.delete_os_disk_on_termination
  network_interface_ids            = [azurerm_network_interface.ni.id]
  vm_size                          = var.size
  zones                            = var.zones

  boot_diagnostics {
    enabled = var.enable_boot_diagnostics
    storage_uri = var.enable_boot_diagnostics == "true" ? join(
      ",",
      azurerm_storage_account.boot-diagnostics.*.primary_blob_endpoint,
    ) : ""
  }

  os_profile {
    computer_name  = coalesce(var.hostname, var.name)
    admin_username = var.admin_username
    admin_password = var.admin_password
    custom_data    = var.custom_data
  }

  dynamic "os_profile_linux_config" {
    for_each = [var.os_config]
    content {
      # TF-UPGRADE-TODO: The automatic upgrade tool can't predict
      # which keys might be set in maps assigned here, so it has
      # produced a comprehensive set here. Consider simplifying
      # this after confirming which keys can be set in practice.

      disable_password_authentication = os_profile_linux_config.value.disable_password_authentication

      dynamic "ssh_keys" {
        for_each = lookup(os_profile_linux_config.value, "ssh_keys", [])

        content {
          key_data = ssh_keys.value.key_data
          path     = ssh_keys.value.path
        }
      }
    }
  }

  dynamic "storage_image_reference" {
    for_each = [var.image]
    content {
      # TF-UPGRADE-TODO: The automatic upgrade tool can't predict
      # which keys might be set in maps assigned here, so it has
      # produced a comprehensive set here. Consider simplifying
      # this after confirming which keys can be set in practice.

      id        = lookup(storage_image_reference.value, "id", null)
      offer     = lookup(storage_image_reference.value, "offer", null)
      publisher = lookup(storage_image_reference.value, "publisher", null)
      sku       = lookup(storage_image_reference.value, "sku", null)
      version   = lookup(storage_image_reference.value, "version", null)
    }
  }

  storage_os_disk {
    name              = "${local.unique_name}-os"
    caching           = "ReadWrite"
    create_option     = "FromImage"
    managed_disk_type = var.os_disk_storage_account_type
  }

  tags = merge(
    var.tags,
    {
      "namemmm" = local.unique_vm_name
    },
  )
}

Debug Output

Crash Output

Expected Behavior

terraform plan finishes successfully and applies upgraded code :-)

Actual Behavior

After upgrade initial terraform plan output is:

Error: Invalid function argument

  on module/main.tf line 77, in resource "azurerm_virtual_machine" "vm-linux":
  77:         for_each = lookup(os_profile_linux_config.value, "ssh_keys", [])

Invalid value for "default" parameter: the default value must have the same
type as the map elements.

This code was upgraded from 0.11.14 to HCL2 using terraform 0.12. As added by the tool there is a post upgrade task included. Unfortunately I couldn't figure out how to resolve it yet. dynamic "ssh_keys" is a map, so I can convert to it, however it's dynamic and it might exists or might not.

I found a post by @apparentlymart and switched that for_each to for_each = { for v in os_profile_linux_config.value : v => v }. After that the error message is:

------------------------------------------------------------------------

Error: Unsupported attribute

  on module/main.tf line 84, in resource "azurerm_virtual_machine" "vm-linux":
  84:           key_data = ssh_keys.value.key_data
    |----------------
    | ssh_keys.value is "false"

This value does not have any attributes.

Error: Unsupported attribute

  on module/main.tf line 85, in resource "azurerm_virtual_machine" "vm-linux":
  85:           path     = ssh_keys.value.path
    |----------------
    | ssh_keys.value is "false"

This value does not have any attributes.

Steps to Reproduce

Additional Context

References

[tbugfinder]

Input to os_config is

  os_config = {
    disable_password_authentication = false
  }

So there's no map of ssh_keys set, it should be skipped.

[tbugfinder]

https://gist.github.com/tbugfinder/deec8a3f0d04f55834c09d455673bc38

[apparentlymart]

Hi @tbugfinder,

Unfortunately because this sort of dynamic block construction wasn't actually intentionally supported in 0.11 and was only working due to a number of validation bugs and coincidences, the upgrade process for this particular usage is just making a best effort based on common patterns we saw in a review of examples shared on GitHub and via the Terraform Registry. It looks like your 0.11 config here was doing something slightly different that the upgrade tool can't handle.

I'm honestly not sure exactly how Terraform 0.11 was interpreting what you had before, but from looking at your full example it seems like your goal is for os_config to be optional and to generate the block only if the caller sets it. The more common pattern that the upgrade tool is assuming here is that your optional argument would be a list and thus an empty list would be a suitable fallback if it isn't set.

In your case it seems to be a single nested object, and so we need a slightly different approach. I think the following is the smallest possible 0.12 adaptation of what you had, which I'd recommend while you're completing the upgrade process just to minimize the risk of accidental changes in behavior. (You might want to adjust this to use the 0.12 features in a different way later, but better to get your migration completed first and then iterate on the details.)

      dynamic "ssh_keys" {
        for_each = contains(keys(os_profile_linux_config.value), "ssh_keys") ? [os_profile_linux_config.value.ssh_keys] : []

        content {
          key_data = ssh_keys.value.key_data
          path     = ssh_keys.value.path
        }
      }

The contains condition in the above should return true only if the ssh_key key is present, in which case the overall result will be a single-element list containing your ssh_keys object, producing in turn a single ssh_keys block. Otherwise the overall result will be an empty list and so no ssh_keys blocks will be produced at all.

That conditional is admittedly quite awkward. There's a new feature in the master branch planned for inclusion in the forthcoming 0.12.20 release which could make it a little more concise:

        for_each = try([os_profile_linux_config.value.ssh_keys], [])

This new try function will try to evaluate each of its arguments in turn and return the first one that doesn't produce any errors. In this case that means it'll first try to construct a single-element list containing the ssh_keys value, but if that fails (likely because ssh_keys doesn't exist) then it'll try to construct an empty list, which always succeeds. This is the sort of feature where it can really hurt understandability if overused, but I expect it'll be handy for situations like this where a module is receiving a data structure whose shape isn't predictable.

[tbugfinder]

Thank You @apparentlymart - that's the solution.

[apparentlymart]

Great, thanks for following up @tbugfinder!

Since terraform 0.12upgrade is essentially frozen now (to avoid situations where folks who upgraded at different times might see subtly different results), I'm going to close this out.

I think even if we were still improving the upgrade tool, there wouldn't really be a good way to infer this because static analysis here (which is what the upgrade tool is relying on) doesn't have enough information to determine the final type of ssh_keys. Indeed, this sort of ambiguity is why the tool tends to generate a TF-UPGRADE-TODO in this situation, so that the person running the upgrade can consider what types they are expecting and adjust as needed. :confounded:

Thanks for reporting this, and for confirming that the proposed solution worked!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.