Azure / bicep

Bicep is a declarative language for describing and deploying Azure resources
MIT License
3.16k stars 729 forks source link

Bicep linter/static-analysis is not recognizing error scenarios related to resource name #5598

Open asilverman opened 2 years ago

asilverman commented 2 years ago

Bicep version

Bicep CLI version 0.4.1124 (66c84c8ee5)

Describe the bug

I was recently attempting to instantiate a bicep resource with a randomly generated name for the purpose of my ci testing process. The name of the resource is constrained (as a Azure policy) to start with a letter.

I inadvertently set the name using uniqueString(), however it turns out this function doesn't guarantee that the result will start with a letter, and the bicep vscode extension didn't warn me about this fact while authoring the resource.

This results in me creating a deployment that will periodically fail introducing flakiness to my CI environment since every now and then (given that I am recreating the environment on each build) I may get a uniqueString that starts with a number resulting in a bad request error.

I am curious if there's a way to have the vscode linter/static-analyzer detect this during authoring since its unreasonable to expect authors to "just know" what are the constraints on a particular azure resource name.

Here is the resource I was setting:

resource key_vault 'Microsoft.KeyVault/vaults@2021-04-01-preview' = {
name: uniqueString('kv', resourceGroup().id)
location: resourceGroup().location
properties: {
enabledForTemplateDeployment: true
tenantId: subscription().tenantId
enableRbacAuthorization:true
sku: {
name: 'standard'
family: 'A'
}
}

Here is the sporadic error I would get every now and then:

Error: Code="DeploymentFailed" Message="At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/DeployOperations for usage details." Details=[{"code":"BadRequest","message":"{\r\n \"error\": {\r\n \"code\": \"VaultNameNotValid\",\r\n \"message\": \"The vault name '2d6lducplqzgu' is invalid. A vault's name must be between 3-24 alphanumeric characters. The name must begin with a letter, end with a letter or digit, and not contain consecutive hyphens. Follow this link for more information: https://go.microsoft.com/fwlink/?linkid=2147742\"\r\n }\r\n}"}]

Here is how I ended up fixing the issue:

resource key_vault 'Microsoft.KeyVault/vaults@2021-04-01-preview' = {
name: 'kv-${uniqueString('kv', resourceGroup().id)}'
location: resourceGroup().location
properties: {
enabledForTemplateDeployment: true
tenantId: subscription().tenantId
enableRbacAuthorization:true
sku: {
name: 'standard'
family: 'A'
}
}

Expected Behavior I would expect the Bicep VSCode extension to underline the stanza name: uniqueString('foo') with a warning or an error that indicates that doing this is unsafe.

alex-frankel commented 2 years ago

Today, naming requirements are not a part of our resource type definitions, though they are sometimes present in swagger, so it would be possible to use that info in our type system. I think we'd also need to add some type information about the type of string that is returned (i.e. string that could start with letter or number).

Related: #1679

anthony-c-martin commented 2 years ago

One fairly straightforward possibility is to include the naming regex in the generated description during type generation.

e.g. here: https://github.com/Azure/azure-rest-api-specs/blob/0bca06c5e06c6b12ea8e560c7416a5968136e276/specification/keyvault/resource-manager/Microsoft.KeyVault/stable/2021-10-01/keyvault.json#L34