chore(terraform): move cross-variable validations to lifecycle preconditions

[Shivam Sharma (ShivamSharma43)]

Pull Request

Description

Moves the 10 cross-variable validation blocks listed in #608 out of their variable blocks and into lifecycle.precondition blocks, and moves 4 more cross-variable checks that were added to 080-azureml and 100-cncf-cluster after the issue was filed. That is 14 preconditions in total. The original 10 keep their conditions and error messages.

With Terraform 1.15.0, terraform test fails before any run starts in components where a test references one of these variables:

Error: Reference to uninitialized variable

  on variables.tf line 58, in variable "fabric_capacity_admins":
  58:     condition     = !var.should_create_fabric_capacity || length(var.fabric_capacity_admins) > 0

The variable var.should_create_fabric_capacity was not processed by the most
recent operation, this likely means the previous operation either failed or
was incomplete due to targeting.

I reproduced this on main with Terraform 1.15.0 in 031-fabric and in 100-cncf-cluster (var.should_assign_roles).

Where the preconditions live. The issue suggests a terraform_data guard resource. I kept that in 031-fabric and 036-managed-redis. In 080-azureml, 100-cncf-cluster and ubuntu-k3s a new managed resource sits inside existing depends_on chains: its one-time create on the first plan after upgrading defers data source reads further down those chains (for example data.azapi_resource.arc_connected_cluster, which sets the Arc RBAC role assignment scope), which can replace existing role assignments. So in those three places the preconditions are on resources that already exist and add no planned change (see Implementation Details).

Related Issue

Fixes #608

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Blueprint modification or addition
• Component modification or addition
• Documentation update
• CI/CD pipeline change
• Other (please describe):

Implementation Details

Component	Precondition carrier	From #608	Additional
src/000-cloud/031-fabric	new terraform_data.validate_inputs	fabric_capacity_admins
src/000-cloud/036-managed-redis	new terraform_data.validate_inputs	private_endpoint_subnet
src/000-cloud/080-azureml	existing data.azurerm_client_config.current	ml_workload_identity, network_security_group, nat_gateway	compute_cluster_min_nodes <= compute_cluster_max_nodes
src/100-edge/100-cncf-cluster	existing data.azurerm_client_config.current	arc_onboarding_identity (both checks), key_vault, should_generate_cluster_server_token	should_use_script_from_secrets_for_deploy, should_deploy_over_ssh (two checks)
src/100-edge/100-cncf-cluster/modules/ubuntu-k3s	existing random_string.cluster_server_token	should_generate_cluster_server_token

• Why a data source. Preconditions on a data source are checked during plan, before the read, including when the read itself is deferred to apply. A data source never adds a planned change, so it cannot defer other reads. azurerm_client_config.current has no count and is already read in both components.
• ubuntu-k3s. The nested guard is gone. The rule now sits on random_string.cluster_server_token, which only exists when should_generate_cluster_server_token = true, so the condition there is just var.cluster_server_token == null. When the module is called from 100-cncf-cluster, a conflicting pair still reports the message twice (root and module).
• compute_cluster_min_nodes. The rule is split: >= 0 stays as a variable validation (it only references itself), and <= compute_cluster_max_nodes is a precondition with its own message.
• 080-azureml null handling. The module inputs are null-safe so a failed check does not also produce Attempt to get attribute from null value / Missing required argument errors: should_enable_nat_gateway, should_associate_network_security_group and the two ml_workload_identity flags are gated on the object being present, and nat_gateway_id = try(var.nat_gateway.id, null). module.network has depends_on = [data.azurerm_client_config.current] so the checks also run in targeted plans (module.workspace and module.inference_cluster_integration inherit it through their existing depends_on). Without it, -target='module.network[0]' skipped the checks and the presence gate quietly dropped the NAT/NSG association. module.network has no data sources, and the data source can only be deferred when the whole component is, so this does not add deferred reads.
• No explicit depends_on on the Fabric/Redis guards. module.fabric_capacity and module.managed_redis do not depend on the guard.
• Tests. 031-fabric/tests/base.tftest.hcl (create_capacity_without_admins) expects the failure on terraform_data.validate_inputs. In 100-cncf-cluster/tests/cluster.tftest.hcl, test_invalid_multiple_identities expects it on data.azurerm_client_config.current, and three new plan runs cover the additional checks: test_invalid_script_from_secrets_without_key_vault_upload, test_invalid_ssh_without_arc_machines and test_invalid_ssh_without_credentials.
• READMEs regenerated with terraform-docs v0.20.0 and markdown-table-formatter (as in npm run tf-docs). 031-fabric and 036-managed-redis gain the terraform_data.validate_inputs row; the 080-azureml, 100-cncf-cluster and ubuntu-k3s READMEs end up with no resource changes.

Testing Performed

• Terraform plan/apply
• Blueprint deployment test
• Unit tests
• Integration tests
• Bug fix includes regression test (see Test Policy)
• Manual validation
• Other: terraform test (plan-level, mocked providers), terraform validate, terraform fmt, TFLint

All results below are my local runs on head 7b710035, with Terraform 1.16.5 and 1.15.0 on Windows, and no ARM_* / AZURE_* variables set. Nothing was planned or applied against Azure.

• terraform init -backend=false + terraform validate in 031-fabric, 036-managed-redis, 080-azureml and 100-cncf-cluster: valid. terraform fmt -check -recursive: no changes.
• TFLint 0.64.0 (azurerm ruleset 0.30.0, repo .tflint.hcl, --disable-rule=terraform_required_providers --recursive) in 080-azureml and 100-cncf-cluster: no findings.
• 031-fabric, terraform test: 5 passed, 0 failed on both versions. On main with 1.15.0 it fails with the error above.
• 100-cncf-cluster: the committed cluster.tftest.hcl uses the real azurerm provider and needs an Azure login, which I don't have. In a local copy with mock_provider blocks for azurerm, azuread and azapi (not committed), all 13 committed runs pass on both versions. Extra temporary runs in that copy:
  • generated token with no cluster_server_token: plans.
  • should_upload_to_key_vault = true with key_vault = null: fails on data.azurerm_client_config.current only.
  • cluster_server_token set together with should_generate_cluster_server_token = true: fails with the original message from the root data source and from module.ubuntu_k3s.random_string.cluster_server_token.
  • should_assign_roles = true with no identity: fails with the original message, plus Missing required argument from module.role_assignments (see Additional Notes).
• 080-azureml has no committed tests. A temporary mocked test file (not committed) passes 8/8 on both versions: valid full plan and valid module.network targeted plan with a NAT gateway; nat_gateway missing in a full plan and in a module.network targeted plan; network_security_group missing in a module.network targeted plan; ml_workload_identity missing in a full plan and in a module.workspace targeted plan; min nodes greater than max nodes. Each failing case reports only the precondition. The same targeted NAT case against the previous head (37f35b5b) planned without any error.
• Upgrade path. I could not run an upgrade plan against a real pre-PR state. I reproduced the dependency mechanism in a small standalone config with only built-in providers (Terraform 1.16.5): after an apply, adding a terraform_data guard inside a module that another module depends_on, with a data source that depends_on the second module, made the next plan read the data source during apply and replace a resource keyed on its result. Putting the same precondition on an existing data source instead gave No changes with valid inputs, and still failed at plan with invalid inputs, including when that data source's own read was deferred.

Validation Steps

1. cd src/000-cloud/031-fabric/terraform && terraform init -backend=false && terraform test with Terraform 1.15.x: passes on this branch, fails on main with Reference to uninitialized variable.
2. terraform init -backend=false && terraform validate in each of the four component directories.
3. Optional: plan 080-azureml with should_enable_nat_gateway = true and no nat_gateway, with and without -target='module.network[0]'. Both should fail with nat_gateway must be provided when should_enable_nat_gateway is true.

Checklist

• I have updated the documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed
• I have run terraform fmt on all Terraform code
• I have run terraform validate on all Terraform code
• I have run az bicep format on all Bicep code
• I have run az bicep build to validate all Bicep code
• I have checked for any sensitive data/tokens that should not be committed
• Lint checks pass (run applicable linters for changed file types)

"All new and existing tests passed" is unchecked because the committed 100-cncf-cluster tests have only been run in the mocked copy described above, not with a real Azure login.

Security Review

• No credentials, secrets, or tokens are hardcoded or logged
• RBAC and identity changes follow least-privilege principles
• No new network exposure or public endpoints introduced without justification
• Dependency additions or updates have been reviewed for known vulnerabilities
• Container image changes use pinned digests or SHA references

Additional Notes

• Follow-on errors in 100-cncf-cluster. A variable validation stopped evaluation before anything used the bad value; a precondition does not. With should_assign_roles = true and no identity, the precondition error is followed by Missing required argument errors from module.role_assignments. The plan still fails with the original message. I left this as is to keep the change focused. Making module.role_assignments depend on the data source would reduce it to a single error, and I'm happy to add that if you prefer.
• Blueprints. 080-azureml and 100-cncf-cluster no longer add any managed resource, so blueprint modules that depends_on them (for example edge_azureml with depends_on = [module.cloud_azureml] in full-multi-node-cluster and robotics) get no extra deferred reads from this change. No blueprint depends_on the cloud_fabric or cloud_managed_redis module calls.
• Other cross-variable validations on main not changed here: 010-security-identity and 030-data (network_security_perimeter_id), 055-vpn-gateway (existing_certificate_name), 085-ai-foundry (private_endpoint_subnet_id), 110-iot-ops (byo_issuer_trust_settings, aio_ca), and in blueprints compute_cluster_min_nodes (azureml, robotics) and cluster_server_ip (full-multi-node-cluster). As far as I can tell, 1.15.0 only fails when a test references such a variable, and no test references these today. I can move them in a follow-up.
• I did not change the ADO terraformVersion: latest pin mentioned at the end of the issue.

Overlap with #861: that PR adds a 036-managed-redis test (create_private_endpoint_without_subnet) that expects var.private_endpoint_subnet. Whichever of the two merges second needs that expect_failures changed to terraform_data.validate_inputs; I'll update whichever is still open.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Chris Montazer (rezatnoMsirhC)]

Is this depends_on needed? A failing precondition on terraform_data.validate_inputs stops the plan regardless of graph ordering (custom conditions), and the 080-azureml, 100-cncf-cluster, and ubuntu-k3s guards don't add one. On a module, depends_on also defers any data source reads inside it, which HashiCorp recommends using only as a last resort. Could we drop it here and in 036-managed-redis/terraform/main.tf (L35) for consistency?

Suggested change

depends_on = [terraform_data.validate_inputs]

[Shivam Sharma (ShivamSharma43)]

Agreed, removed it here and in 036-managed-redis in 37f35b5. The 031-fabric tests still pass, including create_capacity_without_admins.

[Shivam Sharma (ShivamSharma43)]

Removed from both module calls in 37f35b5. One change in b0edf27: Redis now presence-gates should_enable_private_endpoint on private_endpoint_subnet (katriendg's follow-up). Gating alone would make a plan targeting module.managed_redis skip the endpoint without an error, so I put depends_on = [terraform_data.validate_inputs] back on that one call. The Redis module has no data sources, so nothing gets deferred. Fabric still has no depends_on. I've left this thread open so you can check that exception. Happy to drop it if you'd rather accept the targeted-plan gap.

[github-actions]

📚 Documentation Health Report

Generated on: 2026-10-07 21:36:02 UTC

📈 Documentation Statistics

Category	File Count
Main Documentation	223
Infrastructure Components	232
Blueprints	40
GitHub Resources	26
AI Assistant Guides (Copilot)	17
Total	538

🏗️ Three-Tree Architecture Status

• ✅ Bicep Documentation Tree: Auto-generated navigation
• ✅ Terraform Documentation Tree: Auto-generated navigation
• ✅ README Documentation Tree: Manual README organization

🔍 Quality Metrics

• Frontmatter Validation:
  success
• Link Validation: success

---

This report is automatically generated by the Documentation Automation workflow.

[Katrien De Graeve (katriendg)]

Please address the first-upgrade dependency path introduced by the nested validation guard, which can defer the Arc cluster lookup and force RBAC replacement. Please also refresh the PR description and attach current-head Terraform 1.15+ validation and regression evidence.

Reconcile the PR description with the follow-up commit

The PR body still describes the first revision. Fabric and Redis module calls no longer depend on the guard. AzureML min/max and the three cluster script/SSH checks listed as deferred have now moved, bringing the guard total to fourteen preconditions. The min-node rule is split with narrower messages; this does not violate preservation of the original issue's ten messages. AzureML now uses try() and object-presence gates for NAT, NSG, workspace identity and inference identity, so the body should not describe their earlier double-error behavior as current without qualifying it. The test summary omits three added plan runs and the added-tests checklist remains unchecked. This misstates the reviewed deliverable, independently of the four already-addressed source comments.

Update Description, Implementation Details, Additional Notes and the test inventory for commit 37f35b5: ten original migrations plus four additional checks; no explicit guard depends_on on Fabric/Redis module calls; AzureML object-presence gates and try(); split min-node messages; three new cluster plan runs. Retain only genuinely deferred components/blueprints in follow-up scope. Keep runtime success claims explicitly author-reported and version/head/fixture-specific. Reconcile the added-tests and regression-test checklist entries with committed tests without marking execution or human-review boxes complete prematurely.

[Shivam Sharma (ShivamSharma43)]

Katrien De Graeve (@katriendg) thanks for the thorough review. I've updated the description for head 7b71003. It now covers the 10 original and 4 additional preconditions and where each one lives, the Fabric/Redis guards having no depends_on, the AzureML try() and presence gates, the split min-node messages, and the three new cluster plan runs. The test evidence is my own local runs on that head with Terraform 1.16.5 and 1.15.0. Nothing ran against Azure, and "All new and existing tests passed" stays unchecked for that reason.

[Katrien De Graeve (katriendg)]

Reviewed 7b71003 locally on Terraform 1.15.0 and 1.16.5: 37 test runs passed on each, including the committed real-provider CNCF suite and additional isolated AzureML full/targeted negative cases. The original Fabric initialization failure reproduces on main. Focused validation, formatting and TFLint passed.

The earlier guard-induced replacement and targeted-plan concerns are addressed. An Azure-backed comparison using the existing cluster state and equivalent inputs has identical actions and replacement paths on PR and main. Both still propose 35 replacements from existing drift, so neither plan should be applied without separate investigation. Existing-state AzureML/Redis/Fabric upgrades were not covered.

One non-blocking cleanup: Redis with an enabled private endpoint and missing subnet reports the intended guard error plus a secondary missing subnet_id error. Consider the same presence-gating approach used for AzureML. Please retain the AzureML negative regression cases in the suite now available on main, and reconcile/close the six unresolved threads.

[github-actions]

📚 Documentation Health Report

Generated on: 2026-10-14 12:13:07 UTC

📈 Documentation Statistics

Category	File Count
Main Documentation	224
Infrastructure Components	238
Blueprints	40
GitHub Resources	26
AI Assistant Guides (Copilot)	17
Total	545

🏗️ Three-Tree Architecture Status

• ✅ Bicep Documentation Tree: Auto-generated navigation
• ✅ Terraform Documentation Tree: Auto-generated navigation
• ✅ README Documentation Tree: Manual README organization

🔍 Quality Metrics

• Frontmatter Validation:
  success
• Link Validation: success

---

This report is automatically generated by the Documentation Automation workflow.

[Shivam Sharma (ShivamSharma43)]

Katrien De Graeve (@katriendg) thanks for the review and the approval. Follow-up commit b0edf27:

• Redis: should_enable_private_endpoint is passed to the module only when private_endpoint_subnet is set. Enabling the private endpoint without a subnet now reports only the guard error. The module call also depends on the guard, so a plan targeting module.managed_redis still runs the check. That reverses the earlier depends_on removal for this one call, and I've left that thread open for Chris Montazer (@rezatnoMsirhC).
• AzureML: the six negative cases are now in 080-azureml/terraform/tests/base.tftest.hcl: NAT (full and targeted), NSG (targeted), workload identity (full and targeted), and min > max.

Checks on Terraform 1.16.5 and 1.15.0: the 080-azureml suite passed 14/14. fmt, validate and tflint are clean for both components. A scratch mocked Redis test (not committed) reproduced the extra subnet_id error on the previous head and passed on this one for full, targeted and valid-subnet plans. I replied to the review threads with the commits that addressed them and resolved five.