Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
asilverman opened this issue Jan 7, 2022 · 2 comments
Labels
devdiv Related to Bicep tooling efforts in DevDiv good first issue Good for newcomers Needs: Upvote This issue requires more votes to be considered story: linter rules

Comments

@asilverman
Copy link
Contributor

Bicep version

Bicep CLI version 0.4.1124 (66c84c8)

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
Copy link
Collaborator

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 anthony-c-martin added good first issue Good for newcomers Needs: Upvote This issue requires more votes to be considered and removed Needs: Triage 🔍 labels Jan 19, 2022
@anthony-c-martin
Copy link
Member

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

@StephenWeatherford StephenWeatherford added the devdiv Related to Bicep tooling efforts in DevDiv label Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devdiv Related to Bicep tooling efforts in DevDiv good first issue Good for newcomers Needs: Upvote This issue requires more votes to be considered story: linter rules
Projects
None yet
Development

No branches or pull requests

4 participants