Questions around defaults

[JWilkinsonMB]

I wanted to switch from my own default definitions to the ones included in the library, but it prompted a few questions as the schema changed since I last looked. If I should split this into separate issues, or they belong in a different repo please let me know.

1. Version 0.13.0 of the alz provider (latest at time of writing) doesn't seem to load alz_policy_default_values.json. I presume a newer version of the provider will be needed to support this, but just wanted to confirm.

2. In the docs it's stated there can only be one policy default values file in an archetype. Is this a correct statement, or is it that there can only be one default file per library, or per instance of the client? I don't see a way of mapping an archetype to a default file within the schema, and this may not be desirable given it's common for different archetypes to deploy the same policy assignment with the same default. E.g. platform and landing_zones both deploy Deploy-VM-ChangeTrack which requires the log analytics workspace.

3. Will the existing schema for *.alz_policy_default_value.json (one per file) be retained or is it superseded by the single file?

4. If there's only a single default values file per client instance (no merging), and individual defaults are deprecated, how would we extend the library to support using our own defaults if needed? For context, my plan is to use 3 libraries per environment, this base library, a common library for our own standard policies, and a library for the environment itself.

5. Some defaults seem duplicative, e.g. ama_vm_change_tracking_data_collection_rule_id, ama_vmarc_change_tracking_data_collection_rule_id & ama_vmss_change_tracking_data_collection_rule_id all require the same change tracking DCR, but they are separate defaults. Can these be combined into a single default applied to all 3 polices?

6. Some defaults are missing, e.g. the DCR parameters for Deploy-VM-Monitoring, Deploy-VMSS-Monitoring, Deploy-vmHybr-Monitoring & Deploy-MDFC-DefenSQL-AMA. Are there plans to add them? As far as I could tell the defaults seem to be defined here, or do they come from upstream?

Thanks!

[matt-FFFFFF]

Hi @JWilkinsonMB

Thanks for your questions! We decided to consolidate the policy default values into a single file. This will be updated in a new provider version, the code is already in the alzlib go module.

Answers:

1. See above
2. There can be only one policy default values file in a library reference. E.g. for any reference you pass into the provider. The file must be names alz_policy_default_values.[json|yaml|yml].
3. Schema is published here: https://github.com/Azure/Azure-Landing-Zones-Library/blob/main/schemas/default_policy_values.json
4. You can have a defaults file for each of those libraries. Let me know how you get on once we released the new version and if it doesn't work for you we can discuss.
5. These should all be different DCR ids, they are included in the alz management module.
6. This seems like an oversight - let me have a look

[JWilkinsonMB]

Thanks as always @matt-FFFFFF! A few follow-ups:

2. Makes sense, thanks. Can the documentation be updated to reflect that it's per-library rather than per-archetype? https://github.com/Azure/Azure-Landing-Zones-Library/blob/24ce686edddbaf8bc4461f73536fa803a02245d4/docs/content/assets/policy-default-values.md?plain=1#L15
   

3. Sorry, my question more about whether both schemas will be supported going forward? Under schemas there are both default_policy_value.json & default_policy_values.json

‎

5. In the ALZ management module there are 3 DCRs, but only one of them is for change tracking. https://github.com/Azure/terraform-azurerm-avm-ptn-alz-management?tab=readme-ov-file#-data_collection_rules
   The other 2 are the missing monitoring and Defender SQL DCRs. Whilst it's easy enough to take the single change tracking DCR and wire it to 3 separate defaults, it just seems counterintuitive.
   The Azure docs for deploying this also instruct you to have a single DCR that you then reference in all 3 policies https://learn.microsoft.com/en-us/azure/automation/change-tracking/enable-vms-monitoring-agent?tabs=singlevm%2Carcvm#enable-change-tracking-at-scale-using-azure-monitoring-agent
   I would have thought that in the vast majority of cases it would be desirable to have the same DCR applied consistently to a VM, whether that's a standalone VM, in a VMSS, or through Arc, hence as a default, using a single value for all 3 policies feels more appropriate to me and is easier to understand as a consumer of the modules.

[JWilkinsonMB]

Also after testing again since the provider and module were updated, it looks like there's a mistake in the default ama_user_assigned_managed_identity_id in that it's being set on the policy assignment Deploy-vmArc-ChangeTrack, however that policy doesn't have a paramater for userAssignedIdentityResourceId because AMA uses system assigned identity for Arc. https://github.com/Azure/Azure-Landing-Zones-Library/blob/24ce686edddbaf8bc4461f73536fa803a02245d4/platform/alz/alz_policy_default_values.json#L13-L18

│ Error: architectureDataSource.Read() Error applying policy assignment default `ama_user_assigned_managed_identity_id`
│
│   with module.alz.module.alz_global.data.alz_architecture.this,
│   on .terraform\modules\alz.alz_global\main.tf line 1, in data "alz_architecture" "this":
│    1: data "alz_architecture" "this" {
│
│ Hierarchy.AddDefaultPolicyAssignmentValue: error adding default `ama_user_assigned_managed_identity_id` policy assignment value to management group `global-landingzones`    
│ for policy assignment `Deploy-vmArc-ChangeTrack`: HierarchyManagementGroup.ModifyPolicyAssignment: parameter `userAssignedIdentityResourceId` not found in referenced        
│ policySetDefinitions `53448c70-089b-4f52-8f38-89196d7f2de1` for policy assignment `Deploy-vmArc-ChangeTrack

I'd put in a pull request, but I need to get approval to sign the CLA which might take some time, sorry.

[matt-FFFFFF]

Hi there @JWilkinsonMB,

Thank you for reporting back! We appreciate you finding these issues.

Looks like we need to get that default parameter removed.