fix(terraform): rewrite dynamic-around-scalar in two orphan azurerm modules (closes #147)

[cristim]

Summary

Closes #147.

Two terraform modules were silently broken by dynamic blocks wrapped around scalar attributes — invalid HCL in any azurerm version. Both modules are unreferenced (no environment instantiates them) so the breakage went undetected until the new pre-commit CI gate in #105 ran terraform_validate against every module.

compute/azure/cleanup-function/

• vnet_route_all_enabled is a scalar bool attribute on site_config, not a nested block. Replaced the dynamic wrapper with a plain conditional: vnet_route_all_enabled = var.subnet_id != "".
• virtual_network_subnet_id is a scalar string attribute on the function-app resource. Replaced the dynamic wrapper with var.subnet_id != "" ? var.subnet_id : null.

registry/azure/

• In azurerm 4.x (pinned at 4.61.0 via .terraform.lock.hcl) the nested retention_policy and trust_policy blocks were removed in favour of scalar attributes. Replaced all three dynamic blocks with the modern shape:

  quarantine_policy_enabled = var.sku == "Premium"
  trust_policy_enabled      = var.sku == "Premium"
  retention_policy_in_days  = var.sku == "Premium" ? var.image_retention_days : null
  

Standard module structure

Both modules also got the terraform_standard_module_structure treatment that #105 applied to 5 other modules (commit ff0a630fa):

• versions.tf — pin azurerm ~> 4.0 + terraform >= 1.10.0
• variables.tf — variables moved out of main.tf
• outputs.tf — outputs moved out of main.tf
• main.tf — resources only

Without this split, terraform_tflint flagged ~16 terraform_standard_module_structure warnings per module, blocking the pre-commit gate.

Verification

• terraform validate passes cleanly for both modules under azurerm 4.61.0.
• Full pre-commit suite green (gofmt, gosec, trivy, tflint, terraform_fmt, terraform_validate).

Follow-up

#105 (fix/supply-chain) added a .pre-commit-config.yaml exclusion pointing at this issue. Once both PRs land, the exclusion should be dropped — the terraform_validate hook will cover these modules under the standard sweep.

🤖 Generated with claude-flow

Summary by CodeRabbit

• Chores
  • Reorganized infrastructure modules for improved maintainability and clarity
  • Updated Terraform version requirements and provider constraints
  • Simplified infrastructure configuration across Azure compute and registry modules

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

Two orphan Azure Terraform modules had invalid HCL syntax with dynamic blocks wrapping scalar attributes. Both modules are refactored by extracting variables and outputs into separate files, simplifying conditional logic with proper scalar attribute assignments, and adding version constraints.

Changes

Cohort / File(s)	Summary
cleanup-function module refactoring terraform/modules/compute/azure/cleanup-function/main.tf, variables.tf, outputs.tf, versions.tf	Removed variable and output declarations from main.tf; created separate variables.tf with 11 input variables; created outputs.tf exporting function app metadata; added versions.tf with Terraform ≥1.10.0 and azurerm ~>4.0 constraints. Replaced invalid dynamic blocks with conditional scalar assignments: vnet_route_all_enabled = var.subnet_id != "" and virtual_network_subnet_id = var.subnet_id != "" ? var.subnet_id : null.
registry module refactoring terraform/modules/registry/azure/main.tf, variables.tf, outputs.tf, versions.tf	Removed 9 variable and 5 output declarations from main.tf; created separate variables.tf with all input variables including conditional ACR settings; created outputs.tf with 5 exports (registry URL, ID, name, and conditional admin credentials); added versions.tf with Terraform ≥1.10.0 and azurerm ~>4.0 constraints. Replaced three invalid dynamic blocks with conditional scalar attributes for Premium SKU features: quarantine_policy_enabled, retention_policy, and trust_policy.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Dynamic blocks were all askew,
Wrapping scalars—that just won't do!
Now variables dance in their own sweet file,
Conditionals shine with a proper style,
Two orphan modules fixed with flair,
Terraform validation now gets fair! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing invalid HCL by rewriting dynamic blocks in two azurerm modules and closing issue #147.
Linked Issues check	✅ Passed	The PR fully addresses the requirements in issue #147 by fixing both orphan modules' invalid HCL, reorganizing them into standard module structure, and verifying terraform validate passes under azurerm 4.x.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the two orphan modules and their standard module structure as required by issue #147; no out-of-scope changes detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-147-orphan-azurerm

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

terraform/modules/compute/azure/cleanup-function/main.tf (1)

115-123: ⚠️ Potential issue | 🟡 Minor

Using timestamp() causes unnecessary plan churn.

timestamp() returns the current time on every terraform plan, so start_time will always show as changed even when no real update is needed. This forces an update to the Logic App trigger on every apply.

Consider using a static value or making it a variable:

Proposed fix

 resource "azurerm_logic_app_trigger_recurrence" "cleanup" {
   name         = "cleanup-schedule"
   logic_app_id = azurerm_logic_app_workflow.cleanup_trigger.id
   frequency    = "Day"
   interval     = 1
-  start_time   = formatdate("YYYY-MM-DD'T'02:00:00'Z'", timestamp())
+  # Omit start_time to use the trigger creation time, or set a fixed date
   time_zone    = "UTC"
 }

Alternatively, add a lifecycle { ignore_changes = [start_time] } block if the initial value is acceptable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/compute/azure/cleanup-function/main.tf` around lines 115 -
123, The recurrence trigger resource
azurerm_logic_app_trigger_recurrence.cleanup uses timestamp() for start_time
which changes every plan; replace it with a stable value (either a hard-coded
RFC3339 string or a new input variable like var.cleanup_start_time) and update
any docs/variables accordingly, or keep the existing computed value but add a
lifecycle { ignore_changes = [start_time] } block on
azurerm_logic_app_trigger_recurrence.cleanup to prevent plan churn; modify the
resource block for start_time and/or add the lifecycle ignore_changes entry to
resolve the constant diffs.

🧹 Nitpick comments (1)

terraform/modules/registry/azure/variables.tf (1)

16-20: Consider adding validation for the sku variable.

The SKU value determines whether Premium features are enabled. Adding validation would catch typos early:

Optional validation block

 variable "sku" {
   description = "SKU for ACR (Basic, Standard, Premium)"
   type        = string
   default     = "Standard"
+
+  validation {
+    condition     = contains(["Basic", "Standard", "Premium"], var.sku)
+    error_message = "SKU must be one of: Basic, Standard, Premium."
+  }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/registry/azure/variables.tf` around lines 16 - 20, The sku
variable currently accepts any string, which can hide typos; add a validation
block to variable "sku" that restricts allowed values to the supported ACR SKUs
(e.g., "Basic", "Standard", "Premium") and provide a clear error_message
explaining the permitted values so typos are caught during plan/apply; update
the variable declaration for "sku" to include this validation and an explicit
error_message.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@terraform/modules/compute/azure/cleanup-function/main.tf`:
- Around line 115-123: The recurrence trigger resource
azurerm_logic_app_trigger_recurrence.cleanup uses timestamp() for start_time
which changes every plan; replace it with a stable value (either a hard-coded
RFC3339 string or a new input variable like var.cleanup_start_time) and update
any docs/variables accordingly, or keep the existing computed value but add a
lifecycle { ignore_changes = [start_time] } block on
azurerm_logic_app_trigger_recurrence.cleanup to prevent plan churn; modify the
resource block for start_time and/or add the lifecycle ignore_changes entry to
resolve the constant diffs.

---

Nitpick comments:
In `@terraform/modules/registry/azure/variables.tf`:
- Around line 16-20: The sku variable currently accepts any string, which can
hide typos; add a validation block to variable "sku" that restricts allowed
values to the supported ACR SKUs (e.g., "Basic", "Standard", "Premium") and
provide a clear error_message explaining the permitted values so typos are
caught during plan/apply; update the variable declaration for "sku" to include
this validation and an explicit error_message.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 77a69fb8-78ca-430d-9711-cefc2b2c7970

📥 Commits

Reviewing files that changed from the base of the PR and between ed6e236 and e085ad0.

📒 Files selected for processing (8)

• terraform/modules/compute/azure/cleanup-function/main.tf
• terraform/modules/compute/azure/cleanup-function/outputs.tf
• terraform/modules/compute/azure/cleanup-function/variables.tf
• terraform/modules/compute/azure/cleanup-function/versions.tf
• terraform/modules/registry/azure/main.tf
• terraform/modules/registry/azure/outputs.tf
• terraform/modules/registry/azure/variables.tf
• terraform/modules/registry/azure/versions.tf