Allow subnet addressprefix to be a parameter.#14358
Conversation
Follow-up feedback from dotnet#13108
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14358Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14358" |
There was a problem hiding this comment.
Pull request overview
This PR enables subnet address prefixes to be parameterized in Azure Virtual Network configurations, allowing them to be specified dynamically rather than as hardcoded values. This is a follow-up enhancement to PR #13108 which introduced initial Azure Virtual Network support.
Changes:
- Added a new
AddSubnetoverload that acceptsIResourceBuilder<ParameterResource>for the address prefix - Modified
AzureSubnetResourceto support both string literals and parameter resources using a discriminated union pattern - Added comprehensive test coverage including unit tests and Bicep generation verification
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Aspire.Hosting.Azure.Network/AzureSubnetResource.cs |
Refactored to support both string and ParameterResource address prefixes using a discriminated union pattern with two constructors and conditional logic in ToProvisioningEntity |
src/Aspire.Hosting.Azure.Network/AzureVirtualNetworkExtensions.cs |
Added new AddSubnet overload accepting IResourceBuilder<ParameterResource> and refactored common logic into AddSubnetCore helper method |
tests/Aspire.Hosting.Azure.Tests/AzureVirtualNetworkExtensionsTests.cs |
Added three new tests covering parameterized subnet creation, custom subnet naming, and Bicep generation |
tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureVirtualNetworkExtensionsTests.AddSubnet_WithParameterResource_GeneratesBicep.verified.bicep |
Snapshot test file verifying correct Bicep template generation with parameterized address prefix |
mitchdenny
left a comment
There was a problem hiding this comment.
Looks good! One minor suggestion: should the string overload do stricter CIDR notation validation (e.g. regex for x.x.x.x/y)? Currently both the VNet and subnet constructors only check for null/empty, meaning a typo like "10.0.0.0" (missing prefix length) or "not-a-cidr" would be silently accepted and only fail at Azure deployment time. Validating the format early would give developers a faster feedback loop — though it does add a maintenance surface if Azure ever relaxes/changes the accepted formats.
|
We don't really do validation of other Azure accepted strings. If anything maybe the validation should be in Azure.Provisioning or bicep and not us. |
Follow-up feedback from #13108