Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .github/instructions/bicep.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
description: 'Infrastructure as Code with Bicep'
applyTo: '**/*.bicep'
---

# Bicep best-practices
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list of best-practices builds on top of information available at https://learn.microsoft.com/azure/azure-resource-manager/bicep. It provides a more opinionated and up-to-date set of rules for generating high-quality Bicep code. You should aim to follow these rules whenever generating or modifying Bicep code.

## Rules
### General
1. Avoid setting the `name` field for `module` statements - it is no longer required.
1. If you need to input or output a set of logically-grouped values, generate a single `param` or `output` statement with a User-defined type instead of emitting a `param` or `output` statement for each value.
1. If generating parameters, default to generating Bicep parameters files (`*.bicepparam`), instead of ARM parameters files (`*.json`).

### Resources
1. Do not add references from child resources to parent resources by using `/` characters in the child resource `name` property. Instead, use the `parent` property with a symbolic reference to the parent resource.
1. If you are generating a child resource type, sometimes this may require you to add an `existing` resource for the parent if the parent is not already present in the file.
1. If you see diagnostic codes `BCP036`, `BCP037` or `BCP081`, this may indicate you have hallucinated resource types or resource type properties. You may need to double-check against available resource type schema to tune your output.
1. Avoid using multiple `resourceId()` functions and `reference()` function where possible. Instead use symbolic names to refer to ids or properties, creating `existing` resources if needed. For example, write `foo.id` or `foo.properties.id`, instead of `resourceId('...')` or `reference('...').id`.

### Types
1. Avoid using open types such as `array` or `object` when referencing types where possible (e.g. in `output` or `param` statements). Instead, use User-defined types to define a more precise type.
1. Use typed variables instead of untyped variables when exporting values with the `@export()` decorator. For example, use `var foo string = 'blah'` instead of `var foo = bar`.
1. When using User-defined types, aim to avoid repetition, and comment properties with `@description()` where the context is unclear.
1. If you are passing data directly to or from a resource body via a `param` or `output` statement, try to use existing Resource-derived types (`resourceInput<'type@version'>` and `resourceOutput<'type@version'>`) instead of writing User-defined types.

### Security
1. When generating `param` or `output` statements, ALWAYS use the `@secure()` decorator if sensitive data is present.

### Syntax
1. If you hit warnings or errors with null properties, prefer solving them with the safe-dereference (`.?`) operator, in conjunction with the coalesce (`??`) operator. For example, `a.?b ?? c` is better than `a!.b` which may cause runtime errors, or `a != null ? a.b : c` which is unnecessarily verbose.
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The newly added Bicep best practices guidelines in .github/instructions/bicep.instructions.md (line 31) recommend using safe-dereference (.?) with coalesce (??) operators instead of the null-forgiving operator (!). For example, vision.?outputs.endpoint ?? '' would be preferred over vision!.outputs.endpoint. However, throughout this PR, the null-forgiving operator is used extensively.

While the null-forgiving operator usage appears safe in these cases (since the modules are only accessed when the same condition that creates them is true), following the documented best practice would make the code more robust and align with the guidelines being introduced.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed and think that null-forgiving is better in this case since we'd like the runtime error instead.


## Glossary
* Child resource: an Azure resource type with type name consisting of more than 1 `/` characters. For example, `Microsoft.Network/virtualNetworks/subnets` is a child resource. `Microsoft.Network/virtualNetworks` is not.
7 changes: 0 additions & 7 deletions docs/deploy_existing.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ You should set these values before running `azd up`. Once you've set them, retur
* [OpenAI resource](#openai-resource)
* [Azure AI Search resource](#azure-ai-search-resource)
* [Azure App Service Plan and App Service resources](#azure-app-service-plan-and-app-service-resources)
* [Azure Application Insights and related resources](#azure-application-insights-and-related-resources)
* [Azure AI Vision resources](#azure-ai-vision-resources)
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation section for configuring existing Azure Application Insights resources was removed, but the parameters AZURE_APPLICATION_INSIGHTS, AZURE_APPLICATION_INSIGHTS_DASHBOARD, and AZURE_LOG_ANALYTICS (corresponding to applicationInsightsName, applicationInsightsDashboardName, and logAnalyticsName in main.bicep) are still present and functional. Users may still need this documentation to understand how to use existing Application Insights resources with this template.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but we dont have an obvious place to document them at the moment.

* [Azure Document Intelligence resource](#azure-document-intelligence-resource)
* [Azure Speech resource](#azure-speech-resource)
Expand Down Expand Up @@ -72,12 +71,6 @@ You can also customize the search service (new or existing) for non-English sear
1. Run `azd env set AZURE_APP_SERVICE {Name of existing Azure App Service}`.
1. Run `azd env set AZURE_APP_SERVICE_SKU {SKU of Azure App Service, defaults to B1}`.

## Azure Application Insights and related resources
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this section, as these variables do not actually use an existing resource, they just name a new resource.


1. Run `azd env set AZURE_APPLICATION_INSIGHTS {Name of existing Azure App Insights}`.
1. Run `azd env set AZURE_APPLICATION_INSIGHTS_DASHBOARD {Name of existing Azure App Insights Dashboard}`.
1. Run `azd env set AZURE_LOG_ANALYTICS {Name of existing Azure Log Analytics Workspace Name}`.

## Azure AI Vision resources

1. Run `azd env set AZURE_VISION_SERVICE {Name of existing Azure AI Vision Service Name}`
Expand Down
38 changes: 16 additions & 22 deletions infra/backend-dashboard.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ param applicationInsightsName string
param location string = resourceGroup().location
param tags object = {}

// 2020-09-01-preview because that is the latest valid version
resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-preview' = {
name: name
location: location
Expand All @@ -21,7 +20,9 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
colSpan: 2
rowSpan: 1
}
metadata: {
// The Bicep schema only defines MarkdownPart, so we use any() to bypass
// type checking for other valid extension types (AppInsightsExtension, MonitorChartPart)
metadata: any({
inputs: [
{
name: 'id'
Expand All @@ -32,14 +33,13 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
value: '1.0'
}
]
#disable-next-line BCP036
type: 'Extension/AppInsightsExtension/PartType/AspNetOverviewPinnedPart'
asset: {
idInputName: 'id'
type: 'ApplicationInsights'
}
defaultMenuItemId: 'overview'
}
})
}
{
position: {
Expand All @@ -48,7 +48,7 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
colSpan: 1
rowSpan: 1
}
metadata: {
metadata: any({
inputs: [
{
name: 'ComponentId'
Expand All @@ -63,14 +63,13 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
value: '1.0'
}
]
#disable-next-line BCP036
type: 'Extension/AppInsightsExtension/PartType/ProactiveDetectionAsyncPart'
asset: {
idInputName: 'ComponentId'
type: 'ApplicationInsights'
}
defaultMenuItemId: 'ProactiveDetection'
}
})
}
{
position: {
Expand All @@ -79,7 +78,7 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
colSpan: 1
rowSpan: 1
}
metadata: {
metadata: any({
inputs: [
{
name: 'ComponentId'
Expand All @@ -105,13 +104,12 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
value: '78ce933e-e864-4b05-a27b-71fd55a6afad'
}
]
#disable-next-line BCP036
type: 'Extension/AppInsightsExtension/PartType/AppMapButtonPart'
asset: {
idInputName: 'ComponentId'
type: 'ApplicationInsights'
}
}
})
}
{
position: {
Expand Down Expand Up @@ -141,7 +139,7 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
colSpan: 1
rowSpan: 1
}
metadata: {
metadata: any({
inputs: [
{
name: 'ResourceId'
Expand All @@ -167,15 +165,14 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
isOptional: true
}
]
#disable-next-line BCP036
type: 'Extension/AppInsightsExtension/PartType/CuratedBladeFailuresPinnedPart'
isAdapter: true
asset: {
idInputName: 'ResourceId'
type: 'ApplicationInsights'
}
defaultMenuItemId: 'failures'
}
})
}
{
position: {
Expand Down Expand Up @@ -205,7 +202,7 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
colSpan: 1
rowSpan: 1
}
metadata: {
metadata: any({
inputs: [
{
name: 'ResourceId'
Expand All @@ -231,15 +228,14 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
isOptional: true
}
]
#disable-next-line BCP036
type: 'Extension/AppInsightsExtension/PartType/CuratedBladePerformancePinnedPart'
isAdapter: true
asset: {
idInputName: 'ResourceId'
type: 'ApplicationInsights'
}
defaultMenuItemId: 'performance'
}
})
}
{
position: {
Expand All @@ -248,7 +244,7 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
colSpan: 4
rowSpan: 3
}
metadata: {
metadata: any({
inputs: [
{
name: 'options'
Expand Down Expand Up @@ -306,10 +302,9 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
isOptional: true
}
]
#disable-next-line BCP036
type: 'Extension/HubsExtension/PartType/MonitorChartPart'
settings: {}
}
})
}
{
position: {
Expand All @@ -318,7 +313,7 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
colSpan: 4
rowSpan: 3
}
metadata: {
metadata: any({
inputs: [
{
name: 'options'
Expand Down Expand Up @@ -376,10 +371,9 @@ resource applicationInsightsDashboard 'Microsoft.Portal/dashboards@2020-09-01-pr
isOptional: true
}
]
#disable-next-line BCP036
type: 'Extension/HubsExtension/PartType/MonitorChartPart'
settings: {}
}
})
}
]
}
Expand Down
4 changes: 2 additions & 2 deletions infra/core/ai/hub.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ resource hub 'Microsoft.MachineLearningServices/workspaces@2024-07-01-preview' =
category: 'CognitiveSearch'
authType: 'ApiKey'
isSharedToAll: true
target: 'https://${search.name}.search.windows.net/'
target: 'https://${search!.name}.search.windows.net/'
credentials: {
key: !empty(aiSearchName) ? search.listAdminKeys().primaryKey : ''
key: !empty(aiSearchName) ? search!.listAdminKeys().primaryKey : ''
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ML workspace connection stores a Search admin API key in cleartext (credentials.key) combined with authType: 'ApiKey' and is visible to all (isSharedToAll: true). Any user with access to the project can extract this key and gain full control of the Search service. Prefer using Azure AD with a managed identity or a user-assigned identity and role-based access to Search, or store the key in Key Vault and reference it securely; also set isSharedToAll to false to limit exposure.

Example fix:

connections: {
  aiSearch: {
    category: 'CognitiveSearch'
    authType: 'AAD'
    isSharedToAll: false
    target: 'https://${search.name}.search.windows.net/'
    // Use a managed identity with Search RBAC
  }
}

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fortunately, we do not pass searchServiceName to this module. Also this module will go away soon when I port to Foundry projects and red teaming.

}
}
}
Expand Down
4 changes: 2 additions & 2 deletions infra/core/host/appservice.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ resource appService 'Microsoft.Web/sites@2022-03-01' = {
ENABLE_ORYX_BUILD: string(enableOryxBuild)
},
runtimeName == 'python' ? { PYTHON_ENABLE_GUNICORN_MULTIWORKERS: 'true' } : {},
!empty(applicationInsightsName) ? { APPLICATIONINSIGHTS_CONNECTION_STRING: applicationInsights.properties.ConnectionString } : {},
!empty(keyVaultName) ? { AZURE_KEY_VAULT_ENDPOINT: keyVault.properties.vaultUri } : {})
!empty(applicationInsightsName) ? { APPLICATIONINSIGHTS_CONNECTION_STRING: applicationInsights!.properties.ConnectionString } : {},
!empty(keyVaultName) ? { AZURE_KEY_VAULT_ENDPOINT: keyVault!.properties.vaultUri } : {})
}

resource configLogs 'config' = {
Expand Down
2 changes: 1 addition & 1 deletion infra/core/host/container-app-upsert.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ module app 'container-app.bicep' = {
allowedOrigins: allowedOrigins
external: external
env: concat(envAsArray, envSecrets)
imageName: !empty(imageName) ? imageName : exists ? existingApp.properties.template.containers[0].image : ''
imageName: !empty(imageName) ? imageName : exists ? existingApp!.properties.template.containers[0].image : ''
targetPort: targetPort
serviceBinds: serviceBinds
}
Expand Down
4 changes: 2 additions & 2 deletions infra/core/host/container-app.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ module containerRegistryAccess '../security/registry-access.bicep' = if (usePriv
name: '${deployment().name}-registry-access'
params: {
containerRegistryName: containerRegistryName
principalId: usePrivateRegistry ? userIdentity.properties.principalId : ''
principalId: usePrivateRegistry ? userIdentity!.properties.principalId : ''
}
}

Expand Down Expand Up @@ -174,7 +174,7 @@ resource containerAppsEnvironment 'Microsoft.App/managedEnvironments@2023-05-01'
}

output defaultDomain string = containerAppsEnvironment.properties.defaultDomain
output identityPrincipalId string = normalizedIdentityType == 'None' ? '' : (empty(identityName) ? app.identity.principalId : userIdentity.properties.principalId)
output identityPrincipalId string = normalizedIdentityType == 'None' ? '' : (empty(identityName) ? app.identity.principalId : userIdentity!.properties.principalId)
output identityResourceId string = normalizedIdentityType == 'UserAssigned' ? resourceId('Microsoft.ManagedIdentity/userAssignedIdentities', userIdentity.name) : ''
output imageName string = imageName
output name string = app.name
Expand Down
6 changes: 3 additions & 3 deletions infra/core/host/container-apps-environment.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ resource containerAppsEnvironment 'Microsoft.App/managedEnvironments@2025-02-02-
appLogsConfiguration: {
destination: 'log-analytics'
logAnalyticsConfiguration: {
customerId: logAnalyticsWorkspace.properties.customerId
sharedKey: logAnalyticsWorkspace.listKeys().primarySharedKey
customerId: logAnalyticsWorkspace!.properties.customerId
sharedKey: logAnalyticsWorkspace!.listKeys().primarySharedKey
}
}
daprAIInstrumentationKey: daprEnabled && !empty(applicationInsightsName) ? applicationInsights.properties.InstrumentationKey : ''
daprAIInstrumentationKey: daprEnabled && !empty(applicationInsightsName) ? applicationInsights!.properties.InstrumentationKey : ''
publicNetworkAccess: usePrivateIngress ? 'Disabled' : 'Enabled'
vnetConfiguration: usePrivateIngress ? {
infrastructureSubnetId: subnetResourceId
Expand Down
Loading
Loading