Resolve all Bicep warnings

[pamelafox]

Purpose

Our Bicep had various warnings, mostly BCP318 due to possibly null references, which can now be resolved with either the safe-dereference operator or the null-forgiving operator. I resolved most with the latter.

I also removed a dashboard Bicep file that we were never using and that had various warnings.

This should improve developer experience with real Bicep errors, since they won't need to wade through warnings anymore.

Does this introduce a breaking change?

When developers merge from main and run the server, azd up, or azd deploy, will this produce an error?
If you're not sure, try it out on an old environment.

[ ] Yes
[X] No

Does this require changes to learn.microsoft.com docs?

This repository is referenced by this tutorial
which includes deployment, settings and usage instructions. If text or screenshot need to change in the tutorial,
check the box below and notify the tutorial author. A Microsoft employee can do this for you if you're an external contributor.

[ ] Yes
[X] No

Type of change

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

Code quality checklist

See CONTRIBUTING.md for more details.

N/A

I re-ran azd provision to confirm all is working.

[pamelafox]

This is taken from: https://raw.githubusercontent.com/Azure/bicep/refs/heads/main/src/Bicep.McpServer/Files/bestpractices.md

[pamelafox]

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

[Copilot]

Pull request overview

This PR resolves Bicep warnings throughout the infrastructure code, primarily addressing BCP318 warnings related to potentially null references. The changes use the null-forgiving operator (!) to assert that conditionally-created modules will exist when accessed. The PR also removes an unused Application Insights dashboard template file and adds a new Bicep best practices guide for future development.

Key Changes

• Applied null-forgiving operator (!) to 50+ module output references where modules are conditionally created
• Fixed a potential edge case in substring calculation using max() to prevent negative lengths
• Corrected Azure Search hosting mode values from lowercase to PascalCase to match Azure API requirements
• Removed unused applicationinsights-dashboard.bicep file and related references from monitoring.bicep
• Replaced #disable-next-line BCP036 suppressions with any() type casts in backend-dashboard.bicep with explanatory comments
• Added comprehensive Bicep best practices documentation in .github/instructions/bicep.instructions.md

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
infra/main.bicep	Applied null-forgiving operator to conditionally-created module outputs; added max() safeguard to substring calculation
infra/main.test.bicep	Added explicit parameter values for test configuration
infra/core/search/search-services.bicep	Corrected hosting mode allowed values from lowercase to PascalCase
infra/core/monitor/monitoring.bicep	Removed unused dashboard parameter and module reference
infra/core/monitor/applicationinsights-dashboard.bicep	Deleted entire unused file
infra/backend-dashboard.bicep	Replaced BCP036 suppressions with any() casts and added explanatory comments
infra/core/host/container-apps-environment.bicep	Applied null-forgiving operator to conditional resource references
infra/core/host/container-app.bicep	Applied null-forgiving operator to user identity properties
infra/core/host/container-app-upsert.bicep	Applied null-forgiving operator to existing app properties
infra/core/host/appservice.bicep	Applied null-forgiving operator to Application Insights and Key Vault properties
infra/core/ai/hub.bicep	Applied null-forgiving operator to AI Search resource references
docs/deploy_existing.md	Removed Application Insights configuration section from documentation
.github/instructions/bicep.instructions.md	Added new comprehensive Bicep best practices guide

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[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.

[pamelafox]

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

[Copilot]

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.

[pamelafox]

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

[Copilot]

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
  }
}

[pamelafox]

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.

[mattgotteiner]

interesting this was a warning

[pamelafox]

The second arg to substring must be zero or greater, so it flagged that:
https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/bicep-functions-string#substring
I guess the types for the Bicep functions are defined with valid ranges as well?