bicep icon indicating copy to clipboard operation
bicep copied to clipboard

listKeys fires too early in the execution graph

Open itpropro opened this issue 3 years ago • 10 comments

Bicep version Bicep CLI version 0.7.4 (5afc312467)

Describe the bug When using listKeys as a function, it creates two separate steps in the deployment that are not respecting any dependsOn or naming requirements. These leads to deployments that always fail at first deployment as they try to listKeys from a storage account that is not deployed yet. This also means that on first deployment, the Function Apps are not deployed, as a dependency (the listKeys entries) has failed.

image

If the same template is deployed again, it works as the storage accounts already exist and the Function Apps are actually created.

image

It is probably caused by the multiple app settings of the function app that need the storage account key and which are currently implemented like this:

...
      appSettings: [
        {
          name: 'AzureWebJobsStorage'
          value: 'DefaultEndpointsProtocol=https;AccountName=${functionStorage[index].outputs.name};EndpointSuffix=${environment().suffixes.storage};AccountKey=${listKeys(resourceId(subscription().subscriptionId, format('{0}-{1}', prefix, environment), 'Microsoft.Storage/storageAccounts', substring(format('{0}function{1}{2}', prefix, 'storage', environment), 0, length(format('{0}function{1}{2}', prefix, 'storage', environment)) >= 24 ? 24 : length(format('{0}function{1}{2}', prefix, 'storage', environment)))), '2021-09-01').keys[0].value}'
        }
      ...
      ]
...

There is a lot of duplication of functions, as it is currently not possible to directly reference the name or any other property like the resourceId of the storage account resource in functions. Instead we have to execute the storage name generation logic every time we need the storage name, which is used in the resourceId and listKeys functions. As the storage account also has length limitations which are incorporated into the naming function, they are also checked every time.

To Reproduce I have created a reproduction repository with a minimal deployment of a function app to reproduce the error. The following resources are deployed (in a loop over how many environments you want)

  • Resource group
  • storage account for the Function App
  • Hosting plan for the Function App
  • Function App itself with the AzureWebJobsStorage app setting

To recreate the environment, just clone the repository https://github.com/itpropro/bicep-reproduction and execute az deployment sub create -l 'westeurope' -n (New-Guid).Guid -f .\reproduction.bicep, no parameters needed.

When you deploy for the first time, it should look like on the above screenshot, when you deploy the same template a second time, there should be no errors.

itpropro avatar Jun 29 '22 19:06 itpropro

You are right that listKeys() is firing too soon (though I wouldn't call this an idempotency issue). We are aware of the issue and plan to resolve it soon, but can't give a precise ETA just yet. I'm actually surprised we don't have a public-facing issue for this, so thanks for reporting.

Including the internal work item for reference: https://msazure.visualstudio.com/One/_workitems/edit/7512374

alex-frankel avatar Jul 05 '22 22:07 alex-frankel

Linking an internal PR: https://msazure.visualstudio.com/One/_git/AzureUX-Deployments/pullrequest/7844064

We have started some work in the runtime to support dependsOn in the listKeys() as a new, optional argument in the listKeys() function. We will pair this with a capability to autogenerate listKeys() in bicep, just like we do for dependsOn between resources today. Will keep this thread updated as we make more progress.

alex-frankel avatar Apr 25 '23 00:04 alex-frankel

moving comment here from:

  • https://github.com/Azure/bicep/issues/10410
  • Appears to be duplicate
  • The fix may cover this scenario as well, adding example to test later, once closed.
  1. listKeys is never executed 🟩
  2. listKeys is never executed 🟩
  3. listKeys is always executed 🟧
param storageAccountName string = uniqueString(resourceGroup().id)
param myBool bool = false

resource SA 'Microsoft.Storage/storageAccounts@2022-09-01' existing = {
  name: storageAccountName
}

// 1 listKeys is never executed 🟩
var info = true ? 'info' : SA.listKeys().keys[0].value

// 2 listKeys is never executed 🟩
var info2 = false ? SA.listKeys().keys[0].value : 'info2'

module return1 'temp2.bicep' = {
  name: 'return1'
  params: {
    returnValue: myBool
  }
}

/* temp2.bicep
param returnValue bool
param message string = ''
output myBoolean bool = returnValue
output myMessage string = message
*/

module return2 'temp2.bicep' = {
  name: 'return2'
  params: {
    returnValue: return1.outputs.myBoolean

    // 3 listKeys is always executed, even though myBoolean is false
    // uncomment next line to test 🟧
    // message: return1.outputs.myBoolean ? SA.listKeys().keys[0].value : 'message'
  }
}

output info string = info
output info2 string = info2
output myBool bool = return2.outputs.myBoolean
output myMessage string = return2.outputs.myMessage

image

module return2 'temp2.bicep' = {
  name: 'return2'
  params: {
    returnValue: return1.outputs.myBoolean

    // 3 listKeys is always executed, even though myBoolean is false
    message: return1.outputs.myBoolean ? SA.listKeys().keys[0].value : 'message'
  }
}

There are workarounds for this, however is it possible to review that fixing this would need to be a breaking change in ARM?

// 🟩
"value": "[if(true(), 'info', listKeys(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2022-09-01').keys[0].value)]"

// 🟩
"value": "[if(false(), listKeys(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2022-09-01').keys[0].value, 'info2')]"

// 🟧
"value": "[if(reference(resourceId('Microsoft.Resources/deployments', 'return1'), '2022-09-01').outputs.myBoolean.value, listKeys(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2022-09-01').keys[0].value, 'message')]"

brwilkinson avatar Apr 25 '23 01:04 brwilkinson

@alex-frankel are there any updates on this?

gogbg avatar Jan 11 '24 15:01 gogbg

While I'm seeing this still open, I have found an appropriate workaround for my use-case which may help others - to wrap listKeys in a helper module. This allows the function to be blocked appropriately by dependsOn

main.bicep

module listKeysHelper'./list-keys-helper.bicep' = {
  name: 'list-keys-helper-example'
  params: {
    resourceId: resourceId(<your resource id reference>)
    apiVersion: '2022-10-01-preview'
  }
  dependsOn: [
    otherModule, otherResource
  ]
}]

resource resourceNeedingKeys 'Microsoft.blah/blah@2024-07-02' = {
  name: 'example'
  properties: {
    someSecret: listKeysHelper.outputs.keys.primaryConnectionString
  }
}

list-keys-helper.bicep

targetScope = 'subscription'

@description('Resource ID of the resource for which keys are to be fetched.')
param resourceId string

@description('API version of the resource for which keys are to be fetched.')
param apiVersion string

var keys = listKeys(resourceId, apiVersion)

output keys object = keys

carbage avatar Jul 02 '24 09:07 carbage

While I'm seeing this still open, I have found an appropriate workaround for my use-case which may help others - to wrap listKeys in a helper module. This allows the function to be blocked appropriately by dependsOn

main.bicep

module listKeysHelper'./list-keys-helper.bicep' = {
  name: 'list-keys-helper-example'
  params: {
    resourceId: resourceId(<your resource id reference>)
    apiVersion: '2022-10-01-preview'
  }
  dependsOn: [
    otherModule, otherResource
  ]
}]

resource resourceNeedingKeys 'Microsoft.blah/blah@2024-07-02' = {
  name: 'example'
  properties: {
    someSecret: listKeysHelper.outputs.keys.primaryConnectionString
  }
}

list-keys-helper.bicep

targetScope = 'subscription'

@description('Resource ID of the resource for which keys are to be fetched.')
param resourceId string

@description('API version of the resource for which keys are to be fetched.')
param apiVersion string

var keys = listKeys(resourceId, apiVersion)

output keys object = keys

The detail here is that you're outputting sensitive values from your helper module.

https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/scenarios-secrets#avoid-outputs-for-secrets

Don't use Bicep outputs for secure data. Outputs are logged to the deployment history, and anyone with access to the deployment can view the values of a deployment's outputs.

fvilches17 avatar Jul 12 '24 04:07 fvilches17

@fvilches17 You're absolutely correct, as we can't mark outputs with secure(). My use-case is pushing connection strings to key vaults via loosely coupled modules - the solution has now changed to grabbing the connection string within the service bus module and taking inputs there to (optionally) push the secret as part of the accessKey resource creation.

carbage avatar Jul 12 '24 08:07 carbage

FWIW, we will be enabling @secure() decorator on outputs as part of #2163 that is in progress, so that may make this module a more viable workaround once that is available. Of course, we hope to fix the underlying bug too, but have less precise ETAs on that.

We hope to have it done by the end of the summer. cc @Andrewegao as FYI

alex-frankel avatar Jul 15 '24 17:07 alex-frankel

@alex-frankel are you able to provide an imprecise ETA on this issue?

Canuteson avatar Jul 31 '24 19:07 Canuteson

Is this issue fundamental to ARM, or only as an aspect of Bicep's abstraction layer over it? If it's a Bicep problem, can we resolve it by dropping into ARM for this particular functionality and invoking listKeys in a way that works? It doesn't seem credible that ARM being many years old would have such an obvious blocker to a very common, essential workflow - but I can't see how one would get the behaviour expected from ARM that presently isn't available in Bicep.

TWolversonReply avatar Aug 15 '24 18:08 TWolversonReply