[Yingjie Chen (Y1ngJ1eChen)]

Summary

Options.Parallelism is documented as "the number of parallel requests to make to Azure APIs when getting policy definitions and policy set definitions", but it was only validated in Init() and never used. Built-in policy definitions, policy set definitions and the definitions they reference were fetched one request at a time, and Hierarchy.FromArchitecture fetched one management group at a time. Building the alz architecture without a cache makes ~700 ARM requests, so alz_architecture reads took minutes.

This PR fetches them concurrently, bounded by Options.Parallelism (default 10, which terraform-provider-alz uses), without changing which requests are made or what is loaded.

Fixes Azure/Azure-Landing-Zones#4310

Not a regression: up to v0.18.5 an errgroup existed, but each goroutine held az.mu across the HTTP call, so fetching was still serial. #135 removed the errgroup.

Changes

alzlib

• getBuiltInPolicies, getBuiltInPolicySets and ensureReferencedPolicyDefinitions run with errgroup + SetLimit(Parallelism).
• Requests are grouped by definition name (case-insensitive): requests for the same name run serially in their original order; different names run concurrently. A definition is therefore fetched and added at most once, and the final state does not depend on scheduling (no duplicate-add errors with AllowOverwrite=false).
• Azure calls never hold az.mu; the lock only guards map access through the existing accessors.
• New fetchMu serialises concurrent GetDefinitionsFromAzure callers sharing one AlzLib (e.g. several alz_architecture data sources in the provider). Previously they raced to fetch and add the same definitions.
• Azure SDK clients are created once per call instead of the lazy **Client initialisation, which would race under concurrency.
• The version-mismatch error path read az.policyDefinitions[name].Versions() without the lock and panicked on a missing entry; it now uses a locked accessor.
• Policy set definition references are validated before any Azure call (error texts unchanged).
• Parallelism < 1 is treated as 1 internally; Init() still rejects 0.
• Options.Parallelism doc: same-name requests are serial; 1 restores serial fetching.
• golang.org/x/sync v0.18.0 becomes a direct dependency. It is already selected by MVS, and the go directive is unchanged.

deployment

• FromArchitecture prefetches the definitions of the whole architecture in one GetDefinitionsFromAzure call, in a deterministic order, so requests are spread across the configured parallelism instead of one management group at a time. Assignments that cannot be resolved are skipped by the prefetch and still reported by addManagementGroup with the existing error. The per-management-group call remains as a no-op fallback.

tests

• internal/testutil/fakearm: an in-process fake of the ARM built-in policy API (latency, failure injection, request and peak-concurrency counters).
• Tests for the parallelism bound, shared/duplicate definitions fetched once, concurrent callers, first-error cancellation, caller cancellation, the missing-version error path, grouping, the architecture prefetch and error wrapping; plus BenchmarkGetDefinitionsFromAzureSimulatedLatency.

Behaviour notes

• FromArchitecture fetch errors are now wrapped as Hierarchy.FromArchitecture: getting policy definitions from Azure for architecture ... (inner error unchanged). terraform-provider-alz does not parse these strings.
• When several definitions fail, the first error is returned and the remaining work is cancelled, so which error is reported can vary.
• A fetch failure in FromArchitecture now happens before any management group is added. Previously the groups processed before the failing one remained in the hierarchy; callers discard the hierarchy on error.
• The order in which FromArchitecture fetches is now deterministic. Previously child management groups were visited in map order, so the number of requests (and, for a definition needed both as latest and as a constraint, which version was loaded first) could vary between runs.
• Raising Parallelism above 10 is not recommended: ARM throttling and per-request latency increase.

Validation

Check	Result
go test ./... (incl. read-only Azure integration tests), go vet	pass
golangci-lint v2.11.4 with Go 1.25.2 (--new-from-rev)	0 issues
New tests, -count=20	pass, no flakes
go test -race -count=3 on ., ./deployment, ./cache (Windows, MinGW-w64)	no races; the detector was confirmed with a deliberately racy test. Linux make testrace is left to CI
Mutation: remove fetchMu / remove name grouping	each caught by TestGetDefinitionsFromAzureConcurrentCallers / ...SharedDefinitionsFetchedOnce
Separate verification suite against main (cbf351d)	all 11 bug-detecting tests fail on main (incl. a panic on the missing-version path); all 18 guards pass on both
terraform-provider-alz v0.22.0 built against this branch	builds, unit tests pass, no code change needed

Live Azure, read-only (uncached, alz architecture, main vs. this branch):

	main	this PR
Requests / peak in flight	696 / 1	696 / 10 (never 2 for one definition)
Management groups / PDs / PSDs loaded	11 / 841 / 63	11 / 841 / 63
BenchmarkFromArchitectureWithClient, benchstat	157.1 s	31.7 s (-79.8 %, p=0.008, n=5)
Same, on a degraded network	330.7 s	106.2 s (-67.9 %, p=0.002, n=6)
alz_architecture ReadDataSource in terraform plan (provider v0.22.0, platform/alz 2026.10.0)	261.7 s	57.7 s (median, outputs identical)
Two alz_architecture data sources in one plan	failed: policy definition db048e65-…@1.*.* … does not exist in the library (concurrent-caller race)	pass, both equal to the single-data-source result
From a cache (--from-cache)	0 requests	0 requests, identical output

alzlibtool generate architecture output is byte-identical to main: 334/334 files for integrationtest/testdata/alzlib-2025-09-0 and 320/320 for platform/alz@2026.10.0, each with and without --for-alz-bicep.

Follow-ups (not in this PR)

• terraform-provider-alz: bump alzlib so users get the fix; consider an optional parallelism provider attribute (it is hard-coded to 10) for throttled or constrained networks.
• ~700 per-definition version-list requests remain; reducing the request count (bulk prefetch) would be a separate change.

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

❌ Patch coverage is 47.38041% with 231 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.46%. Comparing base (cbf351d) to head (8c42f3f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/testutil/fakearm/fakearm.go	0.00%	188 Missing ⚠️
alzlib.go	77.57%	19 Missing and 5 partials ⚠️
builtin_fetch.go	83.15%	12 Missing and 4 partials ⚠️
deployment/hierarchy.go	93.87%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   50.75%   51.46%   +0.70%     
==========================================
  Files          55       57       +2     
  Lines        5004     5320     +316     
==========================================
+ Hits         2540     2738     +198     
- Misses       2171     2293     +122     
+ Partials      293      289       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.