ipam modules and templates by tulpy · Pull Request #1 · tulpy/bicep

tulpy · 2026-01-26T13:14:27Z

Pull Request Template

Description

Add IPAM templates for blog post

Type of Change

Please select the type of change this pull request represents:

🪲 Bug Fix
🤩 Improvement to the IP
📝 Adding a feature

Checklist

Please review and check the following items before submitting this pull request:

Code has been tested locally
Documentation has been updated (if applicable)
All tests pass
No new linting errors
Follows the coding style guidelines

Related Issues

Please list any related issues or pull requests that are addressed by this pull request.

Additional Notes

Please provide any additional notes or context regarding this pull request.

Copilot

Pull request overview

This pull request adds IPAM (IP Address Management) modules and templates for Azure Virtual Network Manager to support blog post content. The implementation includes hierarchical IPAM pool management with automatic CIDR allocation and splitting between platform and application landing zones.

Changes:

Added main AVNM module with IPAM support (avnm.bicep) that orchestrates network manager and IPAM pool deployment
Implemented hierarchical IPAM pool modules for root and per-region allocation (ipam.bicep, ipamPerRegion.bicep)
Added comprehensive landing zone type definitions with network, storage, key vault, and identity configurations

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
avnm/avnm.bicep	Main orchestration module that deploys Azure Virtual Network Manager and calls IPAM module with region configurations
avnm/ipam.bicep	Creates root IPAM pool and invokes per-region module for each specified region
avnm/ipamPerRegion.bicep	Implements CIDR subdivision logic and creates region, platform, and application IPAM pools
avnm/configuration/shared/lz.type.bicep	Comprehensive type definitions for landing zone resources including VNets, storage, Key Vault, and identity
avnm/configuration/platform/platformConnectivity-avnm.bicepparam	Example parameter file demonstrating IPAM configuration for Australia regions
avnm/configuration/.DS_Store	macOS system file that should not be in version control

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T13:18:49Z

avnm/avnm.bicep

+  @description('Required. Display name of the Azureregion.')
+  displayName: string
+
+  @maxValue(100)
+  @minValue(0)
+  @description('Required. Factor to divide the Azureregion CIDR into platform and application landing zones, in percentage.')


There is a space missing between "Azure" and "region" in the description. It should be "Azure region" instead of "Azureregion".

Suggested change

@description('Required. Display name of the Azureregion.')

displayName: string

@maxValue(100)

@minValue(0)

@description('Required. Factor to divide the Azureregion CIDR into platform and application landing zones, in percentage.')

@description('Required. Display name of the Azure region.')

displayName: string

@maxValue(100)

@minValue(0)

@description('Required. Factor to divide the Azure region CIDR into platform and application landing zones, in percentage.')

Copilot · 2026-01-26T13:18:50Z

avnm/avnm.bicep

+  @description('Required. Display name of the Azureregion.')
+  displayName: string
+
+  @maxValue(100)
+  @minValue(0)
+  @description('Required. Factor to divide the Azureregion CIDR into platform and application landing zones, in percentage.')


There is a space missing between "Azure" and "region" in the description. It should be "Azure region" instead of "Azureregion".

Suggested change

@description('Required. Display name of the Azureregion.')

displayName: string

@maxValue(100)

@minValue(0)

@description('Required. Factor to divide the Azureregion CIDR into platform and application landing zones, in percentage.')

@description('Required. Display name of the Azure region.')

displayName: string

@maxValue(100)

@minValue(0)

@description('Required. Factor to divide the Azure region CIDR into platform and application landing zones, in percentage.')

Copilot · 2026-01-26T13:18:50Z

avnm/ipamPerRegion.bicep

+param platformAndApplicationSplitFactor int
+
+// Calculate the total number of CIDR blocks available in the region
+// Using the RegionCIDRsplitSize parameter to determine subdivision granularity


The comment refers to 'RegionCIDRsplitSize' parameter, but the actual parameter name is 'regionLzCidrSize'. This inconsistency between the comment and the code could cause confusion.

Suggested change

// Using the RegionCIDRsplitSize parameter to determine subdivision granularity

// Using the regionLzCidrSize parameter to determine subdivision granularity

Copilot · 2026-01-26T13:18:50Z

avnm/ipamPerRegion.bicep

+var powersOfTwo = [1, 2, 4, 8, 16, 32, 64, 128, 256, 512]
+var currentRegionCidrSplitSize = int(split(regionCidr, '/')[1])
+var subdivisionSize = regionLzCidrSize // Use the parameter directly for subdivision
+var totalSubnetCount = powersOfTwo[subdivisionSize - currentRegionCidrSplitSize]


The powersOfTwo array only has 10 elements (indexes 0-9), but the code calculates the index as 'subdivisionSize - currentRegionCidrSplitSize'. With regionLzCidrSize (subdivisionSize) having a max value of 32 and currentRegionCidrSplitSize potentially being as small as 8 (from a /8 CIDR), the index could be as high as 24, which would cause an index out of bounds error. The array needs to be extended to handle all valid scenarios, or bounds checking should be added.

Suggested change

var powersOfTwo = [1, 2, 4, 8, 16, 32, 64, 128, 256, 512]

var currentRegionCidrSplitSize = int(split(regionCidr, '/')[1])

var subdivisionSize = regionLzCidrSize // Use the parameter directly for subdivision

var totalSubnetCount = powersOfTwo[subdivisionSize - currentRegionCidrSplitSize]

var currentRegionCidrSplitSize = int(split(regionCidr, '/')[1])

var subdivisionSize = regionLzCidrSize // Use the parameter directly for subdivision

var subnetExponent = subdivisionSize - currentRegionCidrSplitSize

var totalSubnetCount = int(pow(2, subnetExponent))

Copilot · 2026-01-26T13:18:51Z

avnm/configuration/shared/lz.type.bicep

+
+    @minValue(100)
+    @maxValue(4096)
+    @description('Required. Required. The priority of the rule. The value can be between 100 and 4096. The priority number must be unique for each rule in the collection. The lower the priority number, the higher the priority of the rule.')


The word "Required" appears twice in the description. This is redundant and should be removed.

Suggested change

@description('Required. Required. The priority of the rule. The value can be between 100 and 4096. The priority number must be unique for each rule in the collection. The lower the priority number, the higher the priority of the rule.')

@description('Required. The priority of the rule. The value can be between 100 and 4096. The priority number must be unique for each rule in the collection. The lower the priority number, the higher the priority of the rule.')

Copilot · 2026-01-26T13:18:51Z

avnm/ipam.bicep

+      applicationCidrs: regionIpamPool[i].outputs.applicationCIDRs
+      applicationCidrsCount: regionIpamPool[i].outputs.applicationCIDRsCount
+      platformCidrs: regionIpamPool[i].outputs.platformCIDRs
+      platformCidrsCount: regionIpamPool[i].outputs.platformCIDRs


There is a typo in the output property 'platformCidrsCount'. It should reference 'platformCIDRsCount' (using the outputs property) instead of 'platformCIDRs'. This will cause the output to return an array instead of an integer count.

Suggested change

platformCidrsCount: regionIpamPool[i].outputs.platformCIDRs

platformCidrsCount: regionIpamPool[i].outputs.platformCIDRsCount

Copilot · 2026-01-26T13:18:51Z

avnm/ipamPerRegion.bicep

+var platformSubnets = take(allSubnets, platformSubnetCount)
+var applicationSubnets = skip(allSubnets, platformSubnetCount)
+
+// Calculate the CIDRs for application landing zones


The comment on line 47 states "Calculate the CIDRs for application landing zones" but the variable being assigned is 'platformCIDRs'. This appears to be a copy-paste error. The comment should state "Calculate the CIDRs for platform landing zones".

Suggested change

// Calculate the CIDRs for application landing zones

// Calculate the CIDRs for platform landing zones

Copilot · 2026-01-26T13:18:51Z

avnm/configuration/shared/lz.type.bicep

+  @description('Optional. Deploy the outbound and the inbound peering.')
+  remotePeeringEnabled: bool?
+
+  @description('Optional. The name of the VNET Peering resource in the remove Virtual Network. If not provided, default value will be peer-remoteVnetName-localVnetName.')


There's a typo in the word "remove" which should be "remote" in the context of remote Virtual Network peering.

Copilot · 2026-01-26T13:18:52Z

avnm/ipamPerRegion.bicep

+// Calculate the CIDRs for application landing zones
+var platformCIDRs = platformSubnets
+
+// Calculate the CIDRs for application landing zones


The variable assignment is redundant. The variable 'platformCIDRs' is simply assigned the value of 'platformSubnets' without any transformation. Similarly, 'applicationCIDRs' is just assigned 'applicationSubnets'. These intermediate variables don't add value and could be removed for clarity, or if there's intended future processing, a comment should explain the purpose.

Suggested change

// Calculate the CIDRs for application landing zones

var platformCIDRs = platformSubnets

// Calculate the CIDRs for application landing zones

// CIDR allocations for platform landing zones (alias for platformSubnets, kept for clarity/future processing)

var platformCIDRs = platformSubnets

// CIDR allocations for application landing zones (alias for applicationSubnets, kept for clarity/future processing)

Copilot · 2026-01-26T13:18:52Z

avnm/ipamPerRegion.bicep

+
+@maxValue(32)
+@minValue(8)
+@description('CIDR size for the region IPAM pool. This is used to determine how many subnets can be created within the region CIDR.')


The 'regionLzCidrSize' parameter description states it's "CIDR size for the region IPAM pool" but this is not accurate. This parameter actually defines the CIDR size for each landing zone (LZ) subdivision within a region, not for the region IPAM pool itself. The description should clarify that this is the target CIDR prefix size for landing zone allocations within the region.

Suggested change

@description('CIDR size for the region IPAM pool. This is used to determine how many subnets can be created within the region CIDR.')

@description('Target CIDR prefix size for landing zone (LZ) allocations within the region IPAM pool. This value is used to determine how many LZ subnets can be created from the region CIDR.')

ipam modules and templates

7a95a83

tulpy requested a review from Copilot January 26, 2026 13:14

Copilot started reviewing on behalf of tulpy January 26, 2026 13:14 View session

tulpy merged commit 44098fc into main Jan 26, 2026
14 of 15 checks passed

Copilot AI reviewed Jan 26, 2026

View reviewed changes

tulpy deleted the feat/26-01-2026-avnm branch January 26, 2026 13:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ipam modules and templates#1

ipam modules and templates#1
tulpy merged 1 commit intomainfrom
feat/26-01-2026-avnm

tulpy commented Jan 26, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// Using the RegionCIDRsplitSize parameter to determine subdivision granularity
	// Using the regionLzCidrSize parameter to determine subdivision granularity

	@description('Required. Required. The priority of the rule. The value can be between 100 and 4096. The priority number must be unique for each rule in the collection. The lower the priority number, the higher the priority of the rule.')
	@description('Required. The priority of the rule. The value can be between 100 and 4096. The priority number must be unique for each rule in the collection. The lower the priority number, the higher the priority of the rule.')

	platformCidrsCount: regionIpamPool[i].outputs.platformCIDRs
	platformCidrsCount: regionIpamPool[i].outputs.platformCIDRsCount

	// Calculate the CIDRs for application landing zones
	// Calculate the CIDRs for platform landing zones

	@description('CIDR size for the region IPAM pool. This is used to determine how many subnets can be created within the region CIDR.')
	@description('Target CIDR prefix size for landing zone (LZ) allocations within the region IPAM pool. This value is used to determine how many LZ subnets can be created from the region CIDR.')

Conversation

tulpy commented Jan 26, 2026

Pull Request Template

Description

Type of Change

Checklist

Related Issues

Additional Notes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants