Virtual Network - Subnet Child Resource Remediation

[ahmadabdalla]

Change

This PR is intended to close #1111 where the virtual network module fails when the vnet contains existing azure workloads.

Solution:

• Port all subnet properties into the virtual network - subnets array
• remove the custom features (NetworkSecurityGroupResourceGroupName)
• Rename 'Name' to 'Id' for (NetworkSecurityGroup) (RouteTables) (NatGateways)
• Remove the Service Endpoints formatting and variable and align to template reference documentation.
• Added note for the current design for VNET

Notes:

• Porting the features from the subnet child module into the array in the vnet resource

Testing

Dependency Pipeline (Fork)

Type of Change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I'm sure there are no other open Pull Requests for the same update/change
• My corresponding pipelines / checks run clean and green without any errors or warnings
• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (readme)
• I did format my code

[AlexanderSehr]

Quote from issue: Hey @ahmadabdalla, we should definitely not roll back 1:1 as I removed it one purpose due to an issue that was caused by keeping it (most notably e.g. that the original implementation would clash with ALZ policies). I'd suggest we either find a way to reference 'existing' subnets and pass them in (e.g. with an existing reference if that doesn't throw an exception if empty), or we must duplicate the child resource's property (which would be the worst case scenario).

[ahmadabdalla]

Totally agree with not rolling back. Hence it's a draft to get the module working so we can iterate. Pending discussion

[AlexanderSehr]

After reading up on it and experiencing the same behavior with Security Rules for NSG we should probably afterall duplicate all properties. I makes sense what the PG says to 'why' it is implemented in the way it is (i.e. enable removal of entries) - it's just very bad news for us & unfortunately not exactly a consistent practice across Azure resources.

[ahmadabdalla]

Route tables from my experience has a similar problem for the routes property. If they're set outside of the route table object, they'll get cleared up first. This caused an issue for a customer who had an AKS resource that manages its own routes, and when the pipeline ran, ARM deleted the AKS routes and only created the ones in code. AKS does recover its routes but that's a service disruption. So it's not as "incremental" as we want on these resources

[github-actions]

Unit Test Results

    1 files  ±  0  1 suites  ±0   43s ⏱️ +25s
    8 tests +  2  8 ✔️ +2    0 💤 ±  0  0 ❌ ±0 
104 runs  +66  8 ✔️ +2  96 💤 +64  0 ❌ ±0 

Results for commit 9d2c435. ± Comparison against base commit 0d34485.

This pull request removes 6 and adds 8 tests. Note that renamed tests count towards both.

/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.CognitiveServices/accounts] used resource type [accounts] should use on of the recent API version(s). Currently using [2017-04-18]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.CognitiveServices/accounts] used resource type [diagnosticsettings] should use on of the recent API version(s). Currently using [2021-05-01-preview]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.CognitiveServices/accounts] used resource type [locks] should use on of the recent API version(s). Currently using [2017-04-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.CognitiveServices/accounts] used resource type [privateEndpoints/privateDnsZoneGroups] should use on of the recent API version(s). Currently using [2021-02-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.CognitiveServices/accounts] used resource type [privateEndpoints] should use on of the recent API version(s). Currently using [2021-05-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.CognitiveServices/accounts] used resource type [roleassignments] should use on of the recent API version(s). Currently using [2021-04-01-preview]

/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks/subnets] used resource type [virtualNetworks/subnets] should use on of the recent API version(s). Currently using [2021-05-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks/virtualNetworkPeerings] used resource type [virtualNetworks/virtualNetworkPeerings] should use on of the recent API version(s). Currently using [2021-05-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks] used resource type [diagnosticsettings] should use on of the recent API version(s). Currently using [2021-05-01-preview]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks] used resource type [locks] should use on of the recent API version(s). Currently using [2017-04-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks] used resource type [roleassignments] should use on of the recent API version(s). Currently using [2021-04-01-preview]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks] used resource type [virtualNetworks/subnets] should use on of the recent API version(s). Currently using [2021-05-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks] used resource type [virtualNetworks/virtualNetworkPeerings] should use on of the recent API version(s). Currently using [2021-05-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Network/virtualNetworks] used resource type [virtualNetworks] should use on of the recent API version(s). Currently using [2021-05-01]

♻️ This comment has been updated with latest results.