BCP073 thrown for properties in resourceDerivedTypes

[schaijkw]

Bicep version last worked in 0.29.47 broken since 0.30.3

Describe the bug We use the experimental resourceDerivedTypes feature and the new bicep versions are breaking our setup. We have this parameter defined in a module:

param siteProperties resource<'Microsoft.Web/sites@2022-09-01'>.properties = {
  clientAffinityEnabled: false
  clientCertEnabled: true
  clientCertMode: 'OptionalInteractiveUser'
  httpsOnly: true
}

In the module there are no warnings or exceptions. But in the file that is using this module we see exceptions, like this: The property "inProgressOperationId" is read-only. Expressions cannot be assigned to read-only properties. bicep (BCP073)

To Reproduce Create 2 modules;

Enable Experimental Feature in bicepconfig.json

    "experimentalFeaturesEnabled": {
        "resourceDerivedTypes": true
    }

Module 1, parameter with a set of default options:

param siteProperties resource<'Microsoft.Web/sites@2022-09-01'>.properties = {
  clientAffinityEnabled: false
  clientCertEnabled: true
  clientCertMode: 'OptionalInteractiveUser'
  httpsOnly: true
}

resource appService 'Microsoft.Web/sites@2022-09-01' = {
  name: appServiceName
  location: location
  tags: tags
  kind: kind
  identity: {
    type: 'SystemAssigned'
  }
  properties: union({
    serverFarmId: servicePlanId}, 
    siteProperties
  )

Module 2

@description('Site resource specific properties.')
param siteProperties resource<'Microsoft.Web/sites@2022-09-01'>.properties?


module appService 'module1.bicep' = {
  name: 'appService'
  params: {
    siteProperties: siteProperties
  }
}

First use bicep version 0.29.47, then update to 0.30.3 and you'll see the exceptions.

[jeskew]

Bicep 0.30 included an update that performs deep type checking on values passed in as parameters, which has the unexpected side effect documented here. Resource-derived types (unlike standard user-defined types) can have read-only properties, which makes an assignment of type resource<'Microsoft.Web/sites@2022-09-01'>.properties to a target of type resource<'Microsoft.Web/sites@2022-09-01'>.properties raise some warning diagnostics.

One thing we could do here is skip the check that raises BCP073 on read-only properties of the assigned value. This make sense for the case of a resource-derived-typed parameter, since a parent module wouldn't be able to populate those read-only properties without raising its own BCP073 diagnostic.

But it would miss cases like this:

main.bicep

resource appService 'Microsoft.Web/sites@2022-09-01' = {...}

module mod 'mod.bicep' = {
  name: 'mod'
  params: {
    siteProperties: appService.properties
  }
}

mod.bicep

param siteProperties resource<'Microsoft.Web/sites@2022-09-01'>.properties?

resource appService 'Microsoft.Web/sites@2022-09-01' = {
  ...
  properties: siteProperties 
}

since appService.properties is likely to have read-only properties populated since it's a return value from the RP. I expect RPs already have to handle this gracefully, though, since GET-modify-PUT is a pretty common pattern in imperative deployments.

[schaijkw]

If the RPs don't handle this gracefully, is it possible to detect if the property is read-only? Because in that case you can ignore or remove those properties. It will behave as expected because if you try to set a read-only property manually the BCP073 will be raised, and if you were referencing the resource as you do in your example, you don't intent to change those read-only properties.

[jeskew]

Writeup for discussion: https://gist.github.com/jeskew/150611ca5280493e0f10d22823f86e67

[jeskew]

The team had a few discussions on this and landed on adopting the following remediation:

• When narrowing types, Bicep should not raise a violation if the assigned value and the assignment target both have a ReadOnly flag. The BCP073 diagnostic should only be raised when the assignment target has a ReadOnly flag, but the assigned value does not. This will ensure that the diagnostic is only raised at the actual point of violation, and not when value is being passed via a parameter with a resource derived type.
• The type assigned to a resource symbol (e.g., appService in resource appService 'Microsoft.Web/sites@2022-09-01') should not be the same as the value expected on the right hand of the assignment in a resource statement. The former represents the result of a resource deployment action (the resource output), whereas the latter represents what will be sent to resource provider (the resource input). The pertinent distinction here is that ReadOnly flags make sense on the resource input but not on the resource output, and WriteOnly flags make sense on the resource output but not on the resource input.
• Bicep should add a new parameterized type corresponding to each of the aforementioned types, to be named resourceInput and resourceOutput.
  • To calculate resourceInput<'type@version'>, Bicep will look up the named resource type and recursively strip away any WriteOnly flags.
  • To calculate resourceOutput<'type@version'>, Bicep will look up the named resource type and recursively strip away any ReadOnly flags.
• The existing resource<'type@version'> parameterized type should be deprecated. We can add a linter that proposes a fix to resourceInput<'type@version'> on the assumption that that's what template authors usually want with a resource-derived type.
• At or before GA, the resource<'type@version'> type should be removed.