microsoft / PSRule

Validate infrastructure as code (IaC) and objects using PowerShell rules.
https://microsoft.github.io/PSRule/v2/
MIT License
389 stars 51 forks source link

Getting Index outside of bounds of array with first function on empty array. #1529

Closed tstooke closed 1 year ago

tstooke commented 1 year ago

Description of the issue

Getting Index was outside the bounds of the array error with relatively complex conditional using union, first, and filter in Bicep module.

We have a module that will create a Virtual Network and optionally create Subnets and link those Subnets to NAT Gateways.

The intention is to check the incoming NATGateways array for an item that matches the Context of the current Subnet when looping to create the Subnets. This way, we can dynamically link Subnets to NAT Gateways by simply providing the correct input parameters.

The logic is correct, and the module does deploy to Azure.

To Reproduce

This is a simplified version of the module that produces the error (and this also produces the error):

param Location string = resourceGroup().location
param AddressPrefixes array
param Subnets array = []
param NATGateways array = []

resource virtualNetwork 'Microsoft.Network/virtualNetworks@2022-07-01' = {
  name: 'temp-vnet'
  location: Location
  properties: {
    addressSpace: {
      addressPrefixes: AddressPrefixes
    }
    subnets: [for subnet in Subnets: {
      name: subnet.Name
      // Add the NAT Gateway ID to the Subnets with matching Context
      // When there are no NAT Gateways or none match, just use the existing Subnet Properties
      properties: empty(NATGateways) || empty(first(filter(NATGateways, item => item.Context == subnet.Context))) ? subnet.Properties : union(subnet.Properties, {natGateway: {id: first(filter(NATGateways, item => item.Context == subnet.Context)).Id}})
    }]
  }
}

We test that as a module with the following bicep file:

module testWithAllParameters './temp.bicep' = {
  name: 'test-with-all-parameters'
  #disable-next-line explicit-values-for-loc-params
  params: {
    AddressPrefixes: [
      '10.0.0.0/16'
    ]
    Subnets: [
      {
        Name: 'subnet1'
        Context: 'test1'
        Properties: {
          addressPrefix: '10.0.0.0/24'
        }
      }
      {
        Name: 'subnet2'
        Context: 'test2'
        Properties: {
          addressPrefix: '10.0.1.0/24'
        }
      }
    ]
    NATGateways: [
      {
        Context: 'test1'
        Name: 'test-gateway'
        Id: 'test-gateway-id'
      }
    ]
  }
}

The error seems to be related to the second Subnet that has no matching NAT Gateway item. If we take out that Subnet, then there is no error from PSRule.

We're wondering if it's due to the first call on the empty array result from filter. Maybe PSRule is doing something like array[0] for first, which would fail on an empty array?

Expected behaviour

We've tested the module with deployments to Azure, and it does work with the current logic. We get successful deployments with any number of Subnet and any number of NAT Gateways, even with none of them matching on the Context value.

The expected behavior from PSRule would be to not throw this error and handle the logic the same as Bicep and ARM do.

Error output

Failed to expand bicep source '[redacted]\temp.tests.bicep'. 
Exception calling "GetBicepResources" with "2" argument(s): 
"Unable to expand resources because the source file '[redacted]\temp.tests.bicep' was not valid. 
An error occurred evaluating expression 
'[if(or(empty(parameters('NATGateways')), empty(first(filter(parameters('NATGateways'), lambda('item', equals(lambdaVariables('item').Context, parameters('Subnets')[copyIndex('subnets')].Context)))))), parameters('Subnets')[copyIndex('subnets')].Properties, union(parameters('Subnets')[copyIndex('subnets')].Properties, createObject('natGateway', createObject('id', first(filter(parameters('NATGateways'), lambda('item', equals(lambdaVariables('item').Context, parameters('Subnets')[copyIndex('subnets')].Context)))).Id))))]'
 line 95. Index was outside the bounds of the array."

Module in use and version:

Captured output from $PSVersionTable:

PSVersion                      7.3.4
PSEdition                      Core
GitCommitId                    7.3.4
OS                             Microsoft Windows 10.0.19045
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Additional context

To maybe clarify the logic, we check to see if there are any NAT Gateways:

empty(NATGateways)

or if there's a matching NAT Gateway for the current Subnet by filtering and checking for empty:

empty(first(filter(NATGateways, item => item.Context == subnet.Context)))

If both of those are false, we just take subnet.Properties

If we find a matching NAT Gateway, we use union to combine the subnet.Properties with a new object that provides the NAT Gateway ID from the first matching item:

{natGateway: {id: first(filter(NATGateways, item => item.Context == subnet.Context)).Id}}

NOTE: If this turns out to be a PSRule for Azure issue, we'd be happy to report it in that repo, instead.

tstooke commented 1 year ago

Immediately after posting, we eliminated the error by changing this:

empty(first(filter(NATGateways, item => item.Context == subnet.Context)))

to this:

empty(filter(NATGateways, item => item.Context == subnet.Context))

This is in the initial part of the logic that's looking for a match. We don't actually need the first call there - we only need to know if the result of the filter call is empty or not. By removing that first call, PSRule no longer throws the error (and we still get a valid deployment).

That still leaves a first call in the logic, but first is no longer called on an empty array. We think this means there's something not quite right about first in PSRule when dealing with empty arrays.


Happy to close this issue if there's no need to change anything PSRule for this.

BernieWhite commented 1 year ago

@tstooke Nice work on the troubleshooting and thanks for the detail.

If empty(filter(NATGateways, item => item.Context == subnet.Context)) works then I don't believe empty(first(filter(NATGateways, item => item.Context == subnet.Context))) should be a different behavior but we'll double check.

Let's leave the issue open for now until we dig a little deeper and verify there isn't anything funny happening with this use case.

BernieWhite commented 1 year ago

Hi @tstooke, are you able to confirm if this was fixed by PSRule for Azure v1.28.0?

tstooke commented 1 year ago

@BernieWhite I just tested empty(first(filter(...))) and we still see the "Index out of bounds" error. Using empty(filter(...)) still works perfectly. We're on PSRule 2.9.0 and PSRule for Azure 1.28.1.

To clarify, we don't actually need to use first when checking if the filter is empty, so we took out the first call to optimize the chain anyway.

BernieWhite commented 1 year ago

@tstooke Are you able to try PSRule for Azure v1.29.0? Thanks.

tstooke commented 1 year ago

@BernieWhite PSRule for Azure v1.29.0-B0062 seems to do the trick. Tests pass with and without the first in place as mentioned above in this issue.