leverage ServiceConnector to setup connection to keyvault and DB #1
Conversation
| @@ -33,6 +33,13 @@ param numberOfWorkers int = -1 | |||
| param scmDoBuildDuringDeployment bool = false | |||
| param use32BitWorkerProcess bool = false | |||
|
|
|||
| // Target DB properties | |||
| param connectionStringKey string = 'AZURE-SQL-CONNECTION-STRING' | |||
There was a problem hiding this comment.
We wouldn't update /core/host/appservice to use SQL because the app service itself can be used with any DB and it shouldn't default to anything.
| param appUser string = '' | ||
| @secure() | ||
| param appUserPassword string = '' | ||
|
|
There was a problem hiding this comment.
Why do we need appUser for app service?
There was a problem hiding this comment.
when setting up connection between the app service and target db, user need to provide target db's connection info including appUser, appPassword.
| configurationInfo: { | ||
| customizedKeys: { | ||
| 'AZURE_KEYVAULT_RESOURCEENDPOINT': 'AZURE_KEY_VAULT_ENDPOINT' | ||
| } | ||
| } |
There was a problem hiding this comment.
How set are you on using that key? Ideally azd and service connector use the same so we don't have to overide by default.
There was a problem hiding this comment.
azd's default is AZURE_KEY_VAULT_ENDPOINT, service connector default is AZURE_KEYVAULT_RESOURCEENDPOINT, so here is to follow azd default
| resource connectionToTargetDB 'Microsoft.ServiceLinker/linkers@2022-11-01-preview' = if (!empty(targetResourceId)) { | ||
| name: 'conn_db' | ||
| scope: appService | ||
| properties: { | ||
| targetService: { | ||
| id: targetResourceId | ||
| type: 'AzureResource' | ||
| } | ||
| secretStore: { | ||
| keyVaultId: !empty(keyVaultName) ? keyVault.id : '' | ||
| keyVaultSecretName: !empty(keyVaultName) ? connectionStringKey : '' | ||
| } | ||
| authInfo: { | ||
| authType: 'secret' | ||
| name: appUser | ||
| secretInfo: { | ||
| secretType: 'rawValue' | ||
| value: appUserPassword | ||
| } | ||
| } | ||
| clientType: 'dotnet' | ||
| } | ||
| dependsOn: [ | ||
| connectionToKeyVault | ||
| ] | ||
| } | ||
|
|
There was a problem hiding this comment.
This (being in appservice) means that app service can only be linked to one db. I think we need to consider a scenario where all of this is abstracted and included in main.bicep as a module with very few inputs.
| params: { | ||
| keyVaultName: keyVault.outputs.name | ||
| principalId: api.outputs.SERVICE_API_IDENTITY_PRINCIPAL_ID | ||
| targetResourceId: '${sqlServer.outputs.id}/databases/${sqlServer.outputs.databaseName}' |
There was a problem hiding this comment.
I'd add this string as an output of db.bicep
| appUser: appUser | ||
| appUserPassword: appUserPassword |
There was a problem hiding this comment.
Why does appservice need to know about appUser? We don't want to couple the two at the appservice level.
There was a problem hiding this comment.
appservice doesn't know appUser, it's a param passed in from main, user knows their app, and their db's username/password.
| @@ -25,6 +32,10 @@ module api '../core/host/appservice.bicep' = { | |||
| runtimeName: 'dotnetcore' | |||
| runtimeVersion: '6.0' | |||
| scmDoBuildDuringDeployment: false | |||
| targetResourceId: targetResourceId | |||
There was a problem hiding this comment.
To me, targetResourceId name isn't enough to know what it is or used for.
| secretStore: { | ||
| keyVaultId: !empty(keyVaultName) ? keyVault.id : '' | ||
| keyVaultSecretName: !empty(keyVaultName) ? connectionStringKey : '' | ||
| } |
There was a problem hiding this comment.
Do we want to not created the resource if the keyVult isn't passed in?
There was a problem hiding this comment.
no, no need to create keyvault conn is keyvault is empty
resource connectionToKeyVault 'Microsoft.ServiceLinker/linkers@2022-11-01-preview' = if(!empty(keyVaultName)) {
leverage ServiceConnector to setup connections to keyvault and DB