Repository navigation
fix: apply later-library policy (set) definitions when library overwrite is enabled - #252
Open
Yingjie Chen (Y1ngJ1eChen) wants to merge 1 commit into
Open
Yingjie Chen (Y1ngJ1eChen) wants to merge 1 commit into
Yingjie Chen (Y1ngJ1eChen) wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #252 +/- ##
==========================================
+ Coverage 49.54% 49.61% +0.07%
==========================================
Files 54 54
Lines 4967 4962 -5
==========================================
+ Hits 2461 2462 +1
+ Misses 2210 2207 -3
+ Partials 296 293 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Qi Zhang (Zhangqi910)
requested review from
Akshay Rohilla (akshayrohilla) and
Shiva Sai Yeddlapuri (yeddlapurishivasai)
October 9, 2026 07:35
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a regression where AllowOverwrite (
library_overwrite_enabledinterraform-provider-alz,--library-overwrite-enabledinalzlibtool) was silently ignored for policy definitions and policy set definitions. With this change, a later library that redefines one of these assets replaces the earlier version again, and versions added by a later library are retained.Fixes Azure/Azure-Landing-Zones#4295. Follows up on #249, which documented the previous behaviour as a limitation pending a decision on the intended contract. The ALZ documentation, the engine's behaviour before #222, and the
Upsertcontract all show that the later library is meant to win.Problem
Redefining a library asset with the same name in a custom library is the documented way to replace it:
Since policy versioning was introduced in #222 (alzlib v0.29.0, provider v0.20.0), the custom version of a policy (set) definition was discarded and the library version was deployed. No error or warning was raised. Deployments that relied on such overrides had them reverted on the next apply after a provider upgrade:
Two further symptoms had the same cause:
AddPolicyDefinitionsswallowed the real error and returned0 errors occurred:.Root cause
addPolicyAndRoleAssetscalledUpsertwith the receiver and argument swapped.Upsertmerges its argument into the receiver, so the stored collection was merged into the incoming one, and the incoming collection was then discarded:Separately,
AddPolicyDefinitionscalled pdvc.Upsert(pdvc, ...) on failure. That upserts the collection into itself and loses the original error.AddPolicySetDefinitionsalready handled this correctly.How it is resolved
The incoming collection is now merged into the stored one, so the library processed last wins version by version:
The same change is applied to policy set definitions.
AddPolicyDefinitionsnow appends the actualAdderror. No changes were needed in assets;Upsertalready had the intended semantics.Merging per version, rather than replacing the whole collection, keeps earlier versions that the later library does not redefine. Assignments pinned to an older definitionVersion therefore still resolve.
Changes
Upsertreceiver and argument for policy (set) definitions; return the real error fromAddPolicyDefinitionscmd/alzlibtool/command/{check,generate,document,cache}/*.goTest_libraryCmd_policyDefinitionOverrideNotApplied→..._OverrideApplied: now expects exit code 0outputAssetContractnow expects the member's policy definition in the generated artifactBehaviour after the fix
AllowOverwrite = falseAllowOverwrite = trueRedefined assets are still replaced in full, not merged field by field. Behaviour for policy assignments, role definitions, archetypes, architectures and policy default values is unchanged. Duplicate archetype override names remain an error.
Out of scope
terraform-provider-alz: an alzlib bump and a clarifiedlibrary_overwrite_enableddescription will follow in a separate PR after release.Testing
All tests were run locally.
In-repo:
go test ./...(includingintegrationtest) andgolangci-lintpass. The new unit tests fail onmainand pass with this fix.Local verification against
main(d5b42ab) and this branch, covering:Enforce-Guardrails-Networkin realplatform/alz2025.02.0 and 2026.04.2.mainkeepsDeny; this PR appliesAudit.library_references, a three-library chain, and reversed order. The last library processed wins.main; with redefinitions, only the redefined assets differ. Conflicts without the flag are still rejected.Backwards compatibility and upgrade impact
AllowOverwrite = false): conflicts are rejected exactly as before. The only change is that a new version added by a later library is now retained instead of silently dropped.main.Suggested release note