Skip to content

fix(terraform): two orphan azurerm modules contain invalid HCL — fix or delete #147

@cristim

Description

@cristim

Summary

Two terraform modules under terraform/modules/ are not instantiated by any environment but contain HCL bugs that block terraform validate:

  • terraform/modules/compute/azure/cleanup-function/
  • terraform/modules/registry/azure/

The bugs are dynamic blocks wrapped around scalar attributes. dynamic is only valid for nested blocks (the kind that take { ... } content); wrapping it around a scalar attribute (like a boolean or string) is rejected by Terraform regardless of which azurerm provider version is in use.

These were silently broken until the new pre-commit CI gate (PR #105) started running terraform_validate against every module. As a temporary measure, PR #105 excludes both module paths from the terraform_validate hook in .pre-commit-config.yaml with a comment pointing at this issue. The terraform_tflint hook still runs against them and is clean — only validate is bypassed.

The actual bugs

compute/azure/cleanup-function/main.tf

Two dynamic blocks around scalar attributes on azurerm_linux_function_app:

# Line 65 — vnet_route_all_enabled is a scalar bool on site_config,
# not a nested block. The `dynamic` wrapper makes Terraform expect a
# block of that name, which doesn't exist in the provider schema.
dynamic "vnet_route_all_enabled" {
  for_each = var.subnet_id != "" ? [1] : []
  content {
    vnet_route_all_enabled = true
  }
}

# Line 96 — virtual_network_subnet_id is a scalar string on the
# function-app resource, not a nested block.
dynamic "virtual_network_subnet_id" {
  for_each = var.subnet_id != "" ? [1] : []
  content {
    virtual_network_subnet_id = var.subnet_id
  }
}

The intent in both cases is "set this attribute conditionally on whether var.subnet_id is non-empty," which is just:

vnet_route_all_enabled    = var.subnet_id != ""
virtual_network_subnet_id = var.subnet_id != "" ? var.subnet_id : null

registry/azure/main.tf

Three dynamic blocks around azurerm_container_registry policy attributes:

# Line 10
dynamic "quarantine_policy_enabled" { ... }

# Line 18
dynamic "retention_policy" { ... }

# Line 26
dynamic "trust_policy" { ... }

In azurerm 4.x these are scalar/sub-attribute structures rather than top-level nested blocks; in 3.x the shapes were different yet again. The dynamic wrappers don't match either schema. The intent ("only set these when SKU is Premium") collapses to plain conditional attribute assignments.

Why these weren't caught earlier

Before this PR the pre-commit terraform_validate hook was never running in CI (only locally, and the terraform binary was missing on the runner so it skipped silently). Local developers running pre-commit would have seen these errors, but the modules aren't called by any environment — the Azure environment instantiates database/azure, email/azure, compute/azure/container-apps, etc., but never these two — so a developer touching unrelated terraform code never had a reason to step into them.

The .terraform.lock.hcl files committed alongside (azurerm 4.61.0) suggest these modules were init'd at least once against a recent azurerm to seed the lock, but actual validate was probably never run against the dynamic-block syntax.

Fix

Either:

  1. Fix the two modules — rewrite the offending dynamic blocks as plain conditional attribute assignments (~10 lines per module), regenerate the lock files, remove the terraform_validate exclusion in .pre-commit-config.yaml. Verify the fixed modules validate cleanly under both azurerm ~> 4.0 and the convention ~> 3.0 used elsewhere — pick whichever matches the actual resource shapes still in use.

  2. Delete the modules — if cleanup-function and registry/azure are truly orphan code with no plan to instantiate them, remove them outright along with their .tflint.hcl/versions.tf/etc. and the exclusion. Cleaner than carrying broken code forward.

My read: option 2 if you have no near-term plan for either module, option 1 if you intend to wire them up. Either way, the exclusion in .pre-commit-config.yaml should be removed as part of closing this.

References

Severity

Low — not affecting any deployed infrastructure. The cost is operational (broken-by-default modules sit in the tree) and a permanent .pre-commit-config.yaml exclusion that will rot if other modules need similar treatment in the future.

Effort

Small if option 2 (delete), Medium if option 1 (fix + validate against the right azurerm version).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions