Skip to content

test(terraform): add plan-level tests for 080-azureml and 140-azureml - #875

Merged
Katrien De Graeve (katriendg) merged 2 commits into
microsoft:mainfrom
ShivamSharma43:test/152-azureml
Oct 13, 2026
Merged

Katrien De Graeve (katriendg) merged 2 commits into
microsoft:mainfrom
ShivamSharma43:test/152-azureml

Conversation

@ShivamSharma43

Copy link
Copy Markdown
Contributor

Pull Request

Description

Adds plan-level Terraform tests for the last two untested components listed in #152: 080-azureml and 140-azureml. Each component gets a tests/base.tftest.hcl and a tests/setup module, following the pattern from #821 and #874. All runs use command = plan with mock_provider, so no Azure credentials are needed.

Related Issue

Relates to #152

With #821, #861 and #874, this covers the remaining components in that issue. It doesn't touch any component code.

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): tests only, no changes to component code

Implementation Details

080-azureml (mocks azapi and azurerm; azurerm_client_config is overridden):

  • create_default_configuration: the workspace name (mlw-...), location and resource group, the compute subnet name (subnet-<prefix>-azureml-...) and the compute cluster name (cluster-...). No registry, private endpoint, DNS zones, AKS integration or workload identity by default.
  • create_with_custom_names_and_existing_subnet: provided workspace and cluster names are used. With should_create_compute_cluster_snet = false and compute_cluster_subnet, no subnet is created but the cluster still is.
  • create_without_compute_cluster: neither the cluster nor its subnet is created, and the cluster outputs are null.
  • create_with_private_endpoint: the workspace private endpoint name (pe-mlw-...) and the two private DNS zones (privatelink.api.azureml.ms, privatelink.notebooks.azure.net).
  • integrate_aks_cluster: the extension name, and the compute target name with hyphens removed and truncated to 16 characters (environment = "staging-west").
  • reject_invalid_inputs / reject_invalid_resource_prefix: the single-variable validations (compute_cluster_max_nodes, compute_cluster_vm_priority, inference_router_service_type, aks_cluster_purpose, resource_prefix).

140-azureml (mocks azapi, azurerm and tls):

  • create_default_configuration: extension name (azureml-...) and compute target name (arck-...), the target's workspace, location, Kubernetes compute type and connected cluster ID, the default description, local auth disabled, azureml namespace, the component's default instance type (8Gi memory limit), Stable release train, and a system-assigned identity.
  • create_with_custom_compute_settings: provided names, description, local auth, namespace, default instance type, release train and virtual cluster name are passed through. Provided instance types replace the default, and cluster_integration_workspace_identity_id switches the target to a user-assigned identity.
  • configure_extension_with_tls_tolerations_and_workload_identity: runs the inference-cluster-integration module directly, because the extension settings and federated credentials are not exposed as component outputs. Checks the string-typed feature flags, relayserver.enabled = "true", allowInsecureConnections = "false" with the protected TLS settings when CNAME, certificate and key are all set, the indexed toleration[n] / workLoadToleration[n] keys (unset fields left out), and one federated identity credential per subject, with the name derived from the subject, the cluster OIDC issuer and the workload identity.
  • reject_invalid_inference_router_service_type: the variable validation.

Some outputs (kubernetes_compute, azureml_extension) wrap whole resources in try(), so a single unknown computed attribute makes them unknown at plan. The tests override only the computed attributes they depend on (the extension id and identity principal, and the compute id) with override_during = plan.

The 080 tests don't cover the cross-variable validations (nat_gateway, network_security_group, ml_workload_identity, min/max nodes) because #862 moves them. I also ran these 080 tests against the #862 head, and all 8 runs pass there too.

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, mock providers)

Run with no ARM_* or AZURE_* variables set:

  • terraform init -backend=false and terraform test in each component, with Terraform 1.16.5 and 1.15.0: 080-azureml 8 passed, 0 failed; 140-azureml 5 passed, 0 failed.
  • terraform validate in each component: valid.
  • terraform fmt -check -recursive on both tests/ directories: no changes.
  • tflint --disable-rule=terraform_required_providers --recursive with the repo .tflint.hcl: no findings.

I also checked that the tests fail when the component is wrong. I broke a copy of each component, one change at a time, and every one of these 10 changes made at least one run fail:

  • the subnet label
  • the cluster name prefix
  • a DNS zone name
  • the 16-character truncation
  • creating the subnet without a cluster
  • relayserver.enabled
  • the TLS check
  • the default instance type memory
  • the workLoadToleration key
  • the user-assigned identity block

Additional Notes

While writing the registry case for 080 I found that should_deploy_registry = true currently fails at plan with azapi 2.13.0:

embedded schema validation failed: the argument "body" is invalid:
`properties.description` is not expected here.

The root module always passes a description (coalesce(var.registry_description, ...)), and the Microsoft.MachineLearningServices/registries@2025-07-01-preview schema has no properties.description, so the registry can't be planned with schema validation on. I left the registry out of these tests. I'm happy to open a separate issue or fix for it if you'd like.

Validation Steps

  1. cd src/000-cloud/080-azureml/terraform && terraform init -backend=false && terraform test
  2. cd src/100-edge/140-azureml/terraform && terraform init -backend=false && terraform test

No Azure login is needed for either.

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

🤖 Generated with Claude Code

Part of microsoft#152.

- 080-azureml: default workspace, compute subnet and compute cluster,
  custom names with an existing subnet, no compute cluster, workspace
  private endpoint and DNS zones, AKS integration naming, and the
  single-variable validations.
- 140-azureml: default Arc compute target, custom compute settings and
  user-assigned identity, extension settings with TLS, tolerations and
  workload identity federation (integration module), and the inference
  router service type validation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Shivam Sharma <192242476+ShivamSharma43@users.noreply.github.com>
@ShivamSharma43
Shivam Sharma (ShivamSharma43) requested review from a team and a balanced review from Copilot October 12, 2026 10:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution, this is looking good.

@katriendg
Katrien De Graeve (katriendg) merged commit f479861 into microsoft:main Oct 13, 2026
43 checks passed
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.

3 participants