feat: add dynamic Azurite connection string generation using workspace settings #4667
Conversation
…e settings - Generate complete connection strings with all endpoints (blob, queue, table) - Use Azurite workspace settings for dynamic host/port configuration - Support both HTTP and HTTPS protocols based on certificate settings - Add emulator detection functionality for improved connection handling Fixes #3876
- Update validateStorageConnectionPreDebug to handle tuple return type - Adapt validatePreDebug to work with new getLocalSettingsConnectionString signature - Maintain compatibility with emulator detection functionality
There was a problem hiding this comment.
The changes improve Azurite integration by generating complete connection strings dynamically. However, there are some logic issues with emulator detection that need to be addressed, particularly around environment variable handling and false positive detection.
| export async function getLocalSettingsConnectionString(context: IActionContext, connectionKey: ConnectionKey, projectPath: string): Promise<string | undefined> { | ||
| export async function getLocalSettingsConnectionString(context: IActionContext, connectionKey: ConnectionKey, projectPath: string): Promise<[string | undefined, boolean]> { | ||
| // func cli uses environment variable if it's defined on the machine, so no need to prompt | ||
| if (process.env[connectionKey]) { |
There was a problem hiding this comment.
Bug: process.env[connectionKey] is treated as an emulator connection on line 25, but environment variables are typically production connection strings, not emulator connections. This will cause incorrect emulator detection for production deployments that use environment variables.
The return value should be [process.env[connectionKey], false] to indicate it's not an emulator connection.
Review generated with Copilot
| // check for azurite settings and build the connection string if it's an emulator | ||
| connectionString = getLocalSettingsEmulatorConnectionString(); | ||
| } | ||
| const isEmulator = isConnectionStringEmulator(connectionString); |
There was a problem hiding this comment.
Bug: Missing null check for connectionString before calling isConnectionStringEmulator() on line 35. If settings.Values[connectionKey] is undefined, this could cause issues in the emulator detection logic.
The isConnectionStringEmulator function handles undefined values, but the logic flow should be clarified to ensure connectionString is validated before the emulator check.
Review generated with Copilot
There was a problem hiding this comment.
If isConnectionStringEmulator can handle undefined, why are you flagging this issue?
src/funcConfig/local.settings.ts
Outdated
| const tablePort = getWorkspaceSetting('tablePort', undefined, 'azurite') || '10002'; | ||
|
|
||
| const protocol = getTransferProtocol(); | ||
| return `DefaultEndpointsProtocol=${protocol};AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=${protocol}://${blobHost}:${blobPort}/devstoreaccount1;QueueEndpoint=${protocol}://${queueHost}:${queuePort}/devstoreaccount1;TableEndpoint=${protocol}://${tableHost}:${tablePort}/devstoreaccount1;`; |
There was a problem hiding this comment.
The hard-coded account key Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw== should be extracted to a constant for maintainability. This is the standard Azurite development account key, but having it as a magic string makes it harder to maintain.
Consider adding it to constants.ts as azuriteAccountKey or similar.
Review generated with Copilot
src/funcConfig/local.settings.ts
Outdated
| return !!connectionString && | ||
| (connectionString.includes(blobHost) || | ||
| connectionString.includes(queueHost) || | ||
| connectionString.includes(tableHost) || |
There was a problem hiding this comment.
The emulator detection logic on lines 54-59 could produce false positives. Checking for 'localhost' as a blanket condition might match legitimate production connection strings that happen to contain "localhost" in metadata or descriptions.
Consider making the localhost check more specific, such as checking for localhost in endpoint URLs rather than anywhere in the connection string.
Review generated with Copilot
| const azureWebJobsStorage: string | undefined = await getLocalSettingsConnectionString(context, ConnectionKey.Storage, projectPath); | ||
| if (azureWebJobsStorage && azureWebJobsStorage.toLowerCase() === localStorageEmulatorConnectionString.toLowerCase()) { | ||
| const [azureWebJobsStorage, isEmulator] = await getLocalSettingsConnectionString(context, ConnectionKey.Storage, projectPath); | ||
| if (azureWebJobsStorage && isEmulator) { |
There was a problem hiding this comment.
The destructuring assignment here correctly handles the new tuple return type from getLocalSettingsConnectionString(). The isEmulator boolean provides better semantic clarity than string comparison.
Review generated with Copilot
There was a problem hiding this comment.
Don't need to tell me that I am doing things correctly. Please don't do that.
2 participants
Summary
This PR enhances the Azurite integration by dynamically generating complete connection strings using workspace settings instead of returning partial URLs.
Changes
azurite.blobHost,azurite.queueHost,azurite.tableHost(defaults to 127.0.0.1)azurite.blobPort,azurite.queuePort,azurite.tablePort(defaults to 10000, 10001, 10002)Connection String Format
The generated connection string follows the standard Azure Storage format:
References
Testing