Skip to content

fix: handle empty built-in policy version listings instead of panicking - #254

Merged
Qi Zhang (Zhangqi910) merged 3 commits into
mainfrom
user/yingjie/fix-issue-4327-alzlib-versions-nil-pointer-panic
Nov 2, 2026
Merged

Qi Zhang (Zhangqi910) merged 3 commits into
mainfrom
user/yingjie/fix-issue-4327-alzlib-versions-nil-pointer-panic

Conversation

@Y1ngJ1eChen

Copy link
Copy Markdown
Contributor

Summary

Reading an alz_architecture crashes terraform-provider-alz with a nil pointer dereference when Azure's built-in policy definition version listing returns no version for a definition that a built-in policy set references with a definitionVersion. Since around 27 October 2026, Azure returns exactly that in several regions for every Resource Provider mode definition, so planning the default ALZ architecture fails for everyone whose requests are served there.

This PR fixes it in two commits, best reviewed one at a time:

  1. Stop the crash: VersionedPolicyCollection.Versions() is nil-safe, so the existing descriptive error is returned instead of a panic. This commit stands on its own if the second one needs more review.
  2. Unblock the read: when the version listing has no version that satisfies the constraint, alzlib gets the definition itself, selects the version from its properties.versions with the existing GetVersion, and gets that version. A healthy listing costs no extra request.

Refs Azure/Azure-Landing-Zones#4327. Not "Fixes": the issue stays open until a provider release with this change is confirmed by the reporters.

Problem

In UK West, France Central, Spain Central, West Central US, Jio India Central and the two EUAP regions, GET /providers/Microsoft.Authorization/policyDefinitions/{name}/versions answers 200 OK with "value": [] for every Resource Provider mode built-in definition (Microsoft.*.Data), whatever the API version. Everything else alzlib needs still works there: the definition GET (including properties.versions), the version GET and the policy set definition version listing. ARM serves requests from a nearby region, so only some users are affected.

$ az rest --resource https://management.azure.com/ --query "value[].properties.version" \
    --url "https://ukwest.management.azure.com/providers/Microsoft.Authorization/policyDefinitions/c26596ff-4d70-4e6a-9a30-c2506bd2f80c/versions?api-version=2023-04-01"
[]
# The same request to uksouth.management.azure.com returns ["6.2.0", "6.1.0"].

Reading the default alz architecture (platform/alz 2026.10.0) needs 73 such definitions, 68 of them through Microsoft cloud security benchmark v2. In an affected region, v0.32.2 fails with:

panic: runtime error: invalid memory address or nil pointer dereference

goroutine 1 [running]:
github.com/Azure/alzlib/assets.(*VersionedPolicyCollection[...]).Versions(...)
    assets/genericVersionCollection.go:45
github.com/Azure/alzlib.(*AlzLib).fetchReferencedPolicyDefinitionVersions(...)
    alzlib.go:1427
github.com/Azure/alzlib.(*AlzLib).ensureReferencedPolicyDefinition(...)
    alzlib.go:1371
github.com/Azure/alzlib.(*AlzLib).ensureReferencedPolicyDefinitions(...)
    alzlib.go:1282
github.com/Azure/alzlib.(*AlzLib).getBuiltInPolicySets(...)
    alzlib.go:1063
github.com/Azure/alzlib.(*AlzLib).GetDefinitionsFromAzure(...)
    alzlib.go:808
...
github.com/Azure/alzlib/deployment.(*Hierarchy).FromArchitecture(...)
    deployment/hierarchy.go:122

Root cause

Since #222 (v0.29.0), alzlib resolves definitionVersion constraints itself and lists all versions of every referenced built-in definition. When nothing is listed under the requested name, fetchReferencedPolicyDefinitionVersions builds its error message with az.policyDefinitions[name].Versions(). The definition was never stored, so the map returns a nil collection and Versions() dereferences it. The error turns into a panic, and the provider plugin exits ("Plugin did not respond"). Affected: alzlib v0.29.0 to v0.32.2, terraform-provider-alz v0.20.0 to v0.22.x.

Without the panic the read still cannot succeed: in those regions the versions are never listed, and retrying the listing returns the same empty list.

How it is resolved

Commit 1: make VersionedPolicyCollection.Versions nil-safe

Versions() returns no versions for a nil receiver, so the read now fails with:

Alzlib.getBuiltInPolicySets: finding specific version constraint `1.*.*` of built-in policy definition for `<definition>`, referenced in policy set `<set>`. Versions available: []

Fixing it in assets rather than at the call site protects every caller, and keeps this PR free of conflicts with #253, which rewrites that call site.

Commit 2: fall back to the definition when no listed version matches

When a version listing yields no version that satisfies the constraint, because it is empty or because none matches, alzlib:

  1. gets the built-in definition (DefinitionsClient.GetBuiltIn or SetDefinitionsClient.GetBuiltIn) and takes the versions it lists in properties.versions and properties.version;
  2. selects one with the existing GetVersion, on a collection of placeholders that carry only the version. The selection, including -preview handling, is therefore the one a healthy listing gives, and it follows future changes to GetVersion such as Resolve policy definition versions to the version the assignment actually runs #248;
  3. gets the selected version (DefinitionVersionsClient.GetBuiltIn or SetDefinitionVersionsClient.GetBuiltIn), validates it, checks that Azure returned the requested name and version, and stores it under the requested name.

This covers every place that lists versions: definitions referenced by built-in policy sets (fetchReferencedPolicyDefinitionVersions), definitions referenced directly (addVersionedPolicyDefinition), and policy set definitions (addVersionedPolicySetDefinition). Each fallback makes two requests. If it fails too, the error reports what the listing returned and why the fallback failed. A cancelled context stops it before any request. The fallback gets only the selected version, so a definition referenced with two different constraints can be listed and resolved twice; this happens once in the default alz architecture.

Also in this commit:

  • Listed names: a listed version whose definition name differs from the requested one only in case is stored under the requested name, because definitions are stored and looked up by name case-sensitively. Versions of other definitions are ignored and named in the error.
  • cache.NewCacheFromAzure (alzlibtool cache create without --library): a version that Azure lists twice with different content no longer aborts cache creation. The version is got on its own and stored, with a warning. Only if that fails is the version listed first kept, also with a warning. Keeping either copy blindly could cache stale content, for example different roleDefinitionIds, and silently change policy role assignments.
  • Error prefix: errors for directly referenced definitions now name Alzlib.getBuiltInPolicies instead of Alzlib.getBuiltInPolicySets.

No retry is added: in the affected regions the listing is consistently empty, so retrying does not help. The fallback lives in a new file with small hooks in alzlib.go, to keep the merge surface with #253 small.

Changes

Commit File Change
1 assets/genericVersionCollection.go Versions() is nil-safe
1 assets/genericVersionCollection_nil_test.go Nil receivers
1, 2 alzlib_missing_referenced_version_test.go Regression test for the panic through the public API. In commit 2 the definition GET returns 404, so the error path stays covered; the case-mismatch case now succeeds and moves to the fallback tests
2 builtin_version_fallback.go New: the fallback, what the listing returned, and the error
2 alzlib.go Hooks in the three functions above; collectPolicy(Set)DefinitionVersions also return what was listed; error prefix
2 builtin_version_fallback_test.go New: 14 tests with the SDK fakes, which also count requests
2 alzlib_policy_fetch_test.go Call sites of the changed unexported signatures
2 cache/azure.go Duplicate resolution in NewCacheFromAzure
2 cache/azure_duplicates_test.go New: 4 tests
2 docs/cache.md Duplicate handling of cache create

Behaviour after the fix

Scenario v0.32.2 Commit 1 only This PR
Listing empty for a definition referenced by a built-in policy set (MCSB v2 in an affected region) Panic Readable error; the read still fails Succeeds
Listing empty for a policy set definition, or for a definition referenced directly or by a library policy set Error Unchanged Succeeds
Versions listed, but none satisfies the constraint Error Unchanged Succeeds if the definition lists a satisfying version
Versions listed under the definition name in another case (built-in policy set member) Panic Readable error Succeeds, no extra request
Neither the listing nor the definition provides the version Panic or error Readable error Error with what the listing and the definition returned
cache create in full mode, a version listed twice with different content Fails Unchanged Succeeds, with a warning
Healthy listing OK Unchanged Unchanged: same requests, byte-identical output

Testing

All tests were run locally.

In-repo: go build ./..., go vet ./... and go test ./... pass, with the 7 tests that need Azure credentials skipped. golangci-lint with GOTOOLCHAIN=go1.25.2, as in CI, reports no new issues. The new tests also passed 20 runs in a row. They use the SDK fakes and cover: a healthy listing with no extra request; an empty listing; versions listed under another case; versions of another definition; the definition not found; no version satisfying the constraint; the policy set path with the 1.*.*-preview constraint MCSB v2 is assigned with; cancellation; and, for preview constraints, the same selection as a healthy listing. Commit 1's tests panic on main. An end-to-end suite outside the repo ran the scenarios above against main, commit 1 only and this PR, including terraform-provider-alz; every outcome matched the table.

Live, against Azure on 2026-10-30 (read-only). A test-only pipeline policy pinned every ARM request to a regional endpoint, https://{region}.management.azure.com, and x-ms-routing-request-id confirmed the region of every response.

  • Region scan at 13:12 UTC, 62 regions: the listing is empty in West Central US, UK West, France Central, Spain Central, Jio India Central, East US 2 EUAP and Central US EUAP. An Indexed control definition is listed in all of them.

  • Reading platform/alz 2026.10.0 alz as the provider does:

    Build UK South (healthy) UK West (affected)
    main (v0.32.2) OK, 1,026 requests Panic, the stack above
    Commit 1 only - Readable error
    This PR OK, 1,026 requests, output identical to main OK, 1,175 requests, output byte-identical to UK South (320 files, 240 policy role assignments)

    In UK West, 74 listings were empty. The fallback added 74 definition GETs and 74 version GETs, plus one listing: 1a5b4dca-0b6f-4cf5-907c-56316bc1bf3d is referenced with 8.*.* and, by a library initiative, with 9.*.*, so it was listed and resolved once for each. A cache exported after the read in UK West (ExportBuiltInCache) reproduced the same output without any request.

  • Golden comparison: alzlibtool generate architecture output of main and of this PR is byte-identical for platform/alz 2026.04.2 (319 files), 2026.08.0 (319 files) and 2026.10.0 (320 files).

  • terraform-provider-alz v0.22.0 built against this branch with replace: terraform plan of data.alz_architecture, pinned to UK West, succeeds with the same values as UK South (12 management groups, 240 policy role assignments). The released v0.22.0 fails with "Plugin did not respond" and the panic above; built with commit 1 only, it fails with a readable diagnostic. The data source read timeout was raised to 30m because every request from the test machine crossed to the UK and the fetch is serial (fix: honor Options.Parallelism when fetching built-in policy definitions #253).

Open PRs (heads of 2026-10-30): this branch merges without conflicts with #253 (8c42f3f), #248 (ce35388) and #245 (1495111), and with #253 and #248 together; build, vet and tests pass on each result. #253 and #245 conflict with each other in go.sum, with or without this PR.

Backwards compatibility and upgrade impact

  • No public API changes, no new dependencies.
  • Healthy regions: the same requests and byte-identical output.
  • Affected regions: reads that crashed now succeed, at two extra requests per affected definition, about 15% more for the default alz architecture.
  • Error messages change only on paths that failed before: they now include what the listing and the fallback returned, and directly referenced definitions are reported by getBuiltInPolicies.
  • cache create in full mode no longer fails on a conflicting duplicate. In an affected region it stores only the latest version of Resource Provider mode definitions, because their history listings are empty; a cache miss falls back to Azure. Limited mode (--library --architecture), which follows the provider's path, is complete.

Out of scope

Suggested release note

Bug fix: reading an architecture no longer crashes with a nil pointer dereference when the Azure built-in policy definition version listing returns no matching version. Since around 27 October 2026 the listing returns no versions for Resource Provider mode definitions in some Azure regions; the versions are now read from the policy (set) definition itself (Azure/Azure-Landing-Zones#4327).

Behaviour change: alzlibtool cache create no longer fails when Azure lists a version twice with different content: it gets that version on its own and logs a warning.

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.98712% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.13%. Comparing base (04ec026) to head (bd46ebb).

Files with missing lines Patch % Lines
builtin_version_fallback.go 88.15% 10 Missing and 8 partials ⚠️
cache/azure.go 93.33% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
+ Coverage   50.79%   55.13%   +4.34%     
==========================================
  Files          55       56       +1     
  Lines        4999     5213     +214     
==========================================
+ Hits         2539     2874     +335     
+ Misses       2169     2022     -147     
- Partials      291      317      +26     

☔ 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.

Comment thread builtin_version_fallback.go Outdated
return err
}

if err := az.AddPolicyDefinitions(pd); err != nil {

Choose a reason for hiding this comment

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

Fallback makes version selection depend on reference order

The fallback stores only the version selected for the current constraint. Subsequent references use PolicyDefinitionExists, so any cached matching version suppresses further resolution—even when a newer matching version exists.

Reproduced: a policy set references the same definition first with 1.0., then 1..*, with available versions 1.0.2 and 1.2.0:

Healthy listing: the broader constraint resolves to 1.2.0.
Empty listing with fallback: it incorrectly reuses 1.0.2.
This can produce different parameter or role-assignment behavior in affected regions. Track resolved constraints or otherwise ensure a partial fallback result does not suppress resolving a broader constraint.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Confirmed, with your example. For a member of a built-in policy set, the listing stores every listed version, but the fallback stored only the one selected for the first reference, so PolicyDefinitionExists treated a later, broader constraint as satisfied. Fixed in bd46ebb: the fallback now gets every version the definition lists (skipping versions already stored, and adding none unless all are got), so it stores exactly what the listing would have, whatever the reference order. Added TestVersionFallbackIndependentOfReferenceOrder for both orders; it fails without the fix. Offline, reading the default alz architecture with the regional fault injected now stores the same built-in definitions as a healthy read (exported caches identical), at 1,241 requests instead of 1,175 (healthy: 1,026).

A related order dependence is pre-existing and unchanged here. For directly referenced definitions and policy set definitions, the listing path itself stores only the selected version. So on main, in any region, a direct 1.0.* processed before a built-in set's 1.. resolves the latter to 1.0.2. If the versions differ, that can fail Hierarchy.AddManagementGroup with a parameter mismatch, or compute policy role assignments from the older version. It is latent for the ALZ library: its constraints are all N.. / N..-preview, and the two definitions referenced with both resolve to the same version. The fallback now behaves exactly like the listing there, so affected regions are no worse off. Fixing it changes healthy-path behaviour, so I'll open a separate issue rather than widen this hotfix.

Comment thread cache/azure.go
err = c.Add(got, true)
}

if err != nil {

Choose a reason for hiding this comment

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

Duplicate recovery swallows context cancellation

The recovery path handles every GET error by logging a warning and returning nil, including cancellation and deadline errors.

Reproduced: cancelling during duplicate recovery on the final historical-list item makes NewCacheFromAzure return a non-nil cache with no error, despite ctx.Err() being context.Canceled. Callers can therefore report success and persist a cache after cancellation.

Propagate cancellation/deadline errors before applying the best-effort “keep first” behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Confirmed: if the recovery GET is the last request, NewCacheFromAzure returned a cache with a nil error after cancellation. addListedVersion now returns the context error when the GET fails and ctx is done. Other GET failures still keep the version listed first, with a warning. Added TestNewCacheFromAzureReturnsCancellationDuringRecovery for exactly this case; it fails without the fix.

@Zhangqi910
Qi Zhang (Zhangqi910) added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 8e1b5c9 Nov 2, 2026
8 checks passed
@Zhangqi910
Qi Zhang (Zhangqi910) deleted the user/yingjie/fix-issue-4327-alzlib-versions-nil-pointer-panic branch November 2, 2026 11:28

This branch was successfully deployed

1 active deployment
test — bd46ebb2 Deployed Nov 2, 2026 by Y1ngJ1eChen via test (ubuntu-latest) #694
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.

4 participants