Skip to content

Azure SQL Database configuration parameters and module #274

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.

Sign up for GitHub

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

Closed
ramirezmorac2 wants to merge 3 commits into from
Closed

Azure SQL Database configuration parameters and module #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac0e3e9
feat: add Azure SQL Database configuration parameters and module
ramirezmorac2 Feb 19, 2026
7f3dcc9
feat: set default location for SQL Server to 'eastus2'
ramirezmorac2 Feb 19, 2026
82d843e
feat: update Azure SQL Database parameters for flexibility in configu…
ramirezmorac2 Feb 19, 2026
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
44 changes: 40 additions & 4 deletions infra/main.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,27 @@ var dbAccountName = !empty(azureDbAccountName) ? azureDbAccountName : 'dbgpt0-${
@description('Cosmos DB Database Name. Use your own name convention or leave as it is to generate a random name.')
param azureDbDatabaseName string = ''
var dbDatabaseName = !empty(azureDbDatabaseName) ? azureDbDatabaseName : 'db0-${resourceToken}'
@description('Azure SQL Server Name.')
param azureSqlServerName string = ''
var sqlServerName = !empty(azureSqlServerName) ? azureSqlServerName : 'sqlgpt0-${resourceToken}'
@description('Azure SQL Database Name.')
param azureSqlDatabaseName string = ''
var sqlDatabaseName = !empty(azureSqlDatabaseName) ? azureSqlDatabaseName : 'sqldb0-${resourceToken}'
@description('Azure SQL administrator login name.')
param azureSqlAdministratorLogin string = ''
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The azureSqlAdministratorLogin parameter defaults to an empty string but is required by the SQL Server resource. If the AZURE_SQL_ADMIN_LOGIN environment variable is not set, this will cause a deployment failure.

Consider either:

  1. Providing a generated default value (e.g., 'sqladmin-${resourceToken}') to match the pattern used for other resources, or
  2. Removing the default value to make it explicitly required, which would cause validation to fail early with a clear error message

This prevents silent failures during deployment and improves the developer experience.

Suggested change
param azureSqlAdministratorLogin string = ''
param azureSqlAdministratorLogin string = 'sqladmin-${resourceToken}'

Copilot uses AI. Check for mistakes.
@description('Azure SQL administrator password.')
@secure()
param azureSqlAdministratorPassword string
@description('Key Vault secret name to store the Azure SQL administrator password.')
param azureSqlAdminSecretName string = ''
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The azureSqlAdminSecretName parameter defaults to an empty string but is required for storing the SQL administrator password in Key Vault. If the AZURE_SQL_ADMIN_SECRET_NAME environment variable is not set, this will cause a deployment failure when the sqlserver module attempts to create the Key Vault secret.

Consider either:

  1. Providing a generated default value (e.g., 'sql-admin-password') to match the pattern used for other secret names in the codebase (see 'azureDBkey' at line 913, 'azureOpenAIKey' at line 1868, 'azureSearchKey' at line 1927), or
  2. Removing the default value to make it explicitly required

This prevents silent failures during deployment and maintains consistency with how other database secrets are named.

Suggested change
param azureSqlAdminSecretName string = ''
param azureSqlAdminSecretName string = 'sql-admin-password'

Copilot uses AI. Check for mistakes.
@description('Public network access setting for Azure SQL Server.')
@allowed(['Enabled', 'Disabled'])
param azureSqlPublicNetworkAccess string = 'Enabled'
@description('Log Analytics Workspace Name. Use your own name convention or leave as it is to generate a random name.')
param azureLogAnalyticsWorkspaceName string = ''
var logAnalyticsWorkspaceName = !empty(azureLogAnalyticsWorkspaceName) ? azureLogAnalyticsWorkspaceName : 'law0-${resourceToken}'
var logAnalyticsWorkspaceName = !empty(azureLogAnalyticsWorkspaceName)
? azureLogAnalyticsWorkspaceName
: 'law0-${resourceToken}'
@description('Enable PartitionKeyRUConsumption logs for multi-tenant billing')
param enablePartitionKeyRUConsumption bool = true
@description('Key Vault Name. Use your own name convention or leave as it is to generate a random name.')
Expand Down Expand Up @@ -954,6 +972,22 @@ module keyVault './core/security/keyvault.bicep' = {
}
}

module sqlServer './core/db/sqlserver.bicep' = {
name: 'sqlserver'
scope: resourceGroup
params: {
name: sqlServerName
location: 'eastus2'
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The hard-coded location 'eastus2' may cause deployment issues if the primary deployment location specified in the location parameter is different. This creates a potential mismatch where the SQL Server is deployed to a different region than other resources.

While hard-coded locations are used in this codebase for specific services with regional availability constraints (e.g., visionIngestion uses 'westus', o1Deployment uses 'eastus2'), consider whether this SQL Server deployment has similar constraints. If not, using the location parameter would provide more flexibility and keep resources colocated.

If the hard-coded location is intentional due to service availability, add a comment explaining the reason (e.g., '//Workaround for service availability') to maintain consistency with the existing pattern at line 897.

Suggested change
location: 'eastus2'
location: location

Copilot uses AI. Check for mistakes.
tags: tags
administratorLogin: azureSqlAdministratorLogin
administratorLoginPassword: azureSqlAdministratorPassword
databaseName: sqlDatabaseName
keyVaultName: keyVault.outputs.name
publicNetworkAccess: azureSqlPublicNetworkAccess
secretName: azureSqlAdminSecretName
}
}
Comment on lines +975 to +989
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The SQL Server module is deployed but the connection information (server name, database name, connection string) is not being passed to any application components (orchestrator, frontend, dataIngestion, mcpServer).

The outputs AZURE_SQL_SERVER_NAME and AZURE_SQL_DATABASE_NAME are exposed at lines 2155-2156, but no application settings are configured to use these values. This suggests the SQL Server may not be fully integrated.

If this SQL Server is intended to be used by the application, consider adding the necessary connection information to the appSettings of the relevant modules. For example:

  • SQL_SERVER_NAME or DATABASE_SERVER
  • SQL_DATABASE_NAME or DATABASE_NAME
  • SQL_CONNECTION_STRING (retrieving the password from Key Vault)

If the SQL Server is not yet intended to be used and this is preparatory infrastructure, consider making the module deployment conditional to avoid unnecessary resource costs.

Copilot uses AI. Check for mistakes.
Comment on lines +975 to +989
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

When network isolation is enabled, other database resources (like Cosmos DB at line 918) have private endpoints configured with corresponding DNS zones. However, no private endpoint is configured for the SQL Server.

If network isolation is a feature that should apply to all database services consistently, consider adding:

  1. A SQL DNS zone module (similar to blobDnsZone, documentsDnsZone, etc.)
  2. A private endpoint module for SQL Server (similar to cosmospe at line 918)

This ensures consistent network security across all database resources when networkIsolation is enabled. If SQL Server doesn't require network isolation by design, the publicNetworkAccess parameter can handle this, but the inconsistency with other databases should be documented.

Copilot uses AI. Check for mistakes.

module keyvaultpe './core/network/private-endpoint.bicep' = if (networkIsolation) {
name: 'keyvaultpe'
scope: resourceGroup
Expand Down Expand Up @@ -1523,7 +1557,7 @@ module frontEnd 'core/host/appservice.bicep' = {
{
name: 'USER_FEEDBACK_URL'
value: userFeedbackUrl
}
}
{
name: 'ANTHROPIC_API_KEY'
value: orchestratorAnthropicApiKeyVar
Expand Down Expand Up @@ -1649,7 +1683,7 @@ module dataIngestion './core/host/functions.bicep' = {
value: 'text-embedding-3-small'
}
{
name:'FORM_REC_API_VERSION'
name: 'FORM_REC_API_VERSION'
value: '2024-11-30'
}
{
Expand Down Expand Up @@ -1685,7 +1719,7 @@ module dataIngestion './core/host/functions.bicep' = {
value: 'INFO'
}
{
name:'COGNITIVE_SERVICES_KEY'
name: 'COGNITIVE_SERVICES_KEY'
value: cognitiveServices.outputs.key
}
{
Expand Down Expand Up @@ -2118,6 +2152,8 @@ output AZURE_RESOURCE_GROUP_NAME string = azureResourceGroupName
output AZURE_NETWORK_ISOLATION bool = networkIsolation
output AZURE_DB_ACCOUNT_NAME string = azureDbAccountName
output AZURE_DB_DATABASE_NAME string = azureDbDatabaseName
output AZURE_SQL_SERVER_NAME string = sqlServerName
output AZURE_SQL_DATABASE_NAME string = sqlDatabaseName
output AZURE_STORAGE_ACCOUNT_NAME string = storageAccountName
output AZURE_COGNITIVE_SERVICE_NAME string = azureCognitiveServiceName
output AZURE_APP_SERVICE_PLAN_NAME string = azureAppServicePlanName
Expand Down
18 changes: 18 additions & 0 deletions infra/main.parameters.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@
"azureDbDatabaseName": {
"value": "${AZURE_DB_DATABASE_NAME}"
},
"azureSqlServerName": {
"value": ""
},
"azureSqlDatabaseName": {
"value": ""
Comment on lines +54 to +57
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The azureSqlServerName and azureSqlDatabaseName parameters have empty string values, which differs from the pattern used for other similar resources in this file.

Compare with:

  • azureDbAccountName (line 47-48): uses environment variable ${AZURE_DB_ACCOUNT_NAME}
  • azureStorageAccountName (line 80-81): uses environment variable ${AZURE_STORAGE_ACCOUNT_NAME}
  • azureCognitiveServiceName (line 83-84): uses environment variable ${AZURE_COGNITIVE_SERVICE_NAME}

Consider using environment variables like ${AZURE_SQL_SERVER_NAME} and ${AZURE_SQL_DATABASE_NAME} to maintain consistency with other resource naming patterns. This allows users to specify custom names when needed, while the bicep file's default value generation (lines 411 and 414 in main.bicep) will be used when the environment variables are empty.

Suggested change
"value": ""
},
"azureSqlDatabaseName": {
"value": ""
"value": "${AZURE_SQL_SERVER_NAME}"
},
"azureSqlDatabaseName": {
"value": "${AZURE_SQL_DATABASE_NAME}"

Copilot uses AI. Check for mistakes.
},
"azureSqlAdministratorLogin": {
"value": "${AZURE_SQL_ADMIN_LOGIN}"
},
"azureSqlAdministratorPassword": {
"value": "$(secretOrRandomPassword ${AZURE_KEY_VAULT_NAME} ${AZURE_SQL_ADMIN_SECRET_NAME})"
},
"azureSqlAdminSecretName": {
"value": "${AZURE_SQL_ADMIN_SECRET_NAME}"
},
"azureSqlPublicNetworkAccess": {
"value": "${AZURE_SQL_PUBLIC_NETWORK_ACCESS=Enabled}"
},
"azureLogAnalyticsWorkspaceName": {
"value": "${AZURE_LOG_ANALYTICS_WORKSPACE_NAME}"
},
Expand Down