AVM Review

[jchancellor-ms]

Dear module owner,

As per the module ownership requirements and responsibilities at the time of [assignment](REPLACE WITH THE LINK TO THE AVM MODULE PROPOSAL), the AVM Team is opening this issue, requesting you to validate your module against the below AVM specifications and confirm its compliance.

Please don't close this issue and merge your AVM-Review-PR until advised to do so. This review is a prerequisite for publishing your module's v0.1.0 in the Terraform Registry. The AVM team is happy to assist with any questions you might have.

Requested Actions

1. Complete the below task list by ticking off the tasks.
2. Complete the below table by updating the Compliant column with Yes, No or NA as possible values.

Please use the comments columns to provide additional details especially if the Compliant column is updated to No or NA.

### Tasks
- [ ] Address comments on AVM-Review-PR if any
- [ ] Ensure that all checks on AVM-Review-PR are passing
- [ ] Tick this to acknowledge specs with comment "Module Owner to action this spec post-publish as appropriate" in the table below.
- [ ] Ensure that the latest 'chore: repository governance' PR is merged into the main branch and the branch/fork of your AVM-Review-PR is also updated with it.

ID	Spec	Compliant	Comments
1	ID: SFR1 - Category: Composition - Preview Services	Yes	NA if no preview services are used
2	ID: SFR2 - Category: Composition - WAF Aligned	NA	Ensure only high priority reliability & security recommendations are implemented if any
3	ID: SFR3 - Category: Telemetry - Deployment/Usage Telemetry	Yes
4	ID: SFR4 - Category: Telemetry - Telemetry Enablement Flexibility	Yes	Yes if AVM Template Repo has been used
5	ID: SFR5 - Category: Composition - Availability Zones	NA
6	ID: SFR6 - Category: Composition - Data Redundancy	NA
7	ID: SNFR25 - Category: Composition - Resource Naming	Yes
8	ID: SNFR1 - Category: Testing - Prescribed Tests	Yes	Yes if all e2e test, version-check & linting checks passed
9	ID: SNFR2 - Category: Testing - E2E Testing	Yes	Yes if e2e tests passed
10	ID: SNFR3 - Category: Testing - AVM Compliance Tests	Yes	Yes if all e2e test, version-check & linting checks passed
11	ID: SNFR4 - Category: Testing - Unit Tests	NA	NA if no tests created in tests folder
12	ID: SNFR5 - Category: Testing - Upgrade Tests	NA	Module Owner to action this spec post-publish as appropriate
13	ID: SNFR6 - Category: Testing - Static Analysis/Linting Tests	Yes	Yes if all linting checks passed
14	ID: SNFR7 - Category: Testing - Idempotency Tests	Yes	Yes if e2e tests passed
15	ID: SNFR24 - Category: Testing - Testing Child, Extension & Interface Resources	Yes	Yes if e2e tests passed
16	ID: SNFR8 - Category: Contribution/Support - Module Owner(s) GitHub	Yes
17	ID: SNFR20 - Category: Contribution/Support - GitHub Teams Only	yes
18	ID: SNFR9 - Category: Contribution/Support - AVM & PG Teams GitHub Repo Permissions	Yes
19	ID: SNFR10 - Category: Contribution/Support - MIT Licensing	Yes	Yes if AVM Template Repo has been used
20	ID: SNFR11 - Category: Contribution/Support - Issues Response Times	NA	Module Owner to action this spec post-publish as appropriate
21	ID: SNFR12 - Category: Contribution/Support - Versions Supported	NA	Module Owner to action this spec post-publish as appropriate
22	ID: SNFR23 - Category: Contribution/Support - GitHub Repo Labels	Yes
23	ID: SNFR14 - Category: Inputs - Data Types	Yes
24	ID: SNFR22 - Category: Inputs - Parameters/Variables for Resource IDs	No
25	ID: SNFR15 - Category: Documentation - Automatic Documentation Generation	Yes	Yes if linting / docs check passed
26	ID: SNFR16 - Category: Documentation - Examples/E2E	Yes	Yes if e2e tests passed
27	ID: SNFR17 - Category: Release - Semantic Versioning	Yes	Yes if version-check check passed
28	ID: SNFR18 - Category: Release - Breaking Changes	NA	Module Owner to action this spec post-publish as appropriate
29	ID: SNFR19 - Category: Publishing - Registries Targeted	NA	Module Owner to action this spec post-publish as appropriate
30	ID: SNFR21 - Category: Publishing - Cross Language Collaboration	NA	Module Owner to action this spec post-publish as appropriate
31	ID: RMFR1 - Category: Composition - Single Resource Only	Yes
32	ID: RMFR2 - Category: Composition - No Resource Wrapper Modules	Yes
33	ID: RMFR3 - Category: Composition - Resource Groups	Yes
34	ID: RMFR4 - Category: Composition - AVM Consistent Feature & Extension Resources Value Add	Yes	Yes if linting / terraform check passed
35	ID: RMFR5 - Category: Composition - AVM Consistent Feature & Extension Resources Value Add Interfaces/Schemas	Yes	Yes if linting / terraform check passed
36	ID: RMFR8 - Category: Composition - Dependency on child and other resources	Yes
37	ID: RMFR6 - Category: Inputs - Parameter/Variable Naming	Yes
38	ID: RMFR7 - Category: Outputs - Minimum Required Outputs	Yes	Yes if linting / terraform check passed
39	ID: RMNFR1 - Category: Naming - Module Naming	Yes
40	ID: RMNFR2 - Category: Inputs - Parameter/Variable Naming	Yes
41	ID: RMNFR3 - Category: Composition - RP Collaboration	NA	Module Owner to action this spec post-publish as appropriate
42	ID: PMFR1 - Category: Composition - Resource Group Creation	NA	NA if this is not a pattern module
43	ID: PMNFR1 - Category: Naming - Module Naming	NA	NA if this is not a pattern module
44	ID: PMNFR2 - Category: Composition - Use Resource Modules to Build a Pattern Module	NA	NA if this is not a pattern module
45	ID: PMNFR3 - Category: Composition - Use other Pattern Modules to Build a Pattern Module	NA	NA if this is not a pattern module
46	ID: PMNFR4 - Category: Hygiene - Missing Resource Module(s)	NA	NA if this is not a pattern module
47	ID: PMNFR5 - Category: Inputs - Parameter/Variable Naming	NA	NA if this is not a pattern module
48	ID: TFFR1 - Category: Composition - Cross-Referencing Modules	Yes
49	ID: TFFR2 - Category: Outputs - Additional Terraform Outputs	Yes	Yes if linting / terraform check passed
50	ID: TFNFR1 - Category: Documentation - Descriptions	No
51	ID: TFNFR2 - Category: Documentation - Module Documentation Generation	Yes	Yes if linting / docs check passed
52	ID: TFNFR3 - Category: Contribution/Support - GitHub Repo Branch Protection	Yes	Yes if AVM Template Repo has been used
53	ID: TFNFR4 - Category: Composition - Code Styling - lower snake_casing	Yes	Yes if linting / terraform check passed
54	ID: TFNFR5 - Category: Testing - Test Tooling	Yes	Yes if linting / terraform check passed
55	ID: TFNFR6 - Category: Code Style - Resource & Data Order	Yes
56	ID: TFNFR7 - Category: Code Style - count & for_each Use	Yes
57	ID: TFNFR8 - Category: Code Style - Resource & Data Block Orders	Yes	Yes if linting / avmfix check passed
58	ID: TFNFR9 - Category: Code Style - Module Block Order	Yes
59	ID: TFNFR10 - Category: Code Style - No Double Quotes in ignore_changes	Yes
60	ID: TFNFR11 - Category: Code Style - Null Comparison Toggle	Yes
61	ID: TFNFR12 - Category: Code Style - Dynamic for Optional Nested Objects	Yes
62	ID: TFNFR13 - Category: Code Style - Default Values with coalesce/try	Yes
63	ID: TFNFR14 - Category: Inputs - Not allowed variables	Yes
64	ID: TFNFR15 - Category: Code Style - Variable Definition Order	Yes	Yes if linting / avmfix check passed
65	ID: TFNFR16 - Category: Code Style - Variable Naming Rules	Yes	Yes if linting / terraform check passed
66	ID: TFNFR17 - Category: Code Style - Variables with Descriptions	Yes	Yes if linting / terraform check passed
67	ID: TFNFR18 - Category: Code Style - Variables with Types	Yes	Yes if linting / terraform check passed
68	ID: TFNFR19 - Category: Code Style - Sensitive Data Variables	Yes
69	ID: TFNFR20 - Category: Code Style - Non-Nullable Defaults for collection values	No
70	ID: TFNFR21 - Category: Code Style - Discourage Nullability by Default	Yes	Yes if linting / avmfix check passed
71	ID: TFNFR22 - Category: Code Style - Avoid sensitive = false	Yes	Yes if linting / avmfix check passed
72	ID: TFNFR23 - Category: Code Style - Sensitive Default Value Conditions	Yes	Yes if linting / terraform check passed
73	ID: TFNFR24 - Category: Code Style - Handling Deprecated Variables	NA	Module Owner to action this spec post-publish as appropriate
74	ID: TFNFR25 - Category: Code Style - Verified Modules Requirements	Yes	Yes if linting / terraform check passed
75	ID: TFNFR26 - Category: Code Style - Providers in required_providers	Yes	Yes if linting / terraform check passed
76	ID: TFNFR27 - Category: Code Style - Provider Declarations in Modules	Yes	Yes if linting / terraform check passed
77	ID: TFNFR28 - Category: Code Style - Provider Declarations in Modules	Yes	Yes if linting / terraform check passed
78	ID: TFNFR29 - Category: Code Style - Sensitive Data Outputs	Yes	Yes if linting / avmfix check passed
79	ID: TFNFR30 - Category: Code Style - Handling Deprecated Outputs	NA	Module Owner to action this spec post-publish as appropriate
80	ID: TFNFR31 - Category: Code Style - locals.tf for Locals Only	NA
81	ID: TFNFR32 - Category: Code Style - Alphabetical Local Arrangement	Yes	Yes if linting / avmfix check passed
82	ID: TFNFR33 - Category: Code Style - Precise Local Types	Yes
83	ID: TFNFR34 - Category: Code Style - Using Feature Toggles	NA	Module Owner to action this spec post-publish as appropriate
84	ID: TFNFR35 - Category: Code Style - Reviewing Potential Breaking Changes	NA	Module Owner to action this spec post-publish as appropriate
85	ID: TFNFR36 - Category: Code Style - Setting prevent_deletion_if_contains_resources	No
86	ID: TFNFR37 - Category: Code Style - Tool Usage by Module Owner	Yes

[jchancellor-ms]

@sihbher - I've completed the initial AVM review of this module. Please make the following changes to be spec compliant and add a comment when complete and I can approve so you can publish.

Update the rule_id name to rule_resource_id (and update the corresponding code) per SNFR22 Add an example to the description for the customrules variable per spec [TFNFR1](https://azure.github.io/Azure-Verified-Modules/specs/terraform/#id-tfnfr1---category-documentation---descriptions) Add nullable = false to the variable custom_rules per TFNFR20 Add nullable = false to the variable managed_rules per TFNFR20 For each of the examples set prevent_deletion_if_contains_resources to false in the azurerm provider block per TFNFR36

[sihbher]

Hi @jchancellor-ms thanks for your feedback, I attended all of them, except the first one since this is not referring to a resource id, see the next example

Example of managed_rules

TF snippet, review waf_with_custom_managed_rules example ```hcl managed_rules = [ #Managed Rule 1 - Microsoft_DefaultRuleSet { action = "Block" type = "Microsoft_DefaultRuleSet" version = "2.1" #Example of exclusion exclusions = [ { match_variable = "QueryStringArgNames" operator = "Equals" selector = "not_suspicious" } ] #Example of override overrides = [{ rule_group_name = "PHP" rules = [{ rule_id = "933100" enabled = false action = "AnomalyScoring" }, { rule_id = "933110" enabled = true action = "AnomalyScoring" }] }, { rule_group_name = "SQLI" rules = [{ rule_id = "942100" enabled = false action = "AnomalyScoring" }, { rule_id = "942200" action = "AnomalyScoring" exclusions = [{ match_variable = "QueryStringArgNames" operator = "Equals" selector = "innocent" }] }] exclusions = [{ match_variable = "QueryStringArgNames" operator = "Equals" selector = "really_not_suspicious" }] } ] }, #Managed Rule 2 - Microsoft_BotManagerRuleSet { action = "Block" type = "Microsoft_BotManagerRuleSet" version = "1.1" } ] ```

• [ ] Update the rule_id name to rule_resource_id (and update the corresponding code) per SNFR22
• [x] Add an example to the description for the customrules variable per spec [TFNFR1](https://azure.github.io/Azure-Verified-Modules/specs/terraform/#id-tfnfr1---category-documentation---descriptions)
• [x] Add nullable = false to the variable custom_rules per TFNFR20
• [x] Add nullable = false to the variable managed_rules per TFNFR20
• [x] For each of the examples set prevent_deletion_if_contains_resources to false in the azurerm provider block per TFNFR36