bicep icon indicating copy to clipboard operation
bicep copied to clipboard

Feature request: ability to use GetSecret() in a conditional way

Open Adunaphel opened this issue 4 years ago • 10 comments

In ARM, when I want to use a key vault secret that is not a mandatory part of a deployment, I use the following bit of code so that I can pass the secret only when it is being referenced from the parameters passed, or when it is needed to deploy the resource in question (Linux VM with either a password or an SSH key, for instance) (newlines for better readability):

"[if( empty( parameters( 'resource' ).secretValue.keyVaultName ),
  createObject(
    'value', ''
  ),
  createObject(
    'reference', createObject(
      'keyVault', createObject(
        'id', resourceId(
          parameters( 'resource' ).secretValue.keyVaultSubscriptionId,
          parameters( 'resource' ).secretValue.keyVaultResourceGroup,
          Microsoft.KeyVault/vaults',
          parameters( 'resource' ).secretValue.keyVaultName
        )
      ),
    'secretName', parameters( 'resource' ).secretValue.keyVaultSecretName
    )
  )
)]",

Sadly, Bicep doesn't offer me the same level of flexibility with GetSecret(). It would like to be able to do things like this:

resource secretKeyVault 'Microsoft.KeyVault/vaults@2021-06-01-preview' existing = {
  scope: resourceGroup(resource.secretValue.keyVaultSubscriptionId, resource.secretValue.keyVaultResourceGroup)
  name: resource.secretValue.keyVaultName
}

module subResource 'sub-resource.bicep' = {
  name: 'subResource'
  params: {
    secretValue: !empty(resource.secretValue.keyVaultName) ? secretKeyVault.getSecret(resource.secretValue.keyVaultSecretName) : ''
  }
}

But right now, GetSecret can only be used to directly assign a value to a secureString parameter. So right now I'm stuck creating multiple declarations of modules based on whether or not the secret needs to be passed. Which can balloon quickly when there's multiple optional secrets...

Adunaphel avatar Sep 02 '21 08:09 Adunaphel

+1

slavizh avatar Sep 02 '21 08:09 slavizh

I have a similar problem with the creation of a vm. Normally I want to get a secret for the adminPassword value from keyvault, but there are cases where this is not possible/required so I would like to have the option of reverting to a (@secure()) parameter:

adminPassword: empty(adminKeyVaultName) ? adminPassword : kv.getSecret('mySecretName')

AndrevdG avatar Sep 03 '21 11:09 AndrevdG

Adding some technical context: Here https://github.com/Azure/bicep/blob/57854097106ceb4c6f24f6b0d023001841f3c03e/src/Bicep.Core/Emit/ExpressionEmitter.cs#L361 we’d need to special case ternary syntax and use emit with value only on expressions that are not conditional or key vault reference. And do this as long as ternary syntax is argument of yet another ternary syntax.

miqm avatar Jul 23 '22 19:07 miqm

+1

nduijvelshoff avatar Sep 30 '22 06:09 nduijvelshoff

+1

simonhalby avatar Sep 30 '22 07:09 simonhalby

+1

Vladeexx avatar Sep 30 '22 07:09 Vladeexx

+1

PostSven avatar Sep 30 '22 07:09 PostSven

+1

northynorth avatar Sep 30 '22 08:09 northynorth

+1

bdzeeuw avatar Sep 30 '22 11:09 bdzeeuw

A proposed workaround here that I use in the meantime. The URI to the secret isn't something I'm averse to including in logs, so when I create resources, if a Key Vault name is provided, they store their various secrets in the Key Vault and pass the versioned URI to the secret back via the output.

Because the identity executing the deployment also has read access to the Key Vault secrets, I've got another PowerShell-based module that explicitly just downloads the resource from the secret (optionally versioned) URI (authenticating via that identity) and passes the secret back to the caller.

From there, it's just an exercise of mapping the secret names you want to the key vaults they live in and assigning to the appropriate parameters in downstream resources, all without having to use the built-in getSecret() method.

WhitWaldo avatar Sep 30 '22 22:09 WhitWaldo

@WhitWaldo not working workaround for us as this option works for simple templates. Ours are more advanced. We have other workarounds as well but they are workarounds for something that was and it is is available in ARM and it should be available in Bicep. After all Bicep is suppose to have 100% parity with ARM + even more.

slavizh avatar Oct 03 '22 06:10 slavizh

@slavizh Fair enough, but there are a lot of features Bicep has yet to achieve parity with straight ARM on. Like me, if you still prefer to use Bicep, you might consider restructuring your templates to take an approach than is supported today while waiting for the team to get to your ask, as you might otherwise be waiting a while.

I can't tell you how many Powershell deployments I have in my own collection simply because the equivalent Bicep (and even ARM) functionality isn't yet supported.

WhitWaldo avatar Oct 03 '22 06:10 WhitWaldo

@WhitWaldo thanks for the advise but that is not applicable.

slavizh avatar Oct 03 '22 07:10 slavizh

Discussed this today. There are some options for implementing this only in bicep (not in the deployment runtime), which makes this a feasible ask. @miqm will add some pseudocode on what the implementation might look like.

We need to have one more discussion to see if we can get this done in the next few months, otherwise it will be implemented as part of the 1.0 milestone.

alex-frankel avatar Oct 12 '22 20:10 alex-frankel

yep, my ask was always to be in bicep as it is already possible in ARM and no need to implement at deployment runtime.

slavizh avatar Oct 13 '22 06:10 slavizh

Given that @miqm was able to get the draft PR done quickly, I'm going to optimistically move this to v0.13.

alex-frankel avatar Oct 13 '22 17:10 alex-frankel

A question to people watching this issue.

Currently, when we use getSecret not directly on module param a following message is shown: Function "getSecret" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.

Question is - what error message should it be now? any ideas?

My proposal: Function "getSecret" is not valid at this location. It can only be used when assigning, directly or within a ternary operator, to a module parameter with a secure decorator.

miqm avatar Oct 13 '22 20:10 miqm

The issue at hand has the added complexity that the use of the wording directly is open for interpretation if there is no underlying supporting documentation to define it's use within the language. As such I find that the original error message is only wrong due to the fact that it is feature limited, not necessarily because there is a syntactical error once the feature is implemented.

For example, as long as I assign directly to the module parameter, whether with a reference to the key vault, or a ternary operation that has a key vault reference, it is still being made directly against a module parameter with a secure decorator.

module foo 'resource@apiversion' = {
  name:'name'
  parameters:{
    bar:keyVault.getSecret('key')
  }
}
module foo 'resource@apiversion' = {
  name:'name'
  parameters:{
    bar:boolean ? keyVault.getSecret('key') : 'alternantive option'
  }
}

The above are both directly assigned to the module parameter as that is the subject we are referring to, there is no transitory vessel between the assignment and the module parameter, otherwise surely we are syntactically treating the ternary operator as it's own unique variable instead of the typing and value of the result of the operator which might as well be the same as teh below.

var secret = boolean ? keyVault.getSecret('key') : 'alternantive option'
module foo 'resource@apiversion' = {
  name:'name'
  parameters:{
    bar:secret 
  }
}

I would then consider this to be an indirect assignment as there is a transitory variable that is responsible for holding and passing the value into the parameter which is not secure.

Once the feature to resolve the functional issue is fixed we can then continue to use Function "getSecret" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator. as this accurately displays that the only wrong way to use the reference is when having it somewhere that is not directly in the params block key value assignment.

It goes beyond the scope of this specific issue but this issue comes down to the fact that the error message is fuzzy in its definition, if we look at another language like c#(I appreciate it is a considerably more mature ecosystem so has had time and considerable investment in making this so ) they will have very specific error messages a lot very similar that pick up on a very specific issue. which at the root of this specific issue is a problem because there are two different errors happening here, caught in a single check.

We have an error that can be used to catch if a keyvault method is used in an invalid location, and to catch if a keyvault reference is in a valid location but not in a supported operation. Whilst the specific operation is looking to be resolved in milestone 0.13 we still have two separate errors one which may come up again as (we all hope) the language gains full feature parity and advanced logic over ARM deployments. I would suggest as part of a future Milestone we also consider the following errors being introduced to replace BCP180:

Function "getSecret" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.
Function "getSecret" is not valid in this operation. It can only be used when directly assigning, or within a ternary operator, to a module parameter with a secure decorator.

Codemagedon avatar Oct 21 '22 08:10 Codemagedon