diff --git a/alzlib.go b/alzlib.go index 9532dfe..5ff5a23 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" @@ -48,6 +49,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 +544,101 @@ 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. +// 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() defer az.mu.Unlock() - for name, pdvs := range c.PolicyDefinitions() { - if _, exists := az.policyDefinitions[name]; exists { + az.cache = c +} + +// ExportBuiltInCache creates a [cache.Cache] from the built-in policy definitions and policy +// 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. +// 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 } - cpy := deep.MustCopy(pdvs) - az.policyDefinitions[name] = cpy + pds[name] = deep.MustCopy(pdvs) } - for name, psdvs := range c.PolicySetDefinitions() { - if _, exists := az.policySetDefinitions[name]; exists { + for name, psdvs := range az.policySetDefinitions { + if !isBuiltInPolicySetDefinitionVersions(psdvs) { continue } - cpy := deep.MustCopy(psdvs) - az.policySetDefinitions[name] = cpy + psds[name] = deep.MustCopy(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. @@ -652,9 +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. func (az *AlzLib) GetDefinitionsFromAzure(ctx context.Context, reqs []BuiltInRequest) error { policyDefsToGet := make([]BuiltInRequest, 0, len(reqs)) policySetDefsToGet := make([]BuiltInRequest, 0, len(reqs)) @@ -793,21 +864,112 @@ 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, error) { + az.mu.RLock() + c := az.cache + az.mu.RUnlock() + + if c == nil { + return nil, nil + } + + pdvs := c.PolicyDefinitionVersionsByName(name) + if pdvs == nil { + return nil, nil + } + + pd, err := pdvs.GetVersion(version) + if err != nil { + if errors.Is(err, assets.ErrNoVersionFound) { + return nil, nil + } + + return nil, fmt.Errorf("getting policy definition %s from cache: %w", JoinNameAndVersion(name, version), err) + } + + 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, error) { + az.mu.RLock() + c := az.cache + az.mu.RUnlock() + + if c == nil { + return nil, nil + } + + psdvs := c.PolicySetDefinitionVersionsByName(name) + if psdvs == nil { + return nil, nil + } + + psd, err := psdvs.GetVersion(version) + if err != nil { + if errors.Is(err, assets.ErrNoVersionFound) { + return nil, nil + } + + return nil, fmt.Errorf("getting policy set definition %s from cache: %w", JoinNameAndVersion(name, version), err) + } + + return psd, nil +} + // 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. + 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", + JoinNameAndVersion(req.ResourceID.Name, req.Version), + err, + ) + } + + continue + } + + // Fall back to Azure. + 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 = policyClient.NewDefinitionVersionsClient() + client = policyClient.NewDefinitionsClient() + } + var err error switch req.Version { @@ -828,20 +990,57 @@ 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. + 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", + req.String(), + err, + ) + } + + processedRequests = append(processedRequests, psdv) + + continue + } + + // Fall back to Azure. + 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 = policyClient.NewSetDefinitionVersionsClient() + client = policyClient.NewSetDefinitionsClient() + } + var ( psd *assets.PolicySetDefinition err error @@ -1072,17 +1271,20 @@ func (az *AlzLib) ensureReferencedPolicyDefinitions( return nil } - definitionsVersionedClient := az.clients.policyClient.NewDefinitionVersionsClient() - definitionsClient := az.clients.policyClient.NewDefinitionsClient() + // 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, - definitionsClient, - definitionsVersionedClient, def, ref, + &definitionsClient, + &definitionsVersionedClient, ) if err != nil { return err @@ -1095,10 +1297,10 @@ func (az *AlzLib) ensureReferencedPolicyDefinitions( func (az *AlzLib) ensureReferencedPolicyDefinition( ctx context.Context, - definitionsClient *armpolicy.DefinitionsClient, - definitionsVersionedClient *armpolicy.DefinitionVersionsClient, setDefinition *assets.PolicySetDefinition, ref *armpolicy.DefinitionReference, + definitionsClient **armpolicy.DefinitionsClient, + definitionsVersionedClient **armpolicy.DefinitionVersionsClient, ) error { if ref == nil { return nil @@ -1121,12 +1323,60 @@ 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. + 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` "+ + "referenced in policy set `%s`: %w", + JoinNameAndVersion(resID.Name, ref.DefinitionVersion), + JoinNameAndVersion(*setDefinition.Name, setDefinition.Properties.Version), + err, + ) + } + + return nil + } + + // Fall back to Azure. + 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 { - return az.fetchReferencedPolicyDefinitionVersions(ctx, definitionsVersionedClient, resID.Name, setDefinition, ref) + if *definitionsVersionedClient == nil { + *definitionsVersionedClient = policyClient.NewDefinitionVersionsClient() + } + + return az.fetchReferencedPolicyDefinitionVersions(ctx, *definitionsVersionedClient, resID.Name, setDefinition, ref) + } + + if *definitionsClient == nil { + *definitionsClient = 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 3d62f6d..ce52028 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" @@ -1160,7 +1162,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 +1191,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 +1249,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 +1279,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") @@ -1890,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" @@ -1910,3 +1971,191 @@ 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), + }, + }, + } +} + +// 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() + + 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()) +} + +// 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/cache/cache.go b/cache/cache.go index 63c5223..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() { @@ -282,3 +307,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/cmd/alzlibtool/command/cache/create.go b/cmd/alzlibtool/command/cache/create.go index ea6c002..07700f4 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,64 +17,201 @@ 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 (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") verbose, _ := cmd.Flags().GetBool("verbose") + libraryPath, _ := cmd.Flags().GetString("library") + architectureName, _ := cmd.Flags().GetString("architecture") + fromCacheFile, _ := cmd.Flags().GetString("from-cache") - 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) + // --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 + + 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 if verbose { 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, defaultRootMgID, defaultLocation); 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 +220,19 @@ 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 (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/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.") } diff --git a/docs/cache.md b/docs/cache.md index 2d1978f..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 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). 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 @@ -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 is kept until explicitly released — call az.AddCache(nil) +// to allow 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..10c00ac 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. - assert.Greater(t, len(az.PolicyDefinitions()), 1) - assert.Greater(t, len(az.PolicySetDefinitions()), 1) + // 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, "simple", "00000000-0000-0000-0000-000000000000", "testlocation")) + + // 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") }