Bicep support for 'custom' azure environment #551
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive support for custom Azure cloud environments (such as Azure Government, sovereign clouds, or air-gapped environments) to the Bicep deployment infrastructure. The key enhancement is the ability to dynamically detect the Azure cloud environment and configure appropriate endpoints for Microsoft services including Graph API, Authentication, Resource Manager, Cognitive Services, and Azure Search.
Key changes include:
- Dynamic cloud environment detection using
az.environment()instead of hardcoded values - Parameterized OpenAI model configuration for existing OpenAI resources
- Custom environment variable injection for non-standard Azure clouds
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| deployers/bicep/main.bicep | Adds cloud environment detection with 'public', 'usgovernment', and 'custom' options; introduces custom cloud parameters with defaults from az.environment(); exposes OpenAI model parameters for existing resources; fixes imageName split logic |
| deployers/bicep/modules/openAI.bicep | Adds openAIEndpoint output for consistency with existing OpenAI module |
| deployers/bicep/modules/openAI-existing.bicep | Parameterizes chat and embedding model configurations; adds openAIEndpoint output |
| deployers/bicep/modules/appService.bicep | Adds custom environment parameters; changes AZURE_ENDPOINT to AZURE_ENVIRONMENT; removes direct OpenAI resource reference in favor of parameters; injects custom cloud URLs when azurePlatform is 'custom'; uses az.environment() for ACR domain |
| deployers/bicep/modules/appServiceAuthentication.bicep | Replaces hardcoded OpenID issuer URL with dynamic value from az.environment() |
| deployers/bicep/modules/cosmosDb.bicep | Expands from single 'settings' container to explicit definitions for 30+ containers including conversations, messages, documents, agents, actions, and various workspace types (unrelated to PR description) |
| application/single_app/route_backend_users.py | Adds '/v1.0/users' path to CUSTOM_GRAPH_URL_VALUE for consistency with other cloud endpoints |
| application/single_app/config.py | Adds CUSTOM_OIDC_METADATA_URL_VALUE environment variable; sets OIDC_METADATA_URL for custom environment; provides default video_indexer_endpoint for custom clouds; adds KEY_VAULT_DOMAIN configuration |
Comments suppressed due to low confidence (2)
application/single_app/config.py:151
- This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
OIDC_METADATA_URL = f"https://login.microsoftonline.com/{TENANT_ID}/v2.0/.well-known/openid-configuration"
deployers/bicep/modules/cosmosDb.bicep:461
- The
cosmosContributorRoleassignment grants the web app’s managed identity the genericContributorrole on the entirecosmosDbaccount, which provides broad management-plane privileges (including configuration changes, key management, and role assignments) far beyond what is needed for typical data access. If the web app or its managed identity is compromised, an attacker could leverage this over-privileged role to modify or delete the Cosmos DB account or escalate their access by granting additional roles. Restrict this identity to a least-privileged Cosmos DB data role (for example, data contributor scoped to the required database/containers) and avoid assigning the generalContributorrole at the account scope unless the application truly requires full management capabilities.
resource cosmosContributorRole 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
name: guid(cosmosDb.id, managedIdentityId, 'cosmos-contributor')
scope: cosmosDb
properties: {
roleDefinitionId: subscriptionResourceId(
'Microsoft.Authorization/roleDefinitions',
'b24988ac-6180-42a0-ab88-20f7382dd24c'
)
principalId: managedIdentityPrincipalId
principalType: 'ServicePrincipal'
}
}
| // Container definitions | ||
| resource conversationsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'conversations' | ||
| properties: { | ||
| resource: { | ||
| id: 'conversations' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource messagesContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'messages' | ||
| properties: { | ||
| resource: { | ||
| id: 'messages' | ||
| partitionKey: { | ||
| paths: ['/conversation_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource settingsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'settings' | ||
| properties: { | ||
| resource: { | ||
| id: 'settings' | ||
| partitionKey: { | ||
| paths: [ | ||
| '/id' | ||
| ] | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource groupsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'groups' | ||
| properties: { | ||
| resource: { | ||
| id: 'groups' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource publicWorkspacesContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'public_workspaces' | ||
| properties: { | ||
| resource: { | ||
| id: 'public_workspaces' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource documentsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'documents' | ||
| properties: { | ||
| resource: { | ||
| id: 'documents' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource groupDocumentsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'group_documents' | ||
| properties: { | ||
| resource: { | ||
| id: 'group_documents' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource publicDocumentsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'public_documents' | ||
| properties: { | ||
| resource: { | ||
| id: 'public_documents' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource userSettingsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'user_settings' | ||
| properties: { | ||
| resource: { | ||
| id: 'user_settings' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource safetyContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'safety' | ||
| properties: { | ||
| resource: { | ||
| id: 'safety' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource feedbackContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'feedback' | ||
| properties: { | ||
| resource: { | ||
| id: 'feedback' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource archivedConversationsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'archived_conversations' | ||
| properties: { | ||
| resource: { | ||
| id: 'archived_conversations' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource archivedMessagesContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'archived_messages' | ||
| properties: { | ||
| resource: { | ||
| id: 'archived_messages' | ||
| partitionKey: { | ||
| paths: ['/conversation_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource promptsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'prompts' | ||
| properties: { | ||
| resource: { | ||
| id: 'prompts' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource groupPromptsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'group_prompts' | ||
| properties: { | ||
| resource: { | ||
| id: 'group_prompts' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource publicPromptsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'public_prompts' | ||
| properties: { | ||
| resource: { | ||
| id: 'public_prompts' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource fileProcessingContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'file_processing' | ||
| properties: { | ||
| resource: { | ||
| id: 'file_processing' | ||
| partitionKey: { | ||
| paths: ['/document_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource personalAgentsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'personal_agents' | ||
| properties: { | ||
| resource: { | ||
| id: 'personal_agents' | ||
| partitionKey: { | ||
| paths: ['/user_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource personalActionsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'personal_actions' | ||
| properties: { | ||
| resource: { | ||
| id: 'personal_actions' | ||
| partitionKey: { | ||
| paths: ['/user_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource groupMessagesContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'group_messages' | ||
| properties: { | ||
| resource: { | ||
| id: 'group_messages' | ||
| partitionKey: { | ||
| paths: ['/conversation_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource groupConversationsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'group_conversations' | ||
| properties: { | ||
| resource: { | ||
| id: 'group_conversations' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource groupAgentsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'group_agents' | ||
| properties: { | ||
| resource: { | ||
| id: 'group_agents' | ||
| partitionKey: { | ||
| paths: ['/group_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource groupActionsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'group_actions' | ||
| properties: { | ||
| resource: { | ||
| id: 'group_actions' | ||
| partitionKey: { | ||
| paths: ['/group_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource globalAgentsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'global_agents' | ||
| properties: { | ||
| resource: { | ||
| id: 'global_agents' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource globalActionsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'global_actions' | ||
| properties: { | ||
| resource: { | ||
| id: 'global_actions' | ||
| partitionKey: { | ||
| paths: ['/id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource agentFactsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'agent_facts' | ||
| properties: { | ||
| resource: { | ||
| id: 'agent_facts' | ||
| partitionKey: { | ||
| paths: ['/scope_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource searchCacheContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'search_cache' | ||
| properties: { | ||
| resource: { | ||
| id: 'search_cache' | ||
| partitionKey: { | ||
| paths: ['/user_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } | ||
|
|
||
| resource activityLogsContainer 'Microsoft.DocumentDB/databaseAccounts/sqlDatabases/containers@2023-04-15' = { | ||
| parent: cosmosDatabase | ||
| name: 'activity_logs' | ||
| properties: { | ||
| resource: { | ||
| id: 'activity_logs' | ||
| partitionKey: { | ||
| paths: ['/user_id'] | ||
| } | ||
| } | ||
| options: {} | ||
| } | ||
| } |
There was a problem hiding this comment.
This pull request significantly expands the Cosmos DB container definitions from a single 'settings' container to 30+ specific containers (conversations, messages, settings, groups, documents, etc.). However, this substantial infrastructure change is not mentioned in the PR description, which focuses on custom Azure environment support. This appears to be a separate feature or fix that should either be documented in the PR description or potentially moved to a separate PR for clearer change tracking and review.
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | ||
| param customCognitiveServicesScope string = 'https://cognitiveservices.azure.com/.default' |
There was a problem hiding this comment.
The default value for customCognitiveServicesScope is hardcoded to 'https://cognitiveservices.azure.com/.default', which is the public Azure cloud endpoint. This default will be incorrect for users deploying to AzureUSGovernment or other custom clouds, as they use different cognitive services endpoints (e.g., 'https://cognitiveservices.azure.us/.default' for US Government). Consider making this value conditional based on the cloud environment or documenting that users must override this parameter when deploying to non-public clouds.
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | |
| param customCognitiveServicesScope string = 'https://cognitiveservices.azure.com/.default' | |
| @description('Custom Cognitive Services scope, e.g. https://cognitiveservices.azure.com/.default (public), https://cognitiveservices.azure.us/.default (US Gov)') | |
| param customCognitiveServicesScope string = az.environment().name == 'AzureUSGovernment' ? 'https://cognitiveservices.azure.us/.default' : 'https://cognitiveservices.azure.com/.default' |
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | ||
| param customCognitiveServicesScope string = 'https://cognitiveservices.azure.com/.default' | ||
| @description('Custom search resource URL for token audience, e.g. https://search.azure.us') | ||
| param customSearchResourceUrl string = 'https://search.azure.com' |
There was a problem hiding this comment.
The default value for customSearchResourceUrl is hardcoded to 'https://search.azure.com', which is the public Azure cloud endpoint. This default will be incorrect for users deploying to AzureUSGovernment or other custom clouds, as they use different search endpoints (e.g., 'https://search.azure.us' for US Government). Consider making this value conditional based on the cloud environment or documenting that users must override this parameter when deploying to non-public clouds.
| param customSearchResourceUrl string = 'https://search.azure.com' | |
| param customSearchResourceUrl string = cloudEnvironment == 'usgovernment' | |
| ? 'https://search.azure.us' | |
| : (cloudEnvironment == 'public' ? 'https://search.azure.com' : '') |
|
@clarked-msft , @SteveCInVA submitted a huge PR that may have made this OBE. please review. we will be using azd moving forward and the bicep it uses, please review. |
These changes should be to the latest bicep Steve submitted. My understanding is azd does not work AGTS but that is fine. Setup is very minimal from the bicep. |
|
Latest bicep changes are merged. Take a look at the 'cloudEnvironment' param and make sure you're okay with this... The cloud name is already available through az.environment() so I changed this to be the value SimpleChat uses and is really only needed to force 'custom' mode for testing. |
|
@SteveCInVA Does this latest update from @clarked-msft conflict with anything you are working on? Let us know so we can approve otherwise. I want to ensure you are in the loop on this one. I am ready to merge once you give the go ahead. |
SteveCInVA
left a comment
SteveCInVA
left a comment
There was a problem hiding this comment.
So it looks like it will all work correctly. If possible, please sync the code from the dev-sc-privatenw branch and verify again, but I don't see major issues.
|
Conflicts resolved |
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | ||
| param customCognitiveServicesScope string = 'https://cognitiveservices.azure.com/.default' | ||
| @description('Custom search resource URL for token audience, e.g. https://search.azure.us') | ||
| param customSearchResourceUrl string = 'https://search.azure.com' |
There was a problem hiding this comment.
The default value for customSearchResourceUrl uses the public Azure endpoint ('https://search.azure.com'), but this may be incorrect for government or other custom clouds. When az.environment() detects a non-public cloud but cloudEnvironment is set to 'custom', this will use the wrong search endpoint. Consider using az.environment().suffixes or providing environment-specific defaults.
| param customSearchResourceUrl string = 'https://search.azure.com' | |
| param customSearchResourceUrl string = 'https://search.${az.environment().suffixes.search}' |
| @description('Custom Resource Manager URL, e.g. https://management.usgovcloudapi.net') | ||
| param customResourceManagerUrl string = az.environment().resourceManager | ||
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | ||
| param customCognitiveServicesScope string = 'https://cognitiveservices.azure.com/.default' |
There was a problem hiding this comment.
The default value for customCognitiveServicesScope uses the public Azure endpoint ('https://cognitiveservices.azure.com/.default'), but this may be incorrect for government or other custom clouds. When az.environment() detects a non-public cloud but cloudEnvironment is set to 'custom', this will use the wrong cognitive services scope. Consider providing environment-specific defaults based on az.environment().
| param customCognitiveServicesScope string = 'https://cognitiveservices.azure.com/.default' | |
| param customCognitiveServicesScope string = az.environment().name == 'AzureUSGovernment' ? 'https://cognitiveservices.azure.us/.default' : 'https://cognitiveservices.azure.com/.default' |
| @description('Custom Resource Manager URL, e.g. https://management.usgovcloudapi.net') | ||
| param customResourceManagerUrl string? | ||
|
|
||
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') |
There was a problem hiding this comment.
The description for this parameter uses "ex:" instead of the more standard "e.g." or "for example". For consistency with other parameter descriptions in the file (which use "e.g."), consider updating this to match.
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | |
| @description('Custom Cognitive Services scope, e.g. https://cognitiveservices.azure.com/.default') |
| {name: 'CUSTOM_GRAPH_URL_VALUE', value: customGraphUrl} | ||
| {name: 'CUSTOM_IDENTITY_URL_VALUE', value: customIdentityUrl} | ||
| {name: 'CUSTOM_RESOURCE_MANAGER_URL_VALUE', value: customResourceManagerUrl} | ||
| {name: 'CUSTOM_BLOB_STORAGE_URL_VALUE', value: customBlobStorageSuffix} | ||
| {name: 'CUSTOM_COGNITIVE_SERVICES_URL_VALUE', value: customCognitiveServicesScope} | ||
| {name: 'CUSTOM_SEARCH_RESOURCE_MANAGER_URL_VALUE', value: customSearchResourceUrl} |
There was a problem hiding this comment.
The nullable parameters are passed directly to environment variables without null checking. If these parameters are not provided (which is valid for nullable parameters), the environment variables will be set to empty or undefined values. For the 'custom' environment, consider either making these parameters required when azurePlatform is 'custom', or adding validation to ensure they are provided when needed.
| param customIdentityUrl string = az.environment().authentication.loginEndpoint | ||
| @description('Custom Resource Manager URL, e.g. https://management.usgovcloudapi.net') | ||
| param customResourceManagerUrl string = az.environment().resourceManager | ||
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') |
There was a problem hiding this comment.
The description for this parameter uses "ex:" instead of the more standard "e.g." or "for example". For consistency with other parameter descriptions in the file (which use "e.g."), consider updating this to match.
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | |
| @description('Custom Cognitive Services scope e.g. https://cognitiveservices.azure.com/.default') |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
application/single_app/config.py:154
- This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
OIDC_METADATA_URL = f"https://login.microsoftonline.com/{TENANT_ID}/v2.0/.well-known/openid-configuration"
| param customBlobStorageSuffix string? | ||
| @description('Custom Graph API URL, e.g. https://graph.microsoft.us') | ||
| param customGraphUrl string? | ||
| @description('Custom Identity URL, e.g. https://login.microsoftonline.us') | ||
| param customIdentityUrl string? | ||
| @description('Custom Resource Manager URL, e.g. https://management.usgovcloudapi.net') | ||
| param customResourceManagerUrl string? | ||
|
|
||
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | ||
| param customCognitiveServicesScope string? | ||
|
|
||
| @description('Custom search resource URL for token audience, e.g. https://search.azure.us') | ||
| param customSearchResourceUrl string? |
There was a problem hiding this comment.
The parameter definitions use nullable types (string?) but main.bicep provides default values for all of these parameters. This creates an inconsistency - either these should be non-nullable string types to match the fact that they always have values from main.bicep's defaults, or the defaults in main.bicep should be removed and null handling should be added when setting the environment variables. The current approach could be confusing for users and may lead to unexpected behavior.
| param customBlobStorageSuffix string? | |
| @description('Custom Graph API URL, e.g. https://graph.microsoft.us') | |
| param customGraphUrl string? | |
| @description('Custom Identity URL, e.g. https://login.microsoftonline.us') | |
| param customIdentityUrl string? | |
| @description('Custom Resource Manager URL, e.g. https://management.usgovcloudapi.net') | |
| param customResourceManagerUrl string? | |
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | |
| param customCognitiveServicesScope string? | |
| @description('Custom search resource URL for token audience, e.g. https://search.azure.us') | |
| param customSearchResourceUrl string? | |
| param customBlobStorageSuffix string | |
| @description('Custom Graph API URL, e.g. https://graph.microsoft.us') | |
| param customGraphUrl string | |
| @description('Custom Identity URL, e.g. https://login.microsoftonline.us') | |
| param customIdentityUrl string | |
| @description('Custom Resource Manager URL, e.g. https://management.usgovcloudapi.net') | |
| param customResourceManagerUrl string | |
| @description('Custom Cognitive Services scope ex: https://cognitiveservices.azure.com/.default') | |
| param customCognitiveServicesScope string | |
| @description('Custom search resource URL for token audience, e.g. https://search.azure.us') | |
| param customSearchResourceUrl string |
| param customSearchResourceUrl string? | ||
|
|
||
| var tenantId = tenant().tenantId | ||
| var openIdMetadataUrl = '${az.environment().authentication.loginEndpoint}/${tenantId}/v2.0/.well-known/openid-configuration' |
There was a problem hiding this comment.
The openIdMetadataUrl variable is computed using az.environment().authentication.loginEndpoint which will always reflect the actual Azure environment being deployed to, not necessarily matching a 'custom' platform setting. If deploying to a truly custom Azure environment (not AzureCloud/AzureUSGovernment), this constructed URL might not be correct. The variable should potentially use customIdentityUrl when azurePlatform is 'custom' to ensure consistency.
| var openIdMetadataUrl = '${az.environment().authentication.loginEndpoint}/${tenantId}/v2.0/.well-known/openid-configuration' | |
| var identityBaseUrl = azurePlatform == 'custom' && !empty(customIdentityUrl) ? customIdentityUrl : az.environment().authentication.loginEndpoint | |
| var openIdMetadataUrl = '${identityBaseUrl}/${tenantId}/v2.0/.well-known/openid-configuration' |
| OIDC_METADATA_URL = CUSTOM_OIDC_METADATA_URL_VALUE | ||
| resource_manager = CUSTOM_RESOURCE_MANAGER_URL_VALUE | ||
| authority = CUSTOM_IDENTITY_URL_VALUE | ||
| credential_scopes=[resource_manager + "/.default"] | ||
| cognitive_services_scope = CUSTOM_COGNITIVE_SERVICES_URL_VALUE | ||
| search_resource_manager = CUSTOM_SEARCH_RESOURCE_MANAGER_URL_VALUE | ||
| KEY_VAULT_DOMAIN = os.getenv("KEY_VAULT_DOMAIN", ".vault.azure.net") | ||
| video_indexer_endpoint = os.getenv("VIDEO_INDEXER_ENDPOINT", "https://api.videoindexer.ai") |
There was a problem hiding this comment.
Adding CUSTOM_OIDC_METADATA_URL_VALUE is good for consistency, but note that the public cloud environment (else block starting at line 185) is missing the 'search_resource_manager' variable definition which is present in both usgovernment (line 173) and custom (line 182) blocks. While this is a pre-existing issue not introduced by this PR, it will cause runtime errors when search_resource_manager is referenced elsewhere in the code for public cloud deployments. Consider adding 'search_resource_manager = "https://search.azure.com"' to the else block in a follow-up change.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
application/single_app/config.py:154
- This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
This assignment to 'OIDC_METADATA_URL' is unnecessary as it is redefined before this value is used.
OIDC_METADATA_URL = f"https://login.microsoftonline.com/{TENANT_ID}/v2.0/.well-known/openid-configuration"
| 'custom' | ||
| ]) | ||
| param cloudEnvironment string | ||
| param cloudEnvironment string = az.environment().name == 'AzureCloud' ? 'public' : (az.environment().name == 'AzureUSGovernment' ? 'usgovernment' : 'custom') |
There was a problem hiding this comment.
The PR description mentions "Expose bicep param to set openai model params for existing OpenAI" but this change is not visible in the provided diffs. Please verify that this change was included in the pull request or update the PR description to accurately reflect the changes made.
4 participants
Uh oh!
There was an error while loading. Please reload this page.