From 004c63cb91835860cb226a9239f6b0fbfd3aad85 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:09:14 +0000 Subject: [PATCH 01/12] Initial plan From e334118b8399dbc2c8d79033f853e784856c6592 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:27:36 +0000 Subject: [PATCH 02/12] refactor(cache): wire cache into GetDefinitionsFromAzure for lazy loading and GC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cache/cache.go: Add PolicyDefinitionVersionsByName and PolicySetDefinitionVersionsByName per-name lookup methods, avoiding full-map clone for individual lookups - alzlib.go: Extend BuiltInCache interface with new per-name lookup methods - alzlib.go: Add `cache BuiltInCache` field to AlzLib struct - alzlib.go: AddCache now stores only the cache reference (lazy, not eager) - alzlib.go: GetDefinitionsFromAzure defers az.cache = nil so GC can reclaim cache - alzlib.go: getBuiltInPolicies checks cache before Azure; lazy-init Azure client - alzlib.go: getBuiltInPolicySets checks cache before Azure; lazy-init Azure clients - alzlib.go: ensureReferencedPolicyDefinitions removes upfront client creation - alzlib.go: ensureReferencedPolicyDefinition checks AlzLib → cache → Azure in order - alzlib.go: Extract policyDefinitionFromCache/policySetDefinitionFromCache helpers - alzlib_test.go: Update mockBuiltInCache with new interface methods - alzlib_test.go: Replace eager-load tests with lazy-load tests - integrationtest/cache_test.go: Update TestCacheInitWithLibrary for lazy semantics - docs/cache.md: Document lazy loading and GC behaviour Co-authored-by: matt-FFFFFF <16320656+matt-FFFFFF@users.noreply.github.com> --- alzlib.go | 185 ++++++++++++++++++++++++++-------- alzlib_test.go | 79 +++++++++++++-- cache/cache.go | 12 +++ docs/cache.md | 8 +- integrationtest/cache_test.go | 10 +- 5 files changed, 239 insertions(+), 55 deletions(-) diff --git a/alzlib.go b/alzlib.go index 9532dfe..fc4ebe8 100644 --- a/alzlib.go +++ b/alzlib.go @@ -48,6 +48,7 @@ type AlzLib struct { defaultPolicyAssignmentValues DefaultPolicyAssignmentValues metadata []*Metadata + cache BuiltInCache clients *azureClients mu sync.RWMutex // mu is a mutex to concurrency protect the AlzLib maps } @@ -542,37 +543,28 @@ func (az *AlzLib) RoleDefinition(name string) *assets.RoleDefinition { // BuiltInCache is an interface for providing cached built-in Azure policy definitions // and policy set definitions. When a complete cache is supplied via [AlzLib.AddCache], // the policy client ([AlzLib.AddPolicyClient]) is not required because -// [AlzLib.GetDefinitionsFromAzure] will find every definition already present and skip +// [AlzLib.GetDefinitionsFromAzure] will find every definition in the cache and skip // all Azure API calls. type BuiltInCache interface { PolicyDefinitions() map[string]*assets.PolicyDefinitionVersions PolicySetDefinitions() map[string]*assets.PolicySetDefinitionVersions + // PolicyDefinitionVersionsByName returns the policy definition versions for the given name, + // or nil if not found. + PolicyDefinitionVersionsByName(name string) *assets.PolicyDefinitionVersions + // PolicySetDefinitionVersionsByName returns the policy set definition versions for the given name, + // or nil if not found. + PolicySetDefinitionVersionsByName(name string) *assets.PolicySetDefinitionVersions } -// AddCache pre-populates the AlzLib policy definition and policy set definition maps -// from a [BuiltInCache]. Definitions already present in AlzLib are not overwritten. -// Deep copies are made of every definition to ensure the cache is not mutated. +// AddCache stores a [BuiltInCache] for lazy lookup during [AlzLib.GetDefinitionsFromAzure]. +// Definitions are fetched from the cache on demand rather than being loaded eagerly, +// which allows the garbage collector to discard the cache after GetDefinitionsFromAzure completes. +// A previously stored cache is replaced by the new one. func (az *AlzLib) AddCache(c BuiltInCache) { az.mu.Lock() defer az.mu.Unlock() - for name, pdvs := range c.PolicyDefinitions() { - if _, exists := az.policyDefinitions[name]; exists { - continue - } - - cpy := deep.MustCopy(pdvs) - az.policyDefinitions[name] = cpy - } - - for name, psdvs := range c.PolicySetDefinitions() { - if _, exists := az.policySetDefinitions[name]; exists { - continue - } - - cpy := deep.MustCopy(psdvs) - az.policySetDefinitions[name] = cpy - } + az.cache = c } // AddPolicyClient adds an authenticated *armpolicy.ClientFactory to the AlzLib struct. @@ -655,7 +647,12 @@ func (az *AlzLib) Init(ctx context.Context, libs ...LibraryReference) error { // It then fetches them from Azure if they don't already exist (determined by last segment of // resource id). For set definitions we need to get all of them, even if they exist in AlzLib // already because they can contain built-in definitions. +// If a cache has been set via [AlzLib.AddCache], definitions are looked up from the cache +// before falling back to Azure API calls. +// The cache reference is cleared after this function returns, allowing the garbage collector +// to discard the cache. func (az *AlzLib) GetDefinitionsFromAzure(ctx context.Context, reqs []BuiltInRequest) error { + defer func() { az.cache = nil }() policyDefsToGet := make([]BuiltInRequest, 0, len(reqs)) policySetDefsToGet := make([]BuiltInRequest, 0, len(reqs)) @@ -793,21 +790,83 @@ type policySetDefinitionVersionsPager interface { NextPage(context.Context) (armpolicy.SetDefinitionVersionsClientListBuiltInResponse, error) } +// policyDefinitionFromCache returns the policy definition for the given name and version +// from the cache, or nil if the cache is unset or does not contain a matching entry. +func (az *AlzLib) policyDefinitionFromCache(name string, version *string) *assets.PolicyDefinition { + if az.cache == nil { + return nil + } + + pdvs := az.cache.PolicyDefinitionVersionsByName(name) + if pdvs == nil { + return nil + } + + pd, err := pdvs.GetVersion(version) + if err != nil { + return nil + } + + return pd +} + +// policySetDefinitionFromCache returns the policy set definition for the given name and version +// from the cache, or nil if the cache is unset or does not contain a matching entry. +func (az *AlzLib) policySetDefinitionFromCache(name string, version *string) *assets.PolicySetDefinition { + if az.cache == nil { + return nil + } + + psdvs := az.cache.PolicySetDefinitionVersionsByName(name) + if psdvs == nil { + return nil + } + + psd, err := psdvs.GetVersion(version) + if err != nil { + return nil + } + + return psd +} + // getBuiltInPolicies retrieves the built-in policy definitions with the given names // and adds them to the AlzLib struct. func (az *AlzLib) getBuiltInPolicies(ctx context.Context, reqs []BuiltInRequest) error { - if az.clients.policyClient == nil { - return errors.New("Alzlib.getBuiltInPolicies: policy client not set") - } - - versionedClient := az.clients.policyClient.NewDefinitionVersionsClient() - client := az.clients.policyClient.NewDefinitionsClient() + var ( + versionedClient *armpolicy.DefinitionVersionsClient + client *armpolicy.DefinitionsClient + ) for _, req := range reqs { if az.PolicyDefinitionExists(req.ResourceID.Name, req.Version) { continue } + // Check cache before calling Azure. + if pdv := az.policyDefinitionFromCache(req.ResourceID.Name, req.Version); pdv != nil { + if err := az.AddPolicyDefinitions(pdv); err != nil { + return fmt.Errorf( + "Alzlib.getBuiltInPolicies: adding cached built-in policy definition %s: %w", + JoinNameAndVersion(req.ResourceID.Name, req.Version), + err, + ) + } + + continue + } + + // Fall back to Azure. + if az.clients.policyClient == nil { + return errors.New("Alzlib.getBuiltInPolicies: policy client not set") + } + + // Lazily initialise clients on first Azure call. + if client == nil { + versionedClient = az.clients.policyClient.NewDefinitionVersionsClient() + client = az.clients.policyClient.NewDefinitionsClient() + } + var err error switch req.Version { @@ -828,20 +887,44 @@ func (az *AlzLib) getBuiltInPolicies(ctx context.Context, reqs []BuiltInRequest) // getBuiltInPolicySets retrieves the built-in policy set definitions with the given names // and adds them to the AlzLib struct. func (az *AlzLib) getBuiltInPolicySets(ctx context.Context, reqs []BuiltInRequest) error { - if az.clients.policyClient == nil { - return errors.New("Alzlib.getBuiltInPolicySets: policy client not set") - } - processedRequests := make([]*assets.PolicySetDefinition, 0, len(reqs)) - versionedClient := az.clients.policyClient.NewSetDefinitionVersionsClient() - client := az.clients.policyClient.NewSetDefinitionsClient() + var ( + versionedClient *armpolicy.SetDefinitionVersionsClient + client *armpolicy.SetDefinitionsClient + ) for _, req := range reqs { if az.PolicySetDefinitionExists(req.ResourceID.Name, req.Version) { continue } + // Check cache before calling Azure. + if psdv := az.policySetDefinitionFromCache(req.ResourceID.Name, req.Version); psdv != nil { + if err := az.AddPolicySetDefinitions(psdv); err != nil { + return fmt.Errorf( + "Alzlib.getBuiltInPolicySets: adding cached built-in policy set definition %s: %w", + req.String(), + err, + ) + } + + processedRequests = append(processedRequests, psdv) + + continue + } + + // Fall back to Azure. + if az.clients.policyClient == nil { + return errors.New("Alzlib.getBuiltInPolicySets: policy client not set") + } + + // Lazily initialise clients on first Azure call. + if client == nil { + versionedClient = az.clients.policyClient.NewSetDefinitionVersionsClient() + client = az.clients.policyClient.NewSetDefinitionsClient() + } + var ( psd *assets.PolicySetDefinition err error @@ -1072,15 +1155,10 @@ func (az *AlzLib) ensureReferencedPolicyDefinitions( return nil } - definitionsVersionedClient := az.clients.policyClient.NewDefinitionVersionsClient() - definitionsClient := az.clients.policyClient.NewDefinitionsClient() - for _, def := range processedRequests { for _, ref := range def.Properties.PolicyDefinitions { err := az.ensureReferencedPolicyDefinition( ctx, - definitionsClient, - definitionsVersionedClient, def, ref, ) @@ -1095,8 +1173,6 @@ func (az *AlzLib) ensureReferencedPolicyDefinitions( func (az *AlzLib) ensureReferencedPolicyDefinition( ctx context.Context, - definitionsClient *armpolicy.DefinitionsClient, - definitionsVersionedClient *armpolicy.DefinitionVersionsClient, setDefinition *assets.PolicySetDefinition, ref *armpolicy.DefinitionReference, ) error { @@ -1121,10 +1197,39 @@ func (az *AlzLib) ensureReferencedPolicyDefinition( ) } + // Check AlzLib first to avoid redundant work. + if az.PolicyDefinitionExists(resID.Name, ref.DefinitionVersion) { + return nil + } + + // Check cache before calling Azure. + if pdv := az.policyDefinitionFromCache(resID.Name, ref.DefinitionVersion); pdv != nil { + if err := az.AddPolicyDefinitions(pdv); err != nil { + return fmt.Errorf( + "Alzlib.ensureReferencedPolicyDefinition: adding cached built-in policy definition `%s` "+ + "referenced in policy set `%s`: %w", + JoinNameAndVersion(resID.Name, ref.DefinitionVersion), + JoinNameAndVersion(*setDefinition.Name, setDefinition.Properties.Version), + err, + ) + } + + return nil + } + + // Fall back to Azure. + if az.clients.policyClient == nil { + return errors.New("Alzlib.ensureReferencedPolicyDefinition: policy client not set") + } + if ref.DefinitionVersion != nil { + definitionsVersionedClient := az.clients.policyClient.NewDefinitionVersionsClient() + return az.fetchReferencedPolicyDefinitionVersions(ctx, definitionsVersionedClient, resID.Name, setDefinition, ref) } + definitionsClient := az.clients.policyClient.NewDefinitionsClient() + return az.fetchLatestReferencedPolicyDefinition( ctx, definitionsClient, resID.Name, setDefinition, ref.DefinitionVersion, ) diff --git a/alzlib_test.go b/alzlib_test.go index 3d62f6d..2301337 100644 --- a/alzlib_test.go +++ b/alzlib_test.go @@ -1160,7 +1160,17 @@ func (m *mockBuiltInCache) PolicySetDefinitions() map[string]*assets.PolicySetDe return m.policySetDefs } -func TestAddCachePopulatesDefinitions(t *testing.T) { +func (m *mockBuiltInCache) PolicyDefinitionVersionsByName(name string) *assets.PolicyDefinitionVersions { + return m.policyDefs[name] +} + +func (m *mockBuiltInCache) PolicySetDefinitionVersionsByName(name string) *assets.PolicySetDefinitionVersions { + return m.policySetDefs[name] +} + +// TestAddCacheStoresCacheReference verifies that AddCache stores the cache reference for +// lazy lookup, and that definitions are not immediately loaded into AlzLib. +func TestAddCacheStoresCacheReference(t *testing.T) { t.Parallel() az := NewAlzLib(nil) @@ -1179,23 +1189,56 @@ func TestAddCachePopulatesDefinitions(t *testing.T) { az.AddCache(cache) - // Verify the definitions were added. + // Definitions must NOT be loaded eagerly - they are only fetched on demand. + assert.False(t, az.PolicyDefinitionExists("cached-pd", to.Ptr("1.0.*")), + "cached-pd should not be eagerly loaded into AlzLib by AddCache") + assert.False(t, az.PolicySetDefinitionExists("cached-psd", to.Ptr("1.0.*")), + "cached-psd should not be eagerly loaded into AlzLib by AddCache") +} + +// TestAddCacheDefinitionsLoadedOnDemand verifies that definitions are fetched from cache +// during GetDefinitionsFromAzure and are accessible in AlzLib afterwards. +func TestAddCacheDefinitionsLoadedOnDemand(t *testing.T) { + t.Parallel() + + az := NewAlzLib(nil) + + pdvs := assets.NewPolicyDefinitionVersions() + require.NoError(t, pdvs.Add(testPolicyDefinition(t, "cached-pd", "1.0.0"), false)) + + cache := &mockBuiltInCache{ + policyDefs: map[string]*assets.PolicyDefinitionVersions{"cached-pd": pdvs}, + policySetDefs: map[string]*assets.PolicySetDefinitionVersions{}, + } + + az.AddCache(cache) + + pdResID, err := arm.ParseResourceID("/providers/Microsoft.Authorization/policyDefinitions/cached-pd") + require.NoError(t, err) + + err = az.GetDefinitionsFromAzure(context.Background(), []BuiltInRequest{ + {ResourceID: pdResID, Version: to.Ptr("1.0.*")}, + }) + require.NoError(t, err) + + // After GetDefinitionsFromAzure, the definition should be in AlzLib. assert.True(t, az.PolicyDefinitionExists("cached-pd", to.Ptr("1.0.*"))) - assert.True(t, az.PolicySetDefinitionExists("cached-psd", to.Ptr("1.0.*"))) } -func TestAddCacheDoesNotOverwriteExisting(t *testing.T) { +// TestAddCacheExistingDefinitionNotOverwritten verifies that definitions already in AlzLib +// are not replaced by a cache lookup - they are found first in AlzLib and skipped. +func TestAddCacheExistingDefinitionNotOverwritten(t *testing.T) { t.Parallel() az := NewAlzLib(nil) - // Pre-populate with a policy definition. + // Pre-populate with a specific version. existingPd := testPolicyDefinition(t, "existing-pd", "1.0.0") require.NoError(t, az.AddPolicyDefinitions(existingPd)) - // Build a cache with the same name but different version. + // Cache has the same version. pdvs := assets.NewPolicyDefinitionVersions() - require.NoError(t, pdvs.Add(testPolicyDefinition(t, "existing-pd", "2.0.0"), false)) + require.NoError(t, pdvs.Add(testPolicyDefinition(t, "existing-pd", "1.0.0"), false)) cache := &mockBuiltInCache{ policyDefs: map[string]*assets.PolicyDefinitionVersions{"existing-pd": pdvs}, @@ -1204,13 +1247,21 @@ func TestAddCacheDoesNotOverwriteExisting(t *testing.T) { az.AddCache(cache) + pdResID, err := arm.ParseResourceID("/providers/Microsoft.Authorization/policyDefinitions/existing-pd") + require.NoError(t, err) + + // Requesting a version that already exists in AlzLib should succeed without error. + err = az.GetDefinitionsFromAzure(context.Background(), []BuiltInRequest{ + {ResourceID: pdResID, Version: to.Ptr("1.0.*")}, + }) + require.NoError(t, err) + // The pre-existing version 1.0.0 should still be there. assert.True(t, az.PolicyDefinitionExists("existing-pd", to.Ptr("1.0.*"))) - - // The cache version 2.0.0 should NOT have been added because the name already existed. - assert.False(t, az.PolicyDefinitionExists("existing-pd", to.Ptr("2.0.*"))) } +// TestAddCacheDeepCopies verifies that definitions fetched from cache are deep-copied so +// that subsequent mutations to AlzLib do not affect the original cache entries. func TestAddCacheDeepCopies(t *testing.T) { t.Parallel() @@ -1226,6 +1277,14 @@ func TestAddCacheDeepCopies(t *testing.T) { az.AddCache(cache) + pdResID, err := arm.ParseResourceID("/providers/Microsoft.Authorization/policyDefinitions/deep-copy-pd") + require.NoError(t, err) + + // Fetch the definition from cache via GetDefinitionsFromAzure. + require.NoError(t, az.GetDefinitionsFromAzure(context.Background(), []BuiltInRequest{ + {ResourceID: pdResID, Version: to.Ptr("1.0.*")}, + })) + // Mutate the definition in AlzLib via SetAssignPermissionsOnDefinitionParameter. // This should NOT affect the original cache. az.SetAssignPermissionsOnDefinitionParameter("deep-copy-pd", "anyParam") diff --git a/cache/cache.go b/cache/cache.go index 63c5223..494a01b 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -282,3 +282,15 @@ func (c *Cache) PolicySetDefinitionVersionsForName(name string) []semver.Version return psdvs.Versions() } + +// PolicyDefinitionVersionsByName returns the policy definition versions for the given name, +// or nil if not found. +func (c *Cache) PolicyDefinitionVersionsByName(name string) *assets.PolicyDefinitionVersions { + return c.policyDefinitions[name] +} + +// PolicySetDefinitionVersionsByName returns the policy set definition versions for the given name, +// or nil if not found. +func (c *Cache) PolicySetDefinitionVersionsByName(name string) *assets.PolicySetDefinitionVersions { + return c.policySetDefinitions[name] +} diff --git a/docs/cache.md b/docs/cache.md index 2d1978f..b9278dc 100644 --- a/docs/cache.md +++ b/docs/cache.md @@ -14,7 +14,7 @@ Benchmarks run on Apple M1 Pro comparing the two initialization paths: The cache path trades higher memory usage for a ~96x reduction in wall-clock time. The additional memory is transient — definitions not referenced by the library are eligible for garbage collection after initialization. -The cache uses more memory because it loads all built-in definitions upfront, while the Azure client path fetches only the definitions referenced by the library's policy assignments. +The cache is loaded lazily: only definitions that are actually referenced by the library's policy assignments are fetched from the cache (or from Azure if missing). After `GetDefinitionsFromAzure` returns, the cache reference is cleared so the garbage collector can reclaim it. ## Creating a Cache File @@ -72,7 +72,7 @@ if err != nil { return err } -// Pre-populate AlzLib with cached definitions. +// Register the cache with AlzLib for lazy lookup. az := alzlib.NewAlzLib(nil) az.AddCache(c) @@ -80,9 +80,11 @@ az.AddCache(c) // built-in definitions referenced by the library. // If a definition is missing from the cache, AlzLib falls back // to the Azure client (if one has been set via AddPolicyClient). +// The cache reference is released after GetDefinitionsFromAzure +// returns, allowing the garbage collector to reclaim its memory. ``` -Definitions already present in `AlzLib` (e.g. from library files) are not overwritten by the cache. Deep copies are made of every cached definition to ensure the cache remains immutable after loading. +Definitions are fetched from the cache on demand during `GetDefinitionsFromAzure` (called internally by the deployment package). Only the definitions actually needed are loaded into AlzLib, and deep copies are made of every fetched definition to ensure the cache remains immutable. ## Cache Freshness diff --git a/integrationtest/cache_test.go b/integrationtest/cache_test.go index 60a9e9f..9b2092c 100644 --- a/integrationtest/cache_test.go +++ b/integrationtest/cache_test.go @@ -84,11 +84,17 @@ func TestCacheInitWithLibrary(t *testing.T) { lib := alzlib.NewCustomLibraryReference("./testdata/simple-existingmg") require.NoError(t, az.Init(ctx, lib)) - // Library definitions should be present. + // Library definitions should be present immediately after Init. assert.Contains(t, az.PolicyDefinitions(), "test-policy-definition") assert.Contains(t, az.PolicySetDefinitions(), "test-policy-set-definition") - // Cached built-in definitions should also be present. + // Built-in definitions from cache are loaded lazily on demand via GetDefinitionsFromAzure + // (triggered by deployment.FromArchitecture). Only library-defined definitions are + // present at this point. + h := deployment.NewHierarchy(az) + require.NoError(t, h.FromArchitecture(ctx, "alz", "00000000-0000-0000-0000-000000000000", "testlocation")) + + // After building the hierarchy, referenced built-in definitions should be present. assert.Greater(t, len(az.PolicyDefinitions()), 1) assert.Greater(t, len(az.PolicySetDefinitions()), 1) } From 73bbd440599afb2904f3085fa259190ee1973829 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:51:53 +0000 Subject: [PATCH 03/12] feat(cache): add ExportBuiltInCache method and NewCacheFromDefinitions constructor - cache/cache.go: Add NewCacheFromDefinitions constructor that builds a Cache from existing PolicyDefinitionVersions/PolicySetDefinitionVersions maps - alzlib.go: Add ExportBuiltInCache() method that collects only built-in and static definitions from the loaded AlzLib state, filtering out custom definitions - alzlib.go: Add isBuiltInPolicyDefinitionVersions / isBuiltInPolicySetDefinitionVersions helpers for PolicyType-based filtering - alzlib_test.go: Add 5 tests for ExportBuiltInCache covering the filter logic, save/reload round-trip, empty state, nil PolicyType, and static type Co-authored-by: matt-FFFFFF <16320656+matt-FFFFFF@users.noreply.github.com> --- alzlib.go | 72 +++++++++++++++++++++++++ alzlib_test.go | 144 +++++++++++++++++++++++++++++++++++++++++++++++++ cache/cache.go | 25 +++++++++ 3 files changed, 241 insertions(+) diff --git a/alzlib.go b/alzlib.go index fc4ebe8..da89f4a 100644 --- a/alzlib.go +++ b/alzlib.go @@ -12,6 +12,7 @@ import ( "sync" "github.com/Azure/alzlib/assets" + "github.com/Azure/alzlib/cache" "github.com/Azure/alzlib/internal/processor" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy" @@ -567,6 +568,77 @@ func (az *AlzLib) AddCache(c BuiltInCache) { az.cache = c } +// ExportBuiltInCache creates a [cache.Cache] from the built-in policy definitions and policy +// set definitions that are currently loaded in AlzLib. Only definitions with a +// [armpolicy.PolicyTypeBuiltIn] or [armpolicy.PolicyTypeStatic] policy type are included; +// custom definitions (loaded from library files) are excluded. +// +// This is useful when callers want to persist a minimal cache covering only the definitions +// referenced by their specific library — rather than using alzlibtool to cache everything. +// The returned cache can be saved with [cache.Cache.Save] and reloaded later with +// [cache.NewCache], then re-injected via [AlzLib.AddCache] to skip Azure API calls on +// subsequent runs. +func (az *AlzLib) ExportBuiltInCache() *cache.Cache { + az.mu.RLock() + defer az.mu.RUnlock() + + pds := make(map[string]*assets.PolicyDefinitionVersions) + psds := make(map[string]*assets.PolicySetDefinitionVersions) + + for name, pdvs := range az.policyDefinitions { + if !isBuiltInPolicyDefinitionVersions(pdvs) { + continue + } + + pds[name] = pdvs + } + + for name, psdvs := range az.policySetDefinitions { + if !isBuiltInPolicySetDefinitionVersions(psdvs) { + continue + } + + psds[name] = psdvs + } + + return cache.NewCacheFromDefinitions(pds, psds) +} + +// isBuiltInPolicyDefinitionVersions returns true if any version in the collection has a +// built-in or static policy type. Collections with no type set are treated as built-in to +// ensure definitions fetched from Azure (which always set PolicyType) are never excluded. +func isBuiltInPolicyDefinitionVersions(pdvs *assets.PolicyDefinitionVersions) bool { + for def := range pdvs.AllVersions() { + if def.Properties == nil || def.Properties.PolicyType == nil { + return true + } + + switch *def.Properties.PolicyType { + case armpolicy.PolicyTypeBuiltIn, armpolicy.PolicyTypeStatic: + return true + } + } + + return false +} + +// isBuiltInPolicySetDefinitionVersions returns true if any version in the collection has a +// built-in or static policy type. Collections with no type set are treated as built-in. +func isBuiltInPolicySetDefinitionVersions(psdvs *assets.PolicySetDefinitionVersions) bool { + for def := range psdvs.AllVersions() { + if def.Properties == nil || def.Properties.PolicyType == nil { + return true + } + + switch *def.Properties.PolicyType { + case armpolicy.PolicyTypeBuiltIn, armpolicy.PolicyTypeStatic: + return true + } + } + + return false +} + // AddPolicyClient adds an authenticated *armpolicy.ClientFactory to the AlzLib struct. // This is needed to get policy objects from Azure. func (az *AlzLib) AddPolicyClient(client *armpolicy.ClientFactory) { diff --git a/alzlib_test.go b/alzlib_test.go index 2301337..331c6d0 100644 --- a/alzlib_test.go +++ b/alzlib_test.go @@ -4,12 +4,14 @@ package alzlib import ( + "bytes" "context" "os" "path/filepath" "testing" "github.com/Azure/alzlib/assets" + "github.com/Azure/alzlib/cache" "github.com/Azure/alzlib/internal/auth" "github.com/Azure/alzlib/internal/processor" "github.com/Azure/alzlib/to" @@ -1969,3 +1971,145 @@ func testPolicySetDefinition(t *testing.T, name, version string) *assets.PolicyS }, } } + +// testBuiltInPolicyDefinition creates a policy definition with PolicyTypeBuiltIn. +func testBuiltInPolicyDefinition(t *testing.T, name, version string) *assets.PolicyDefinition { + t.Helper() + + desc := name + " description" + + return &assets.PolicyDefinition{ + Definition: armpolicy.Definition{ + Name: to.Ptr(name), + Properties: &armpolicy.DefinitionProperties{ + DisplayName: to.Ptr(name), + Description: &desc, + Metadata: map[string]any{}, + PolicyRule: map[string]any{"if": map[string]any{}, "then": map[string]any{}}, + PolicyType: to.Ptr(armpolicy.PolicyTypeBuiltIn), + Version: to.Ptr(version), + }, + }, + } +} + +// testBuiltInPolicySetDefinition creates a policy set definition with PolicyTypeBuiltIn. +func testBuiltInPolicySetDefinition(t *testing.T, name, version string) *assets.PolicySetDefinition { + t.Helper() + + desc := name + " description" + + return &assets.PolicySetDefinition{ + SetDefinition: armpolicy.SetDefinition{ + Name: to.Ptr(name), + Properties: &armpolicy.SetDefinitionProperties{ + DisplayName: to.Ptr(name), + Description: &desc, + Metadata: map[string]any{}, + PolicyDefinitions: []*armpolicy.DefinitionReference{}, + Parameters: map[string]*armpolicy.ParameterDefinitionsValue{}, + PolicyType: to.Ptr(armpolicy.PolicyTypeBuiltIn), + Version: to.Ptr(version), + }, + }, + } +} + +// TestExportBuiltInCacheOnlyIncludesBuiltIns verifies that ExportBuiltInCache filters out +// custom policy definitions and set definitions, keeping only built-ins. +func TestExportBuiltInCacheOnlyIncludesBuiltIns(t *testing.T) { + t.Parallel() + + az := NewAlzLib(nil) + + // Add a built-in policy definition. + require.NoError(t, az.AddPolicyDefinitions(testBuiltInPolicyDefinition(t, "builtin-pd", "1.0.0"))) + + // Add a custom policy definition (PolicyTypeCustom). + customPd := testPolicyDefinition(t, "custom-pd", "1.0.0") + customPd.Properties.PolicyType = to.Ptr(armpolicy.PolicyTypeCustom) + require.NoError(t, az.AddPolicyDefinitions(customPd)) + + // Add a built-in policy set definition. + require.NoError(t, az.AddPolicySetDefinitions(testBuiltInPolicySetDefinition(t, "builtin-psd", "1.0.0"))) + + // Add a custom policy set definition. + customPsd := testPolicySetDefinition(t, "custom-psd", "1.0.0") + require.NoError(t, az.AddPolicySetDefinitions(customPsd)) + + c := az.ExportBuiltInCache() + + // Only built-in definitions should be in the exported cache. + pds := c.PolicyDefinitions() + psds := c.PolicySetDefinitions() + + assert.Contains(t, pds, "builtin-pd", "built-in policy definition should be in cache") + assert.NotContains(t, pds, "custom-pd", "custom policy definition should not be in cache") + assert.Contains(t, psds, "builtin-psd", "built-in policy set definition should be in cache") + assert.NotContains(t, psds, "custom-psd", "custom policy set definition should not be in cache") +} + +// TestExportBuiltInCacheStaticIsIncluded verifies that StaticBuiltIn definitions are included. +func TestExportBuiltInCacheStaticIsIncluded(t *testing.T) { + t.Parallel() + + az := NewAlzLib(nil) + + staticPd := testPolicyDefinition(t, "static-pd", "1.0.0") + staticPd.Properties.PolicyType = to.Ptr(armpolicy.PolicyTypeStatic) + require.NoError(t, az.AddPolicyDefinitions(staticPd)) + + c := az.ExportBuiltInCache() + + assert.Contains(t, c.PolicyDefinitions(), "static-pd", "static policy definition should be in cache") +} + +// TestExportBuiltInCacheNilPolicyTypeIsIncluded verifies that definitions with nil PolicyType +// are treated as built-in and included in the exported cache. +func TestExportBuiltInCacheNilPolicyTypeIsIncluded(t *testing.T) { + t.Parallel() + + az := NewAlzLib(nil) + + // testPolicyDefinition does not set PolicyType (nil). + require.NoError(t, az.AddPolicyDefinitions(testPolicyDefinition(t, "nil-type-pd", "1.0.0"))) + + c := az.ExportBuiltInCache() + + assert.Contains(t, c.PolicyDefinitions(), "nil-type-pd", + "definition with nil PolicyType should be treated as built-in") +} + +// TestExportBuiltInCacheEmpty verifies that an empty AlzLib produces an empty cache. +func TestExportBuiltInCacheEmpty(t *testing.T) { + t.Parallel() + + az := NewAlzLib(nil) + c := az.ExportBuiltInCache() + + assert.Empty(t, c.PolicyDefinitions()) + assert.Empty(t, c.PolicySetDefinitions()) +} + +// TestExportBuiltInCacheCanSaveAndReload verifies that the exported cache can be saved and +// re-loaded via cache.NewCache without errors. +func TestExportBuiltInCacheCanSaveAndReload(t *testing.T) { + t.Parallel() + + az := NewAlzLib(nil) + require.NoError(t, az.AddPolicyDefinitions(testBuiltInPolicyDefinition(t, "save-pd", "2.0.0"))) + require.NoError(t, az.AddPolicySetDefinitions(testBuiltInPolicySetDefinition(t, "save-psd", "2.0.0"))) + + exported := az.ExportBuiltInCache() + + var buf bytes.Buffer + require.NoError(t, exported.Save(&buf)) + + reloaded, err := cache.NewCache(&buf) + require.NoError(t, err) + + assert.Equal(t, 1, reloaded.PolicyDefinitionNames()) + assert.Equal(t, 1, reloaded.PolicySetDefinitionNames()) + assert.Equal(t, 1, reloaded.PolicyDefinitionCount()) + assert.Equal(t, 1, reloaded.PolicySetDefinitionCount()) +} diff --git a/cache/cache.go b/cache/cache.go index 494a01b..2c46180 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -141,6 +141,31 @@ func NewCache(r io.Reader) (*Cache, error) { return c, nil } +// NewCacheFromDefinitions creates a [Cache] from pre-loaded policy definition and policy set +// definition version collections. This is useful when callers want to generate a cache that +// is tailored to their specific use case — for example, after calling +// [alzlib.AlzLib.GetDefinitionsFromAzure] to load only the definitions referenced by a +// particular library. +// +// The supplied maps are shallow-copied: the keys are copied but the value pointers are +// shared. Callers should not modify the values after creating the cache. +func NewCacheFromDefinitions( + pds map[string]*assets.PolicyDefinitionVersions, + psds map[string]*assets.PolicySetDefinitionVersions, +) *Cache { + c := &Cache{ + policyDefinitions: make(map[string]*assets.PolicyDefinitionVersions, len(pds)), + policySetDefinitions: make(map[string]*assets.PolicySetDefinitionVersions, len(psds)), + } + + maps.Copy(c.policyDefinitions, pds) + maps.Copy(c.policySetDefinitions, psds) + + c.computeCounts() + + return c +} + // computeCounts calculates and stores the total version counts for // policy definitions and policy set definitions. func (c *Cache) computeCounts() { From a4bd09b916b82ccc630baab830d21f630eba4139 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 23:09:23 +0000 Subject: [PATCH 04/12] feat(alzlibtool): add --from-cache to generate architecture; add library+architecture mode to cache create - generate/architecture.go: add --from-cache flag; loads seed cache into AlzLib before Init/FromArchitecture to reduce Azure API calls - cache/create.go: add --library, --architecture, --from-cache, --rootmg, --location flags; new architecture-scoped mode uses alzlib.ExportBuiltInCache() to produce a minimal cache; same path allowed for --from-cache and --output (in-place update) Co-authored-by: matt-FFFFFF <16320656+matt-FFFFFF@users.noreply.github.com> --- cmd/alzlibtool/command/cache/create.go | 194 ++++++++++++++++-- .../command/generate/architecture.go | 28 +++ 2 files changed, 200 insertions(+), 22 deletions(-) diff --git a/cmd/alzlibtool/command/cache/create.go b/cmd/alzlibtool/command/cache/create.go index ea6c002..d3b944a 100644 --- a/cmd/alzlibtool/command/cache/create.go +++ b/cmd/alzlibtool/command/cache/create.go @@ -7,7 +7,9 @@ import ( "log/slog" "os" + alzlib "github.com/Azure/alzlib" "github.com/Azure/alzlib/cache" + "github.com/Azure/alzlib/deployment" "github.com/Azure/alzlib/internal/auth" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" @@ -15,30 +17,70 @@ import ( "github.com/spf13/cobra" ) +const ( + // defaultRootMgID is the placeholder management group ID used when none is supplied. + defaultRootMgID = "00000000-0000-0000-0000-000000000000" + // defaultLocation is the default Azure location for architecture processing. + defaultLocation = "northeurope" +) + var createCmd = cobra.Command{ Use: "create", Short: "Create a cache file from Azure built-in definitions.", - Long: `Scans an Azure tenant for all built-in policy definitions and policy set definitions -and writes them to a local cache file. Requires Azure credentials (e.g. az login).`, + Long: `Creates a cache of Azure built-in policy definitions and policy set definitions. + +By default, the full set of Azure built-in definitions is scanned from the tenant and written +to the output file. Requires Azure credentials (e.g. az login). + +When --library and --architecture are specified, only the definitions referenced by that +architecture are included, producing a smaller, use-case-specific cache. This is useful for +embedding the minimal set of definitions needed for a given deployment workflow. + +Use --from-cache to seed from an existing cache file. Definitions already present in the seed +cache are used directly and not re-fetched from Azure. This allows efficient incremental updates. +The same file may be used for both --from-cache and --output to update a cache in-place.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, _ []string) { outFile, _ := cmd.Flags().GetString("output") verbose, _ := cmd.Flags().GetBool("verbose") + libraryPath, _ := cmd.Flags().GetString("library") + architectureName, _ := cmd.Flags().GetString("architecture") + fromCacheFile, _ := cmd.Flags().GetString("from-cache") + rootMg, _ := cmd.Flags().GetString("rootmg") + location, _ := cmd.Flags().GetString("location") - creds, err := auth.NewToken() - if err != nil { - cmd.PrintErrf("%s could not get Azure credential: %v\n", cmd.ErrPrefix(), err) + // --library and --architecture must be specified together. + if (libraryPath == "") != (architectureName == "") { + cmd.PrintErrf( + "%s --library and --architecture must be specified together\n", + cmd.ErrPrefix(), + ) os.Exit(1) } - cf, err := armpolicy.NewClientFactory("", creds, &arm.ClientOptions{ - ClientOptions: policy.ClientOptions{ - Cloud: auth.GetCloudFromEnv(), - }, - }) - if err != nil { - cmd.PrintErrf("%s could not create Azure policy client factory: %v\n", cmd.ErrPrefix(), err) - os.Exit(1) + // Read the seed cache BEFORE opening the output file, because --from-cache + // and --output may point to the same path. + var seedCache *cache.Cache + if fromCacheFile != "" { + f, err := os.Open(fromCacheFile) + if err != nil { + cmd.PrintErrf( + "%s could not open seed cache file %s: %v\n", + cmd.ErrPrefix(), fromCacheFile, err, + ) + os.Exit(1) + } + + seedCache, err = cache.NewCache(f) + f.Close() //nolint:errcheck // close immediately; do not defer so the same path can be opened for writing below + + if err != nil { + cmd.PrintErrf( + "%s could not read seed cache %s: %v\n", + cmd.ErrPrefix(), fromCacheFile, err, + ) + os.Exit(1) + } } var logger *slog.Logger @@ -46,33 +88,118 @@ and writes them to a local cache file. Requires Azure credentials (e.g. az login logger = slog.New(slog.NewTextHandler(cmd.OutOrStdout(), &slog.HandlerOptions{ Level: slog.LevelInfo, })) + } else { + logger = slog.New(slog.DiscardHandler) } - cmd.Printf("Scanning Azure tenant for built-in definitions...\n") + var resultCache *cache.Cache - c, err := cache.NewCacheFromAzure(cmd.Context(), cf, logger) - if err != nil { - cmd.PrintErrf("%s could not create cache from Azure: %v\n", cmd.ErrPrefix(), err) - os.Exit(1) + if libraryPath != "" { + // Architecture-scoped mode: process the library + architecture and export + // only the built-in definitions that were actually referenced. + thisLib := alzlib.NewCustomLibraryReference(libraryPath) + + allLibs, err := thisLib.FetchWithDependencies(cmd.Context()) + if err != nil { + cmd.PrintErrf( + "%s could not fetch libraries with dependencies: %v\n", + cmd.ErrPrefix(), err, + ) + os.Exit(1) + } + + az := alzlib.NewAlzLib(nil) + + if seedCache != nil { + az.AddCache(seedCache) + } + + creds, err := auth.NewToken() + if err != nil { + cmd.PrintErrf("%s could not get Azure credential: %v\n", cmd.ErrPrefix(), err) + os.Exit(1) + } + + cf, err := armpolicy.NewClientFactory("", creds, &arm.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Cloud: auth.GetCloudFromEnv(), + }, + }) + if err != nil { + cmd.PrintErrf( + "%s could not create Azure policy client factory: %v\n", + cmd.ErrPrefix(), err, + ) + os.Exit(1) + } + + az.AddPolicyClient(cf) + + if err := az.Init(cmd.Context(), allLibs...); err != nil { + cmd.PrintErrf("%s could not initialize alzlib: %v\n", cmd.ErrPrefix(), err) + os.Exit(1) + } + + h := deployment.NewHierarchy(az) + if err := h.FromArchitecture(cmd.Context(), architectureName, rootMg, location); err != nil { + cmd.PrintErrf( + "%s could not process architecture %q: %v\n", + cmd.ErrPrefix(), architectureName, err, + ) + os.Exit(1) + } + + resultCache = az.ExportBuiltInCache() + } else { + // Full-scan mode: fetch all Azure built-in definitions from the tenant. + creds, err := auth.NewToken() + if err != nil { + cmd.PrintErrf("%s could not get Azure credential: %v\n", cmd.ErrPrefix(), err) + os.Exit(1) + } + + cf, err := armpolicy.NewClientFactory("", creds, &arm.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Cloud: auth.GetCloudFromEnv(), + }, + }) + if err != nil { + cmd.PrintErrf( + "%s could not create Azure policy client factory: %v\n", + cmd.ErrPrefix(), err, + ) + os.Exit(1) + } + + cmd.Printf("Scanning Azure tenant for built-in definitions...\n") + + resultCache, err = cache.NewCacheFromAzure(cmd.Context(), cf, logger) + if err != nil { + cmd.PrintErrf("%s could not create cache from Azure: %v\n", cmd.ErrPrefix(), err) + os.Exit(1) + } } f, err := os.Create(outFile) if err != nil { - cmd.PrintErrf("%s could not create output file %s: %v\n", cmd.ErrPrefix(), outFile, err) + cmd.PrintErrf( + "%s could not create output file %s: %v\n", + cmd.ErrPrefix(), outFile, err, + ) os.Exit(1) } defer f.Close() //nolint:errcheck - if err := c.Save(f); err != nil { + if err := resultCache.Save(f); err != nil { cmd.PrintErrf("%s could not write cache file: %v\n", cmd.ErrPrefix(), err) os.Exit(1) } cmd.Printf("Cache written to %s\n", outFile) cmd.Printf(" Policy definitions: %d names, %d total versions\n", - c.PolicyDefinitionNames(), c.PolicyDefinitionCount()) + resultCache.PolicyDefinitionNames(), resultCache.PolicyDefinitionCount()) cmd.Printf(" Policy set definitions: %d names, %d total versions\n", - c.PolicySetDefinitionNames(), c.PolicySetDefinitionCount()) + resultCache.PolicySetDefinitionNames(), resultCache.PolicySetDefinitionCount()) }, } @@ -81,4 +208,27 @@ func init() { StringP("output", "o", "alzlib-cache.json.gz", "Path to the output cache file.") createCmd.Flags(). BoolP("verbose", "v", false, "Display detailed progress during cache creation.") + createCmd.Flags(). + StringP( + "library", "L", "", + "Path to a library. When set together with --architecture, creates a minimal cache "+ + "containing only the definitions referenced by the specified architecture.") + createCmd.Flags(). + StringP( + "architecture", "a", "", + "Name of the architecture within the library to process. Requires --library.") + createCmd.Flags(). + String( + "from-cache", "", + "Path to an existing cache file to use as a seed. Definitions found in the seed cache "+ + "are not re-fetched from Azure. The same path may be used for both --from-cache and --output "+ + "to update a cache in-place.") + createCmd.Flags(). + StringP( + "rootmg", "r", defaultRootMgID, + "Root management group ID to use when processing the architecture (used with --library and --architecture).") + createCmd.Flags(). + StringP( + "location", "l", defaultLocation, + "Location to use when processing the architecture (used with --library and --architecture).") } diff --git a/cmd/alzlibtool/command/generate/architecture.go b/cmd/alzlibtool/command/generate/architecture.go index 07a9dc2..a7d22f6 100644 --- a/cmd/alzlibtool/command/generate/architecture.go +++ b/cmd/alzlibtool/command/generate/architecture.go @@ -10,6 +10,7 @@ import ( "regexp" "github.com/Azure/alzlib" + alzlibcache "github.com/Azure/alzlib/cache" "github.com/Azure/alzlib/deployment" "github.com/Azure/alzlib/internal/auth" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" @@ -44,6 +45,25 @@ var generateArchitectureBaseCmd = cobra.Command{ az := alzlib.NewAlzLib(nil) + // Load seed cache if --from-cache is specified. + fromCacheFile, _ := cmd.Flags().GetString("from-cache") + if fromCacheFile != "" { + f, err := os.Open(fromCacheFile) + if err != nil { + cmd.PrintErrf("%s could not open cache file %s: %v\n", cmd.ErrPrefix(), fromCacheFile, err) + os.Exit(1) + } + defer f.Close() //nolint:errcheck + + c, err := alzlibcache.NewCache(f) + if err != nil { + cmd.PrintErrf("%s could not load cache file %s: %v\n", cmd.ErrPrefix(), fromCacheFile, err) + os.Exit(1) + } + + az.AddCache(c) + } + creds, err := auth.NewToken() if err != nil { cmd.PrintErrf("%s could not get Azure credential: %v\n", cmd.ErrPrefix(), err) @@ -141,4 +161,12 @@ func init() { "for-alz-bicep", false, "When exporting to a directory, add custom ARM escaping and other transformations specific to ALZ Bicep.") + + generateArchitectureBaseCmd.Flags(). + String( + "from-cache", + "", + "Path to a cache file to seed built-in definitions from. "+ + "Definitions found in the cache are used before falling back to Azure API calls, "+ + "reducing the number of requests made to Azure.") } From 3cb6a94d6994ec9da7a954d1967f5e280713a3fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 00:03:37 +0000 Subject: [PATCH 05/12] fix: address unresolved review comments in alzlib.go - ExportBuiltInCache: deep-copy *PolicyDefinitionVersions values so AlzLib mutations (e.g. SetAssignPermissionsOnDefinitionParameter) cannot affect the exported cache - GetDefinitionsFromAzure: remove defer az.cache=nil; cache is now kept for AlzLib lifetime (callers release with AddCache(nil)). Fixes bug where only the first management group could use the cache. - policyDefinitionFromCache / policySetDefinitionFromCache: snapshot az.cache under az.mu.RLock to prevent data race with AddCache - ensureReferencedPolicyDefinitions: lazily create DefinitionsClient / DefinitionVersionsClient once per invocation and reuse across all policy-set sub-references instead of creating per cache miss - Add TestExportBuiltInCacheIsDeepCopied and helper testBuiltInPolicyDefinitionWithParam - Update AddCache docstring and docs/cache.md for new cache lifecycle Co-authored-by: matt-FFFFFF <16320656+matt-FFFFFF@users.noreply.github.com> --- alzlib.go | 52 +++++++++++++++++++++++++++++++++++--------------- alzlib_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++ docs/cache.md | 6 +++--- 3 files changed, 86 insertions(+), 18 deletions(-) diff --git a/alzlib.go b/alzlib.go index da89f4a..9277ca5 100644 --- a/alzlib.go +++ b/alzlib.go @@ -558,8 +558,9 @@ type BuiltInCache interface { } // AddCache stores a [BuiltInCache] for lazy lookup during [AlzLib.GetDefinitionsFromAzure]. -// Definitions are fetched from the cache on demand rather than being loaded eagerly, -// which allows the garbage collector to discard the cache after GetDefinitionsFromAzure completes. +// Definitions are fetched from the cache on demand rather than being loaded eagerly. +// The cache is retained for the lifetime of AlzLib; call AddCache(nil) to release it +// explicitly and allow the garbage collector to reclaim the memory. // A previously stored cache is replaced by the new one. func (az *AlzLib) AddCache(c BuiltInCache) { az.mu.Lock() @@ -590,7 +591,7 @@ func (az *AlzLib) ExportBuiltInCache() *cache.Cache { continue } - pds[name] = pdvs + pds[name] = deep.MustCopy(pdvs) } for name, psdvs := range az.policySetDefinitions { @@ -598,7 +599,7 @@ func (az *AlzLib) ExportBuiltInCache() *cache.Cache { continue } - psds[name] = psdvs + psds[name] = deep.MustCopy(psdvs) } return cache.NewCacheFromDefinitions(pds, psds) @@ -721,10 +722,9 @@ func (az *AlzLib) Init(ctx context.Context, libs ...LibraryReference) error { // already because they can contain built-in definitions. // If a cache has been set via [AlzLib.AddCache], definitions are looked up from the cache // before falling back to Azure API calls. -// The cache reference is cleared after this function returns, allowing the garbage collector -// to discard the cache. +// The cache is retained for the lifetime of AlzLib; callers can explicitly clear it +// by calling AddCache(nil) when they no longer need it. func (az *AlzLib) GetDefinitionsFromAzure(ctx context.Context, reqs []BuiltInRequest) error { - defer func() { az.cache = nil }() policyDefsToGet := make([]BuiltInRequest, 0, len(reqs)) policySetDefsToGet := make([]BuiltInRequest, 0, len(reqs)) @@ -865,11 +865,15 @@ type policySetDefinitionVersionsPager interface { // policyDefinitionFromCache returns the policy definition for the given name and version // from the cache, or nil if the cache is unset or does not contain a matching entry. func (az *AlzLib) policyDefinitionFromCache(name string, version *string) *assets.PolicyDefinition { - if az.cache == nil { + az.mu.RLock() + c := az.cache + az.mu.RUnlock() + + if c == nil { return nil } - pdvs := az.cache.PolicyDefinitionVersionsByName(name) + pdvs := c.PolicyDefinitionVersionsByName(name) if pdvs == nil { return nil } @@ -885,11 +889,15 @@ func (az *AlzLib) policyDefinitionFromCache(name string, version *string) *asset // policySetDefinitionFromCache returns the policy set definition for the given name and version // from the cache, or nil if the cache is unset or does not contain a matching entry. func (az *AlzLib) policySetDefinitionFromCache(name string, version *string) *assets.PolicySetDefinition { - if az.cache == nil { + az.mu.RLock() + c := az.cache + az.mu.RUnlock() + + if c == nil { return nil } - psdvs := az.cache.PolicySetDefinitionVersionsByName(name) + psdvs := c.PolicySetDefinitionVersionsByName(name) if psdvs == nil { return nil } @@ -1227,12 +1235,20 @@ func (az *AlzLib) ensureReferencedPolicyDefinitions( return nil } + // Lazily initialised Azure clients, shared across all references within this invocation. + var ( + definitionsClient *armpolicy.DefinitionsClient + definitionsVersionedClient *armpolicy.DefinitionVersionsClient + ) + for _, def := range processedRequests { for _, ref := range def.Properties.PolicyDefinitions { err := az.ensureReferencedPolicyDefinition( ctx, def, ref, + &definitionsClient, + &definitionsVersionedClient, ) if err != nil { return err @@ -1247,6 +1263,8 @@ func (az *AlzLib) ensureReferencedPolicyDefinition( ctx context.Context, setDefinition *assets.PolicySetDefinition, ref *armpolicy.DefinitionReference, + definitionsClient **armpolicy.DefinitionsClient, + definitionsVersionedClient **armpolicy.DefinitionVersionsClient, ) error { if ref == nil { return nil @@ -1295,15 +1313,19 @@ func (az *AlzLib) ensureReferencedPolicyDefinition( } if ref.DefinitionVersion != nil { - definitionsVersionedClient := az.clients.policyClient.NewDefinitionVersionsClient() + if *definitionsVersionedClient == nil { + *definitionsVersionedClient = az.clients.policyClient.NewDefinitionVersionsClient() + } - return az.fetchReferencedPolicyDefinitionVersions(ctx, definitionsVersionedClient, resID.Name, setDefinition, ref) + return az.fetchReferencedPolicyDefinitionVersions(ctx, *definitionsVersionedClient, resID.Name, setDefinition, ref) } - definitionsClient := az.clients.policyClient.NewDefinitionsClient() + if *definitionsClient == nil { + *definitionsClient = az.clients.policyClient.NewDefinitionsClient() + } return az.fetchLatestReferencedPolicyDefinition( - ctx, definitionsClient, resID.Name, setDefinition, ref.DefinitionVersion, + ctx, *definitionsClient, resID.Name, setDefinition, ref.DefinitionVersion, ) } diff --git a/alzlib_test.go b/alzlib_test.go index 331c6d0..e1c565a 100644 --- a/alzlib_test.go +++ b/alzlib_test.go @@ -1993,6 +1993,19 @@ func testBuiltInPolicyDefinition(t *testing.T, name, version string) *assets.Pol } } +// testBuiltInPolicyDefinitionWithParam creates a built-in policy definition that includes a named +// parameter, allowing tests that exercise parameter mutation (e.g. SetAssignPermissionsOnDefinitionParameter). +func testBuiltInPolicyDefinitionWithParam(t *testing.T, name, version, paramName string) *assets.PolicyDefinition { + t.Helper() + + pd := testBuiltInPolicyDefinition(t, name, version) + pd.Properties.Parameters = map[string]*armpolicy.ParameterDefinitionsValue{ + paramName: {Type: to.Ptr(armpolicy.ParameterTypeString)}, + } + + return pd +} + // testBuiltInPolicySetDefinition creates a policy set definition with PolicyTypeBuiltIn. func testBuiltInPolicySetDefinition(t *testing.T, name, version string) *assets.PolicySetDefinition { t.Helper() @@ -2113,3 +2126,36 @@ func TestExportBuiltInCacheCanSaveAndReload(t *testing.T) { assert.Equal(t, 1, reloaded.PolicyDefinitionCount()) assert.Equal(t, 1, reloaded.PolicySetDefinitionCount()) } + +// TestExportBuiltInCacheIsDeepCopied verifies that mutating AlzLib after ExportBuiltInCache does +// not affect the exported cache (i.e., the export contains independent copies of the definitions). +func TestExportBuiltInCacheIsDeepCopied(t *testing.T) { + t.Parallel() + + az := NewAlzLib(nil) + + // Build a built-in policy definition that has a parameter so we can exercise mutation. + pd := testBuiltInPolicyDefinitionWithParam(t, "dc-pd", "1.0.0", "myParam") + require.NoError(t, az.AddPolicyDefinitions(pd)) + + exported := az.ExportBuiltInCache() + + // Mutate the AlzLib copy; this must NOT touch the exported cache. + az.SetAssignPermissionsOnDefinitionParameter("dc-pd", "myParam") + + // Retrieve the exported definition directly from the cache. + exportedPdvs := exported.PolicyDefinitionVersionsByName("dc-pd") + require.NotNil(t, exportedPdvs) + + exportedPd, err := exportedPdvs.GetVersion(to.Ptr("1.0.*")) + require.NoError(t, err) + require.NotNil(t, exportedPd) + + if exportedPd.Properties != nil && exportedPd.Properties.Parameters != nil { + if param, ok := exportedPd.Properties.Parameters["myParam"]; ok { + if param.Metadata != nil && param.Metadata.AssignPermissions != nil { + t.Fatal("exported cache definition was mutated - ExportBuiltInCache is not deep-copying") + } + } + } +} diff --git a/docs/cache.md b/docs/cache.md index b9278dc..3e36e4f 100644 --- a/docs/cache.md +++ b/docs/cache.md @@ -14,7 +14,7 @@ Benchmarks run on Apple M1 Pro comparing the two initialization paths: The cache path trades higher memory usage for a ~96x reduction in wall-clock time. The additional memory is transient — definitions not referenced by the library are eligible for garbage collection after initialization. -The cache is loaded lazily: only definitions that are actually referenced by the library's policy assignments are fetched from the cache (or from Azure if missing). After `GetDefinitionsFromAzure` returns, the cache reference is cleared so the garbage collector can reclaim it. +The cache is loaded lazily: only definitions that are actually referenced by the library's policy assignments are fetched from the cache (or from Azure if missing). The cache reference is retained for the lifetime of AlzLib — call `az.AddCache(nil)` to release it explicitly when no further lookups are needed. ## Creating a Cache File @@ -80,8 +80,8 @@ az.AddCache(c) // built-in definitions referenced by the library. // If a definition is missing from the cache, AlzLib falls back // to the Azure client (if one has been set via AddPolicyClient). -// The cache reference is released after GetDefinitionsFromAzure -// returns, allowing the garbage collector to reclaim its memory. +// The cache is kept until explicitly released — call az.AddCache(nil) +// to allow the garbage collector to reclaim its memory. ``` Definitions are fetched from the cache on demand during `GetDefinitionsFromAzure` (called internally by the deployment package). Only the definitions actually needed are loaded into AlzLib, and deep copies are made of every fetched definition to ensure the cache remains immutable. From 28aa64cc6a7a8bc8d7b875c7481bffbf64ae2b1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 07:52:29 +0000 Subject: [PATCH 06/12] fix: snapshot policyClient under RLock in Azure fallback paths; fix ExportBuiltInCache docstring - getBuiltInPolicies: snapshot az.clients.policyClient under az.mu.RLock() before the Azure fallback to prevent data race with AddPolicyClient - getBuiltInPolicySets: same fix - ensureReferencedPolicyDefinition: same fix - ExportBuiltInCache: update docstring to accurately reflect that nil PolicyType is also included (treated as built-in), not only PolicyTypeBuiltIn and PolicyTypeStatic Co-authored-by: matt-FFFFFF <16320656+matt-FFFFFF@users.noreply.github.com> --- alzlib.go | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/alzlib.go b/alzlib.go index 9277ca5..6737e34 100644 --- a/alzlib.go +++ b/alzlib.go @@ -570,9 +570,10 @@ func (az *AlzLib) AddCache(c BuiltInCache) { } // ExportBuiltInCache creates a [cache.Cache] from the built-in policy definitions and policy -// set definitions that are currently loaded in AlzLib. Only definitions with a -// [armpolicy.PolicyTypeBuiltIn] or [armpolicy.PolicyTypeStatic] policy type are included; -// custom definitions (loaded from library files) are excluded. +// set definitions that are currently loaded in AlzLib. Definitions are included when their +// policy type is [armpolicy.PolicyTypeBuiltIn], [armpolicy.PolicyTypeStatic], or is +// unspecified/nil (which is treated as built-in); custom definitions (loaded from library +// files) are excluded. // // This is useful when callers want to persist a minimal cache covering only the definitions // referenced by their specific library — rather than using alzlibtool to cache everything. @@ -937,14 +938,18 @@ func (az *AlzLib) getBuiltInPolicies(ctx context.Context, reqs []BuiltInRequest) } // Fall back to Azure. - if az.clients.policyClient == nil { + az.mu.RLock() + policyClient := az.clients.policyClient + az.mu.RUnlock() + + if policyClient == nil { return errors.New("Alzlib.getBuiltInPolicies: policy client not set") } // Lazily initialise clients on first Azure call. if client == nil { - versionedClient = az.clients.policyClient.NewDefinitionVersionsClient() - client = az.clients.policyClient.NewDefinitionsClient() + versionedClient = policyClient.NewDefinitionVersionsClient() + client = policyClient.NewDefinitionsClient() } var err error @@ -995,14 +1000,18 @@ func (az *AlzLib) getBuiltInPolicySets(ctx context.Context, reqs []BuiltInReques } // Fall back to Azure. - if az.clients.policyClient == nil { + az.mu.RLock() + policyClient := az.clients.policyClient + az.mu.RUnlock() + + if policyClient == nil { return errors.New("Alzlib.getBuiltInPolicySets: policy client not set") } // Lazily initialise clients on first Azure call. if client == nil { - versionedClient = az.clients.policyClient.NewSetDefinitionVersionsClient() - client = az.clients.policyClient.NewSetDefinitionsClient() + versionedClient = policyClient.NewSetDefinitionVersionsClient() + client = policyClient.NewSetDefinitionsClient() } var ( @@ -1308,20 +1317,24 @@ func (az *AlzLib) ensureReferencedPolicyDefinition( } // Fall back to Azure. - if az.clients.policyClient == nil { + az.mu.RLock() + policyClient := az.clients.policyClient + az.mu.RUnlock() + + if policyClient == nil { return errors.New("Alzlib.ensureReferencedPolicyDefinition: policy client not set") } if ref.DefinitionVersion != nil { if *definitionsVersionedClient == nil { - *definitionsVersionedClient = az.clients.policyClient.NewDefinitionVersionsClient() + *definitionsVersionedClient = policyClient.NewDefinitionVersionsClient() } return az.fetchReferencedPolicyDefinitionVersions(ctx, *definitionsVersionedClient, resID.Name, setDefinition, ref) } if *definitionsClient == nil { - *definitionsClient = az.clients.policyClient.NewDefinitionsClient() + *definitionsClient = policyClient.NewDefinitionsClient() } return az.fetchLatestReferencedPolicyDefinition( From 495c0614400abca0f4ecb59fcd5f760970847cb5 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Fri, 13 Mar 2026 16:54:43 +0000 Subject: [PATCH 07/12] chore: nolint --- alzlib_test.go | 2 +- cmd/alzlibtool/command/cache/create.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/alzlib_test.go b/alzlib_test.go index e1c565a..ce52028 100644 --- a/alzlib_test.go +++ b/alzlib_test.go @@ -1951,7 +1951,7 @@ func testPolicyDefinition(t *testing.T, name, version string) *assets.PolicyDefi } } -func testPolicySetDefinition(t *testing.T, name, version string) *assets.PolicySetDefinition { +func testPolicySetDefinition(t *testing.T, name, version string) *assets.PolicySetDefinition { //nolint:unparam t.Helper() desc := name + " description" diff --git a/cmd/alzlibtool/command/cache/create.go b/cmd/alzlibtool/command/cache/create.go index d3b944a..81529c1 100644 --- a/cmd/alzlibtool/command/cache/create.go +++ b/cmd/alzlibtool/command/cache/create.go @@ -61,6 +61,7 @@ The same file may be used for both --from-cache and --output to update a cache i // Read the seed cache BEFORE opening the output file, because --from-cache // and --output may point to the same path. var seedCache *cache.Cache + if fromCacheFile != "" { f, err := os.Open(fromCacheFile) if err != nil { From dc10559b03f37b39297cea0af3917d31ae20536a Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Mon, 23 Mar 2026 12:38:40 +0000 Subject: [PATCH 08/12] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- alzlib.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/alzlib.go b/alzlib.go index 6737e34..a87d91b 100644 --- a/alzlib.go +++ b/alzlib.go @@ -718,13 +718,14 @@ func (az *AlzLib) Init(ctx context.Context, libs ...LibraryReference) error { } // GetDefinitionsFromAzure takes a slice of requests for built-in definitions. -// It then fetches them from Azure if they don't already exist (determined by last segment of -// resource id). For set definitions we need to get all of them, even if they exist in AlzLib -// already because they can contain built-in definitions. +// It fetches requested definitions from Azure only when they do not already exist in AlzLib +// (determined by the last segment of the resource ID). For policy set definitions, existing +// sets in AlzLib are inspected to discover referenced built-in definitions, and any missing +// referenced definitions are then fetched as needed. // If a cache has been set via [AlzLib.AddCache], definitions are looked up from the cache // before falling back to Azure API calls. -// The cache is retained for the lifetime of AlzLib; callers can explicitly clear it -// by calling AddCache(nil) when they no longer need it. +// The cache is retained for the lifetime of AlzLib; callers can explicitly clear it by calling +// AddCache(nil) when they no longer need it. func (az *AlzLib) GetDefinitionsFromAzure(ctx context.Context, reqs []BuiltInRequest) error { policyDefsToGet := make([]BuiltInRequest, 0, len(reqs)) policySetDefsToGet := make([]BuiltInRequest, 0, len(reqs)) From bc35c88639f08655909601bd4909a35c3fee71b8 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Mon, 23 Mar 2026 12:39:01 +0000 Subject: [PATCH 09/12] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- docs/cache.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/cache.md b/docs/cache.md index 3e36e4f..4b8061c 100644 --- a/docs/cache.md +++ b/docs/cache.md @@ -12,9 +12,9 @@ Benchmarks run on Apple M1 Pro comparing the two initialization paths: | Memory allocated | 1,406 MB | 716 MB | Cache uses ~2x more | | Allocations | 11.2M | 7.1M | Cache uses ~1.6x more | -The cache path trades higher memory usage for a ~96x reduction in wall-clock time. The additional memory is transient — definitions not referenced by the library are eligible for garbage collection after initialization. +The cache path trades higher memory usage for a ~96x reduction in wall-clock time. The cache data and any loaded definitions are retained in memory for as long as the cache is registered with AlzLib, so the additional memory is not automatically reclaimed unless the cache is released. -The cache is loaded lazily: only definitions that are actually referenced by the library's policy assignments are fetched from the cache (or from Azure if missing). The cache reference is retained for the lifetime of AlzLib — call `az.AddCache(nil)` to release it explicitly when no further lookups are needed. +The cache is loaded lazily: only definitions that are actually referenced by the library's policy assignments are fetched from the cache (or from Azure if missing). The cache reference is retained for the lifetime of AlzLib — call `az.AddCache(nil)` (and drop any other references to the cache) to allow the garbage collector to reclaim its memory when no further lookups are needed. ## Creating a Cache File From 159156c2c48f3329440de4ef6ffde15db2049241 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Mon, 23 Mar 2026 12:39:49 +0000 Subject: [PATCH 10/12] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- alzlib.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/alzlib.go b/alzlib.go index a87d91b..e502a63 100644 --- a/alzlib.go +++ b/alzlib.go @@ -866,50 +866,58 @@ type policySetDefinitionVersionsPager interface { // policyDefinitionFromCache returns the policy definition for the given name and version // from the cache, or nil if the cache is unset or does not contain a matching entry. -func (az *AlzLib) policyDefinitionFromCache(name string, version *string) *assets.PolicyDefinition { +func (az *AlzLib) policyDefinitionFromCache(name string, version *string) (*assets.PolicyDefinition, error) { az.mu.RLock() c := az.cache az.mu.RUnlock() if c == nil { - return nil + return nil, nil } pdvs := c.PolicyDefinitionVersionsByName(name) if pdvs == nil { - return nil + return nil, nil } pd, err := pdvs.GetVersion(version) if err != nil { - return nil + if errors.Is(err, assets.ErrNoVersionFound) { + return nil, nil + } + + return nil, fmt.Errorf("getting policy definition %q (version %v) from cache: %w", name, version, err) } - return pd + return pd, nil } // policySetDefinitionFromCache returns the policy set definition for the given name and version // from the cache, or nil if the cache is unset or does not contain a matching entry. -func (az *AlzLib) policySetDefinitionFromCache(name string, version *string) *assets.PolicySetDefinition { +func (az *AlzLib) policySetDefinitionFromCache(name string, version *string) (*assets.PolicySetDefinition, error) { az.mu.RLock() c := az.cache az.mu.RUnlock() if c == nil { - return nil + return nil, nil } psdvs := c.PolicySetDefinitionVersionsByName(name) if psdvs == nil { - return nil + return nil, nil } psd, err := psdvs.GetVersion(version) if err != nil { - return nil + if errors.Is(err, assets.ErrNoVersionFound) { + return nil, nil + } + + return nil, fmt.Errorf("getting policy set definition %q (version %v) from cache: %w", name, version, err) } - return psd + return psd, nil } // getBuiltInPolicies retrieves the built-in policy definitions with the given names From e9a09a8cfb2bf4e15e51c5bb782bb63845cf1682 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Mon, 23 Mar 2026 12:56:30 +0000 Subject: [PATCH 11/12] fix: handle cache lookup errors, remove rootmg/location from cache create, restrict --from-cache to architecture-scoped mode - alzlib.go: properly handle 2 return values from policyDefinitionFromCache and policySetDefinitionFromCache (fixes compiler errors) - cache create: remove --rootmg and --location flags, use internal defaults - cache create: disallow --from-cache without --library/--architecture since full-scan bulk APIs fetch everything regardless and a seed cannot reduce work - cache_test.go: fix TestCacheInitWithLibrary to use correct architecture name "simple" (matching simple-existingmg testdata) and adjust assertions --- alzlib.go | 37 +++++++++++++++++++++++--- cmd/alzlibtool/command/cache/create.go | 37 ++++++++++++++------------ integrationtest/cache_test.go | 14 +++++----- 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/alzlib.go b/alzlib.go index e502a63..a050441 100644 --- a/alzlib.go +++ b/alzlib.go @@ -934,7 +934,16 @@ func (az *AlzLib) getBuiltInPolicies(ctx context.Context, reqs []BuiltInRequest) } // Check cache before calling Azure. - if pdv := az.policyDefinitionFromCache(req.ResourceID.Name, req.Version); pdv != nil { + pdv, cacheErr := az.policyDefinitionFromCache(req.ResourceID.Name, req.Version) + if cacheErr != nil { + return fmt.Errorf( + "Alzlib.getBuiltInPolicies: looking up cached built-in policy definition %s: %w", + JoinNameAndVersion(req.ResourceID.Name, req.Version), + cacheErr, + ) + } + + if pdv != nil { if err := az.AddPolicyDefinitions(pdv); err != nil { return fmt.Errorf( "Alzlib.getBuiltInPolicies: adding cached built-in policy definition %s: %w", @@ -994,7 +1003,16 @@ func (az *AlzLib) getBuiltInPolicySets(ctx context.Context, reqs []BuiltInReques } // Check cache before calling Azure. - if psdv := az.policySetDefinitionFromCache(req.ResourceID.Name, req.Version); psdv != nil { + psdv, cacheErr := az.policySetDefinitionFromCache(req.ResourceID.Name, req.Version) + if cacheErr != nil { + return fmt.Errorf( + "Alzlib.getBuiltInPolicySets: looking up cached built-in policy set definition %s: %w", + req.String(), + cacheErr, + ) + } + + if psdv != nil { if err := az.AddPolicySetDefinitions(psdv); err != nil { return fmt.Errorf( "Alzlib.getBuiltInPolicySets: adding cached built-in policy set definition %s: %w", @@ -1255,7 +1273,7 @@ func (az *AlzLib) ensureReferencedPolicyDefinitions( // Lazily initialised Azure clients, shared across all references within this invocation. var ( - definitionsClient *armpolicy.DefinitionsClient + definitionsClient *armpolicy.DefinitionsClient definitionsVersionedClient *armpolicy.DefinitionVersionsClient ) @@ -1311,7 +1329,18 @@ func (az *AlzLib) ensureReferencedPolicyDefinition( } // Check cache before calling Azure. - if pdv := az.policyDefinitionFromCache(resID.Name, ref.DefinitionVersion); pdv != nil { + pdv, cacheErr := az.policyDefinitionFromCache(resID.Name, ref.DefinitionVersion) + if cacheErr != nil { + return fmt.Errorf( + "Alzlib.ensureReferencedPolicyDefinition: looking up cached built-in policy definition `%s` "+ + "referenced in policy set `%s`: %w", + JoinNameAndVersion(resID.Name, ref.DefinitionVersion), + JoinNameAndVersion(*setDefinition.Name, setDefinition.Properties.Version), + cacheErr, + ) + } + + if pdv != nil { if err := az.AddPolicyDefinitions(pdv); err != nil { return fmt.Errorf( "Alzlib.ensureReferencedPolicyDefinition: adding cached built-in policy definition `%s` "+ diff --git a/cmd/alzlibtool/command/cache/create.go b/cmd/alzlibtool/command/cache/create.go index 81529c1..07700f4 100644 --- a/cmd/alzlibtool/command/cache/create.go +++ b/cmd/alzlibtool/command/cache/create.go @@ -36,9 +36,10 @@ When --library and --architecture are specified, only the definitions referenced architecture are included, producing a smaller, use-case-specific cache. This is useful for embedding the minimal set of definitions needed for a given deployment workflow. -Use --from-cache to seed from an existing cache file. Definitions already present in the seed -cache are used directly and not re-fetched from Azure. This allows efficient incremental updates. -The same file may be used for both --from-cache and --output to update a cache in-place.`, +Use --from-cache to seed from an existing cache file (requires --library and --architecture). +Definitions already present in the seed cache are used directly and not re-fetched from Azure, +reducing the number of API calls. The same file may be used for both --from-cache and --output +to update a cache in-place.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, _ []string) { outFile, _ := cmd.Flags().GetString("output") @@ -46,8 +47,6 @@ The same file may be used for both --from-cache and --output to update a cache i libraryPath, _ := cmd.Flags().GetString("library") architectureName, _ := cmd.Flags().GetString("architecture") fromCacheFile, _ := cmd.Flags().GetString("from-cache") - rootMg, _ := cmd.Flags().GetString("rootmg") - location, _ := cmd.Flags().GetString("location") // --library and --architecture must be specified together. if (libraryPath == "") != (architectureName == "") { @@ -58,6 +57,18 @@ The same file may be used for both --from-cache and --output to update a cache i os.Exit(1) } + // --from-cache only makes sense in architecture-scoped mode where the + // lazy cache-first lookup can skip individual Azure API calls. In full-scan + // mode the bulk listing APIs fetch everything regardless, so a seed cache + // cannot reduce work. + if fromCacheFile != "" && libraryPath == "" { + cmd.PrintErrf( + "%s --from-cache requires --library and --architecture\n", + cmd.ErrPrefix(), + ) + os.Exit(1) + } + // Read the seed cache BEFORE opening the output file, because --from-cache // and --output may point to the same path. var seedCache *cache.Cache @@ -142,7 +153,7 @@ The same file may be used for both --from-cache and --output to update a cache i } h := deployment.NewHierarchy(az) - if err := h.FromArchitecture(cmd.Context(), architectureName, rootMg, location); err != nil { + if err := h.FromArchitecture(cmd.Context(), architectureName, defaultRootMgID, defaultLocation); err != nil { cmd.PrintErrf( "%s could not process architecture %q: %v\n", cmd.ErrPrefix(), architectureName, err, @@ -221,15 +232,7 @@ func init() { createCmd.Flags(). String( "from-cache", "", - "Path to an existing cache file to use as a seed. Definitions found in the seed cache "+ - "are not re-fetched from Azure. The same path may be used for both --from-cache and --output "+ - "to update a cache in-place.") - createCmd.Flags(). - StringP( - "rootmg", "r", defaultRootMgID, - "Root management group ID to use when processing the architecture (used with --library and --architecture).") - createCmd.Flags(). - StringP( - "location", "l", defaultLocation, - "Location to use when processing the architecture (used with --library and --architecture).") + "Path to an existing cache file to use as a seed (requires --library and --architecture). "+ + "Definitions found in the seed cache are not re-fetched from Azure, reducing API calls. "+ + "The same path may be used for both --from-cache and --output to update a cache in-place.") } diff --git a/integrationtest/cache_test.go b/integrationtest/cache_test.go index 9b2092c..10c00ac 100644 --- a/integrationtest/cache_test.go +++ b/integrationtest/cache_test.go @@ -88,13 +88,13 @@ func TestCacheInitWithLibrary(t *testing.T) { assert.Contains(t, az.PolicyDefinitions(), "test-policy-definition") assert.Contains(t, az.PolicySetDefinitions(), "test-policy-set-definition") - // Built-in definitions from cache are loaded lazily on demand via GetDefinitionsFromAzure - // (triggered by deployment.FromArchitecture). Only library-defined definitions are - // present at this point. + // The simple-existingmg library has a "simple" architecture whose policy set + // only references custom (library-local) definitions, so FromArchitecture + // does not trigger any built-in lookups. h := deployment.NewHierarchy(az) - require.NoError(t, h.FromArchitecture(ctx, "alz", "00000000-0000-0000-0000-000000000000", "testlocation")) + require.NoError(t, h.FromArchitecture(ctx, "simple", "00000000-0000-0000-0000-000000000000", "testlocation")) - // After building the hierarchy, referenced built-in definitions should be present. - assert.Greater(t, len(az.PolicyDefinitions()), 1) - assert.Greater(t, len(az.PolicySetDefinitions()), 1) + // Library definitions should still be present after building the hierarchy. + assert.Contains(t, az.PolicyDefinitions(), "test-policy-definition") + assert.Contains(t, az.PolicySetDefinitions(), "test-policy-set-definition") } From bd327edef2937abd6e72d442fbf7e676c23506e7 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Mon, 23 Mar 2026 13:18:12 +0000 Subject: [PATCH 12/12] fix: use JoinNameAndVersion in cache lookup error messages instead of %v on *string --- alzlib.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/alzlib.go b/alzlib.go index a050441..5ff5a23 100644 --- a/alzlib.go +++ b/alzlib.go @@ -886,7 +886,7 @@ func (az *AlzLib) policyDefinitionFromCache(name string, version *string) (*asse return nil, nil } - return nil, fmt.Errorf("getting policy definition %q (version %v) from cache: %w", name, version, err) + return nil, fmt.Errorf("getting policy definition %s from cache: %w", JoinNameAndVersion(name, version), err) } return pd, nil @@ -914,7 +914,7 @@ func (az *AlzLib) policySetDefinitionFromCache(name string, version *string) (*a return nil, nil } - return nil, fmt.Errorf("getting policy set definition %q (version %v) from cache: %w", name, version, err) + return nil, fmt.Errorf("getting policy set definition %s from cache: %w", JoinNameAndVersion(name, version), err) } return psd, nil