bicep icon indicating copy to clipboard operation
bicep copied to clipboard

Set or skip an object property based on a condition

Open majastrz opened this issue 4 years ago • 87 comments

Is your feature request related to a problem? Please describe. When conditionally enabling ipv6 in a template, there are cases where some properties need to be either set or omitted from a resource declaration. Currently in Bicep, this requires creating two separate object literals and choosing between them via the ternary operator.

Describe the solution you'd like Can we create a simpler way of doing this that doesn't involve declaring two separate objects? Should we consider introducing a concept of undefined that is separate from null?

majastrz avatar Aug 29 '20 02:08 majastrz

@marcre - we're pretty sure null with the ternary operator would fulfill this, so wondering if we are missing something.

alex-frankel avatar Sep 03 '20 21:09 alex-frankel

Yes and no - depends on how we handle NULL.. Does null mean we omit the value when we send to the RP, or do we send the null on. If we don't send the null, then we don't need undefined. However, with PUTCH RPs like compute you may need to be able to explicitly set something to null to make sure it is cleared if previously set.

marcre avatar Sep 04 '20 03:09 marcre

Agree with that. We would like the distinction between null and undefined/omitted properties not to exist, but the reality disagrees.

majastrz avatar Sep 04 '20 06:09 majastrz

agree as well. Not all RPs react on null equally.

slavizh avatar Sep 04 '20 09:09 slavizh

I have had to rely on the createObject function nested in a compex comparison function inside a copy for an array property and it is very painful. Particularly when error messages from the RPs do not help.

Happy to share a sample of this!

I'd say that having an easy way of defining if you want a property to be omitted, set to null, or set to an object would be very useful.

pacodelacruz avatar Nov 12 '20 19:11 pacodelacruz

+1, this would be super useful in so many use-cases. for example, like in Javascript

param autoscaller bool
param agentCount int = 3

// define the regular node profile
var nodepool1Profile = {
  name: 'nodepool1'
  count: agentCount
  enableAutoScaling: autoscaller
}

// add the 'maxCount' property to agentPoolProfiles  if autoscaller is true
let agentPoolProfiles =  autoscaller? {...nodepool1Profile, maxCount: agentCount+10} : nodepool1Profile 

khowling avatar Jan 14 '21 22:01 khowling

+1 really like this to be supported. My scenarios is - I have 3 different set of nsg rules, which contains 90% same rules. I'd like to be able to define all rules in one array and use bool parameters to control which rules to be included when instantiated. I think if makes more sense than ternary in this case: myarray: [ if (includeA) { itemA } { ItemB } ]

yuanyi2121 avatar Mar 03 '21 20:03 yuanyi2121

Would love if this existed. I'm trying to define the 'analytical TTL' on a Cosmos container and it only accepts an 'int' but in the .NET SDK it's a nullable int, so 'undefined' would be the only way I could conditionally set an analytical storage ttl.

Or it could be done w/ spread ( #1560 )

Or per the ugly way:

image

onionhammer avatar May 12 '21 13:05 onionhammer

Looks like AppGateway uses a number of resource specific top-level properties (zones, identity etc). I cannot see any way to conditionally include these (for my use-case, the identity object). Looks like you cannot use a variable for the entire resource body resource appgw 'Microsoft....@2020' = varname , so cannot use the union method. Any suggestions?

image

khowling avatar Jul 23 '21 14:07 khowling

@khowling does using the ternary operator work?

var shouldUseIdentity = true

resource appGw 'Microsoft.Network/applicationGateways@2021-02-01' = {
  name: 'foo'
  identity: shouldUseIdentity ? {
    type: 'UserAssigned'
    userAssignedIdentities: {
      '${userAssignedMI}': {}
    }
  } : null
}

alex-frankel avatar Jul 26 '21 16:07 alex-frankel

@alex-frankel Unfortunately not, that produces:

Deployment template parse failed: 'Required property 'type' not found in JSON. Path

if I replace {} with null, I get

Error BCP036: The property "identity" expected a value of type "ManagedServiceIdentity" but the provided value is of type "null | object".

khowling avatar Jul 26 '21 17:07 khowling

I ran into this issue trying to create a bicep file that creates an Azure function with a consumption plan in the test environment and a standard plan (for always on) in production.

Cannot update the site 'test-func' because it uses AlwaysOn feature which is not allowed in the target compute mode.

It fails the value for AlwaysOn is null or false, so the property needs to be omitted entirely.

daniellittledev avatar Aug 18 '21 07:08 daniellittledev

I encountered this today when trying to conditionally add vnet rules to a storage account depending on a boolean input. ARM will not accept an empty virtualNetworkRules property.

ehrnst avatar Sep 09 '21 13:09 ehrnst

Maybe undefined or omit should be a literal value thats assignable to properties.

onionhammer avatar Sep 09 '21 13:09 onionhammer

@khowling does using the ternary operator work?

var shouldUseIdentity = true

resource appGw 'Microsoft.Network/applicationGateways@2021-02-01' = {
  name: 'foo'
  identity: shouldUseIdentity ? {
    type: 'UserAssigned'
    userAssignedIdentities: {
      '${userAssignedMI}': {}
    }
  } : null
}

Thanks Alex. I'm new to Bicep and your little snip helped me out. Only thing I changed was the null for {}

Here is what I ended up using when one probe used statusCodes, but the second probe did not need "Use probe matching conditions" checked:

probes: [for probe in appGateway.probes : {
      name: probe.name
      properties: {
        protocol: probe.protocol
        path: probe.path
        interval: probe.interval
        timeout: probe.timeout
        unhealthyThreshold: probe.unhealthyThreshold
        pickHostNameFromBackendHttpSettings: probe.pickHostNameFromBackendHttpSettings
        minServers: probe.minServers
        match: probe.shouldShowMatchStatusCodes ? {
          statusCodes: [
            probe.matchStatusCodes
          ]
        } : {}
      }
    }]

reidcurry avatar Sep 17 '21 00:09 reidcurry

Another use case where this is causing trouble: I'm making a template for an App Gateway. In the backendHttpSettingsCollection, there are two attributes, hostname and pickHostNameFromBackendAddress. If pickHostNameFromBackendAddress is true, you don't need hostname, otherwise you do. I tried to do this conditionally based on my parameters:

backendHttpSettingsCollection: [for backendHttpSetting in backendHttpSettingsCollection : {
   name: backendHttpSetting.name
   properties: {
     port: (contains(backendHttpSetting.properties, 'port')) ? backendHttpSetting.properties.port : null
     protocol: (contains(backendHttpSetting.properties, 'protocol')) ? backendHttpSetting.properties.protocol : null
     hostName: (contains(backendHttpSetting.properties, 'hostname')) ? backendHttpSetting.properties.hostname : null
     pickHostNameFromBackendAddress: (contains(backendHttpSetting.properties, 'pickHostNameFromBackendAddress')) ? backendHttpSetting.properties.pickHostNameFromBackendAddress : false

The problem is that even having "hostname: null" in your deployment causes an error if pickHostNameFromBackendAddress is true:

ApplicationGatewayBackendHttpSettingsHostNameFieldConflict - HostName field may only be specified if pickHostNameFromBackendAddress is set to false in context '/subscriptions/......'

I have two backendHttpSettingsCollections, one with pickHostNameFromBackendAddress false, the other with it true. So I need to be able to conditionally include or exclude the hostname attribute, not just conditionally assign a value or a null to it.

jrobins-tfa avatar Sep 23 '21 16:09 jrobins-tfa

We need an 'undefined' value that can be put in.. it's familiar to people who know any JavaScript

onionhammer avatar Sep 23 '21 17:09 onionhammer

What we do now as a workaround (awaiting a proper alternative) is using modules and conditional deployment. We have several VM images that needs to deploy, but only some of the have plans. We now need one module for VmWithPlan, and one VmWithoutPlan. It works, but It means we need to repeat our code in two files which we don't want. Please keep me posted regarding a fix.

erlandsen-tech avatar Sep 30 '21 09:09 erlandsen-tech

We have a similar issue. We're trying to structure our templates with dedicated modules for child-resources. However, the user does not need to provide all values in the template's parameter file. The problem is that the child-resource template (e.g. container for storage account) comes with its own set of parameters, default values & validation - and not all parameters are mandatory. The problem is, the invoking template (e.g. storage account) will need an option to pass all parameters to this child-template the user specified, but not more. However, there is no way of achieving this and e.g. hand over null so that the child-template would fall back tot he default:

For example: image will result into: Error: The value of deployment parameter 'cors' is null. Please specify the value or use the parameter reference

The only workaround here is to not pass null but instead duplicate the default value we have already specified in the child-template parameter: image

With some sort of undefined type value we could solve this issue and have the child-template just fall back to the default.

AlexanderSehr avatar Oct 26 '21 16:10 AlexanderSehr

Also have another use case for this.

When deploying a subnet, in some cases a route table is not required (say for example, appgw, or bastion), but other subnets that contain IaaS resources or others may require a route table.

I tried applying some of the workarounds listed here, but was still getting errors, but found using json('null') instead of {} fixed these issues. Issues such as:

Error: The value of deployment parameter 'routeTable' is null. Please specify the value or use the parameter reference

image

image

nubgamerz avatar Nov 19 '21 08:11 nubgamerz

The problem of conditionally passing params to a resource reminds me of #3351 where the coherent expectation of null/empty/missing property value would be required by the RPs.

stan-sz avatar Nov 19 '21 13:11 stan-sz

Could we use a version of the conditional for resource itself to only include the setting in the compiled template if the condition is true?

resource storage 'Microsoft.Storage/storageAccounts@2021-06-01' = { name: storageName location: resourceGroup().location kind: 'StorageV2' sku:{ name:'Standard_LRS' } properties: { accessTier if(accessRequired):'Hot' } }

paulhasell avatar Jan 31 '22 16:01 paulhasell

Will not be possible. This needs to be implemented in Azure Resource Manager rather in Bicep as Bicep needs to know the value for the condition at compilation and if the value comes from parameter Bicep would not know the value thus not knowing to turn the defined resource in json with the property or without it.

slavizh avatar Feb 01 '22 06:02 slavizh

@slavizh wouldn't it just compile to a union?

I prefer the 'undefined' approach persnally;

resource storage 'Microsoft.Storage/storageAccounts@2021-06-01' = {
name: storageName
location: resourceGroup().location
kind: 'StorageV2'
sku:{
name:'Standard_LRS'
}
properties: {
accessTier:  accessRequired ? 'Hot' : undefined
}
}

onionhammer avatar Feb 01 '22 14:02 onionhammer

@onionhammer accessRequired can be a parameter and parameter is passed during deployment which means that when bicep converts to ARM it will not know what is the value of accessRequired thus not knowing to remove accessTier property from properties or leave it .

slavizh avatar Feb 02 '22 07:02 slavizh

These limitations leave ARM and Bicep unsuitable for a large range of deployment scenarios, especially 'upgrades' of existing environments with new features which need to conditionally add/update properties and collections. Powershell will have to remain our primary tool for scripting deployments although that seems to be the lowest priority in getting updated by MS.

paulhasell avatar Feb 02 '22 09:02 paulhasell

@slavizh I'm not saying it is known at compile time, I'm saying it compiles to ARM's 'union' the same exact way this is done today with bicep, albeit with a TON more code..

union({ 
 ..other properties
}, accessRequired ? {
  accessTier: 'Hot'
} : {})

onionhammer avatar Feb 02 '22 15:02 onionhammer

This will most likely require both a Bicep and ARM template change. On the bicep side, we need some new keyword/gesture for saying "don't include this" and on the ARM Template/Deployments service side we (most likely) need an equivalent gesture. I don't think the template language has a way of expressing this today.

alex-frankel avatar Feb 02 '22 19:02 alex-frankel

@alex-frankel slavizh's concern was that if 'accessRequired' is a paameter, this wouldnt work - I use parameters like this today

param isPremium bool = env == 'prod'

resource app 'Microsoft.Web/sites@2020-06-01' = { 
  
   resource settings 'config' = { 
      properties: union({
          // non-conditional settings...
      }, isPremium ? {
          // conditional on 'premium' parameter
          WEBSITE_CONTENTAZUREFILECONNECTIONSTRING: storageAccountString
          WEBSITE_CONTENTSHARE: '${env}-worker'
       } : { })
    }
}

What am I not understanding where this fails?

onionhammer avatar Feb 02 '22 19:02 onionhammer

Some examples that should work today. Code is a mock so when you use actual resources you will need to check which properties are required.

param isPremium bool
param env string = 'prod'

resource app 'Microsoft.Web/sites@2020-06-01' = {
   resource settings 'config' = {
      properties: isPremium ? {} : {
          WEBSITE_CONTENTAZUREFILECONNECTIONSTRING: storageAccountString
          WEBSITE_CONTENTSHARE: '${env}-worker'
      }
    }
}
param isPremium bool
param env string = 'prod'

resource app 'Microsoft.Web/sites@2020-06-01' = {
   resource settings 'config' = {
     properties: union( {
       someProperty: 'value'
     }, isPremium ? {
          WEBSITE_CONTENTAZUREFILECONNECTIONSTRING: storageAccountString
          WEBSITE_CONTENTSHARE: '${env}-worker'
      }: {})
    }
}

slavizh avatar Feb 03 '22 07:02 slavizh