fix: Add typed CIDR placeholders for unresolved address prefixes - #3908
Open
Jerome Brown (oWretch) wants to merge 2 commits into
Open
fix: Add typed CIDR placeholders for unresolved address prefixes#3908Jerome Brown (oWretch) wants to merge 2 commits into
Jerome Brown (oWretch) wants to merge 2 commits into
Conversation
When address prefixes are allocated at deployment time, such as by Azure Virtual Network Manager IPAM pools, `reference()` cannot resolve them during Bicep expansion. The empty result was then passed to `cidrHost()` and `cidrSubnet()`, which failed with "The specified CIDR '' is not valid". Add a source-aware placeholder table keyed on resource type and normalized property path, so unresolved properties return a typed mock value instead of an empty one. Placeholders use RFC 5737 TEST-NET-1 (192.0.2.0/24) so they are obvious in output and cannot collide with real address space. Covered today: - `Microsoft.Network/virtualNetworks` - `addressSpace.addressPrefixes` - `Microsoft.Network/virtualNetworks/subnets` - `addressPrefix`, `addressPrefixes` - `Microsoft.Network/networkManagers/ipamPools` - `addressPrefixes` The table is the extension point, so additional resource properties can be added without further changes to the expansion code. The `cidr*()` functions are deliberately left strict. Only indexed access into a placeholder array yields a CIDR string, so genuine authoring mistakes, such as passing `id` or an unindexed `addressPrefixes`, still fail as before. Fixes Azure#3907 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`tryGet(array, 'propertyName')` should return null for an invalid
property lookup against an array, matching ARM/Bicep semantics, so a
surrounding `coalesce()` can fall back to another value.
`ExpressionHelpers.TryPropertyOrField` treated any non-JValue JToken,
including JArray, as a property bag and attempted an index lookup by
name, throwing:
Accessed JArray values with invalid key value: "addressPrefixes".
Int32 array index expected.
Exclude JArray from that branch so TryPropertyOrField returns false for
an array, and Functions.TryGet returns null as intended.
This was found expanding a template with:
coalesce(
tryGet(lambdaVariables('env').value, 'addressPrefixes'),
lambdaVariables('env').value
)
where `env.value` can be either an object with an `addressPrefixes`
property, or an array of CIDR strings.
Related to Azure#3907
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3907
Problem
When address prefixes are allocated at deployment time — for example by Azure Virtual Network Manager IPAM pools —
reference()cannot resolve them during Bicep expansion. The unresolved property came back empty, and the empty value was then passed straight intocidrHost()/cidrSubnet():This makes it impossible to validate otherwise-correct templates that use IPAM-allocated address space.
Approach
Add a source-aware placeholder table in
Mock.cs, keyed on resource type + normalized property path. When a property cannot be resolved, expansion now returns a typed mock value appropriate for that property rather than an empty one.Covered today:
Microsoft.Network/virtualNetworksaddressSpace.addressPrefixes["192.0.2.0/24"]Microsoft.Network/virtualNetworks/subnetsaddressPrefix"192.0.2.0/28"Microsoft.Network/virtualNetworks/subnetsaddressPrefixes["192.0.2.0/28"]Microsoft.Network/networkManagers/ipamPoolsaddressPrefixes["192.0.2.0/24"]Placeholders use RFC 5737 TEST-NET-1 (
192.0.2.0/24) so they are obvious in output and cannot collide with real customer address space.The table is the extension point — other resources with inferable properties (network interfaces, firewalls, DNS resolver endpoints, and so on) can be added later by adding rows, without further changes to the expansion code.
To make this work for both
existingand deployed resources, resource type context is now carried through symbol lookup:IDeploymentSymbolgainsTryGetResource, implemented byObjectDeploymentSymbolandArrayDeploymentSymbol, so symbol-only (existing) resources retain their type.MockResourceObjectsupplies typed placeholders for missing properties on a known resource while keeping normal object semantics, so partially-knownpropertiesobjects don't lose resource type context.TemplateContext.TryGetResourcekeeps the_ResourceIdslookup authoritative (so module/deployment references still return aDeploymentValue), falling back to the symbol-attached resource.The
cidr*()functions stay strictThis deliberately does not relax
cidrHost()/cidrSubnet(). Only indexed access into a placeholder array yields a CIDR string —MockResourcePropertyArrayremains aMockArray. So genuine authoring mistakes still fail exactly as they do today:reference('vnet').idaddressPrefixesinstead ofaddressPrefixes[0]addressPrefixvs arrayaddressPrefixesconfusionAdditional fix:
tryGet()on arraysWhile validating against a wider set of real templates, found a related expansion failure with a different shape:
Here
env.valuemay be either an object with anaddressPrefixesproperty, or an array of CIDR strings directly. ARM/BiceptryGet(array, 'propertyName')should returnnullfor an invalid property lookup socoalesce()falls back to the array, but PSRule threw:ExpressionHelpers.TryPropertyOrFieldtreated any non-JValueJToken, includingJArray, as a property bag. ExcludedJArrayfrom that branch so it returnsfalsefor an array + string property, andFunctions.TryGet()returnsnullas intended.Testing
FunctionTests.cscovering placeholder resolution, existing/deployed symbol resolution, that concrete values are still preferred over placeholders, andtryGet()returningnullfor an array.tryGet/coalesceshape above; all now expand and evaluate cleanly.