Azure SQL Database configuration parameters and module#273
Azure SQL Database configuration parameters and module#273ramirezmorac2 wants to merge 1 commit intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Azure SQL Database provisioning to the existing infra/main.bicep deployment, including new parameters to configure SQL server/database naming and access settings.
Changes:
- Introduces new Azure SQL parameters (server/database name, admin credentials, public network access).
- Adds a new
sqlServerBicep module invocation to deploy an Azure SQL Server + Database and store the admin password in Key Vault. - Exposes SQL server/database names as deployment outputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
infra/main.parameters.json |
Adds parameter mappings for Azure SQL configuration values. |
infra/main.bicep |
Adds Azure SQL params/vars, deploys the SQL module, and outputs SQL resource names. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| module sqlServer './core/db/sqlserver.bicep' = { |
There was a problem hiding this comment.
PR description still contains placeholders (e.g., "[Describe your changes here]") and the checklist isn’t filled out. Please update the description with the intended behavior/rollout notes and confirm test/validation steps for the new Azure SQL deployment.
| "value": "${AZURE_DB_DATABASE_NAME}" | ||
| }, | ||
| "azureSqlServerName": { | ||
| "value": "" |
There was a problem hiding this comment.
azureSqlServerName is hard-coded to an empty string. This is inconsistent with other resource name parameters (which are populated from env vars) and prevents persisting the SQL server name across azd env refresh / redeploys. Consider mapping this to an env var (e.g., AZURE_SQL_SERVER_NAME, optionally with an empty default).
| "value": "" | |
| "value": "${AZURE_SQL_SERVER_NAME}" |
| "value": "" | ||
| }, | ||
| "azureSqlDatabaseName": { | ||
| "value": "" |
There was a problem hiding this comment.
azureSqlDatabaseName is hard-coded to an empty string. Like azureSqlServerName, this should likely be mapped from an env var (e.g., AZURE_SQL_DATABASE_NAME) so the chosen/generated name is stable across refresh/redeploys.
| "value": "" | |
| }, | |
| "azureSqlDatabaseName": { | |
| "value": "" | |
| "value": "${AZURE_SQL_SERVER_NAME}" | |
| }, | |
| "azureSqlDatabaseName": { | |
| "value": "${AZURE_SQL_DATABASE_NAME}" |
| administratorLoginPassword: azureSqlAdministratorPassword | ||
| databaseName: sqlDatabaseName | ||
| keyVaultName: keyVault.outputs.name | ||
| publicNetworkAccess: azureSqlPublicNetworkAccess |
There was a problem hiding this comment.
networkIsolation is used elsewhere to disable public access and configure private endpoints, but the new SQL module is always deployed and doesn’t follow that pattern (no conditional public access / private endpoint wiring here). In a network-isolated deployment this can undermine isolation if public access stays enabled, or break connectivity if it’s disabled without private link. Consider aligning SQL with the existing networkIsolation approach or making SQL deployment conditional based on networkIsolation.
| publicNetworkAccess: azureSqlPublicNetworkAccess | |
| publicNetworkAccess: networkIsolation ? 'Disabled' : azureSqlPublicNetworkAccess |
| administratorLoginPassword: azureSqlAdministratorPassword | ||
| databaseName: sqlDatabaseName | ||
| keyVaultName: keyVault.outputs.name | ||
| publicNetworkAccess: azureSqlPublicNetworkAccess |
There was a problem hiding this comment.
The new sqlServer module invocation here deploys ./core/db/sqlserver.bicep, which defines a Microsoft.Sql/servers/firewallRules resource named AllowAllAzureServices with startIpAddress set to 0.0.0.0 and endIpAddress set to 255.255.255.255. Combined with publicNetworkAccess defaulting to Enabled both here (azureSqlPublicNetworkAccess) and in the module, this effectively exposes the Azure SQL Server to all IPv4 addresses on the public internet, significantly increasing the risk of unauthorized access or brute-force attacks against the administratorLogin. To mitigate this, tighten the SQL firewall to only trusted IP ranges or disable publicNetworkAccess and use private endpoints/network isolation, and remove or replace the "allow all" firewall rule in sqlserver.bicep with a more restrictive configuration.
| publicNetworkAccess: azureSqlPublicNetworkAccess | |
| publicNetworkAccess: 'Disabled' |
JIRA Ticket
PROJ-XXX
Description
[Describe your changes here]
Checklist