feat(vscode): add "Deploy to Azure" webview and deploy flow (create Logic App without wizard) #8691
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | Keep; minor cosmetic suggestion optional. |
| Commit Type | ✅ | No change. |
| Risk Level | Change label to risk:high or provide justification for Medium. | |
| What & Why | ✅ | OK; can expand slightly. |
| Impact of Change | ❌ | Fill Developers & System sections (see recommendations). |
| Test Plan | ❌ | Add automated tests or detailed manual test steps and justifications. |
| Contributors | Optionally add contributor credits. | |
| Screenshots/Videos | ❌ | Add UI screenshots/videos demonstrating the webview and flows. |
Final notes & action items (high priority)
- Tests: add unit and/or E2E tests for the new logic (preferred), or expand the Test Plan with detailed manual test cases that include negative scenarios and telemetry validation. Without this, the PR cannot pass.
- Risk label: update the PR label to
risk:highor provide an explicit justification in the PR body explaining why the change is Medium and what mitigations exist (feature flag, staged rollout, small subset of users, etc.). - Impact of Change: fill Developers and System subsections. Call out new commands, new dependencies, and CI/node engine implications.
- Screenshots/Videos: attach images of the new webview UI (create new + deploy existing), and sample successful/failed deployments.
- pnpm-lock & node engine: the pnpm-lock shows new package entries with engines node >=20. Please verify CI/build pipeline and contributor guidelines support Node 20. If not, either pin compatible package versions or document required Node upgrade and CI changes.
- Security/logic correctness: there's a likely logic/comment mismatch in LogicAppCreateStep — you set httpsOnly: true (good) and vnetRouteAllEnabled: false with a comment saying "Enable VNet route all for enhanced security". If you intend to enable VNet route all, set true; otherwise fix the comment. Please confirm intended behavior.
- Telemetry: you add telemetry properties (e.g., deploymentSource=webview). Ensure telemetry schema/allowlist is updated if required by your telemetry governance.
- Manual-create flow: ensure createChildWithoutPrompts handles all failure cases cleanly; include tests for partial/failing ARM calls and cleanup if resources are partially created.
Please update the PR body to address the items above and either add tests or a thorough manual test plan, update the risk label (or justify), add screenshots, and fix the vnet/https comment discrepancy. After those updates, re-request review. Thank you for the thorough feature — it adds valuable capability, but it requires stricter testing and risk handling before merge.
Last updated: Tue, 20 Jan 2026 22:28:49 GMT
lambrianmsft
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| state.isCreatingNewStorageAccount = action.payload === '__CREATE_NEW__'; | ||
| }, | ||
| setNewStorageAccountName(state, action: PayloadAction<string>) { | ||
| state.newStorageAccountName = action.payload; |
Check failure
Code scanning / CodeQL
Insecure randomness High
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request implements a significant UX improvement by migrating the Logic App deployment command from VS Code's native wizard interface to a custom webview. This change provides better control over the deployment process, enhanced validation, and persistent display of user inputs.
Key Changes
- New deployment webview: Custom React-based UI for deployment with comprehensive validation
- Enhanced state management: Added Redux slice for managing deployment state across subscription, resource group, Logic App, and dependent resource selections
- Wizard-free Logic App creation: New code path to create Logic Apps programmatically without user prompts
- Azure resource fetching: New API service methods to fetch subscriptions, Logic Apps, resource groups, locations, app service plans, and storage accounts
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added Azure ARM dependencies (@azure/arm-resources, @azure/arm-subscriptions) for resource management |
| libs/vscode-extension/src/lib/models/project.ts | Added 'deploy' project name constant |
| libs/vscode-extension/src/lib/models/extensioncommand.ts | Added deploy-related extension commands (deploy, cancel_deploy, getFilteredLocations) |
| apps/vs-code-react/src/stateWrapper.tsx | Added navigation routing for deploy project |
| apps/vs-code-react/src/state/store.ts | Registered deploySlice reducer in Redux store |
| apps/vs-code-react/src/state/deploySlice.ts | Complete Redux slice for deployment state management with 21 state properties |
| apps/vs-code-react/src/run-service/types.ts | Added logicApps resource type and deploy route |
| apps/vs-code-react/src/run-service/export/index.ts | Added methods for fetching Logic Apps, locations, app service plans, storage accounts, and name availability checking |
| apps/vs-code-react/src/router/index.tsx | Added DeployApp route |
| apps/vs-code-react/src/intl/messages.ts | Added 271 new localization strings for deployment UI |
| apps/vs-code-react/src/intl/index.ts | Exported deployMessages |
| apps/vs-code-react/src/app/deploy/deployStyles.ts | Styling definitions for deployment UI |
| apps/vs-code-react/src/app/deploy/deploy.tsx | Main deployment React component (853 lines) with form validation and Azure resource management |
| apps/vs-code-designer/src/extensionVariables.ts | Added deploy webview key |
| apps/vs-code-designer/src/app/tree/subscriptionTree/subscriptionTreeItem.ts | Added createChildWithoutPrompts method for programmatic Logic App creation (254 lines) |
| apps/vs-code-designer/src/app/commands/registerCommands.ts | Changed deploy command from wizard to webview handler |
| apps/vs-code-designer/src/app/commands/deploy/deployWebview.ts | New webview handler for deployment with message processing |
| apps/vs-code-designer/src/app/commands/deploy/deploy.ts | Exported deploy function for reuse |
| apps/vs-code-designer/src/app/commands/createLogicApp/createLogicAppSteps/logicAppCreateStep.ts | Added httpsOnly and vnetRouteAllEnabled properties to site config |
| apps/vs-code-designer/src/app/commands/createLogicApp/createLogicApp.ts | Added createLogicAppWithoutWizard function for programmatic creation |
| apps/vs-code-designer/package.json | Added Azure ARM dependencies |
| Localize/lang/strings.json | Added localization keys and descriptions for deployment UI |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| reserved: context.newSiteOS === WebsiteOS.linux, | ||
| identity: context.customLocation ? undefined : { type: 'SystemAssigned' }, | ||
| httpsOnly: true, | ||
| vnetRouteAllEnabled: false, // Enable VNet route all for enhanced security |
There was a problem hiding this comment.
Inconsistent handling of vnetRouteAllEnabled property. The comment says "Enable VNet route all for enhanced security" but the value is set to false, which disables the feature. Either the comment is incorrect or the value should be true.
Suggested change
| vnetRouteAllEnabled: false, // Enable VNet route all for enhanced security | |
| vnetRouteAllEnabled: true, // Enable VNet route all for enhanced security |
Comment on lines
25
to
56
| createHandler: async (actionContext: IActionContext, data: any) => { | ||
| if (data.createNew) { | ||
| // User wants to create a new Logic App without wizard prompts | ||
| const createContext: any = { | ||
| ...actionContext, | ||
| newSiteName: data.newLogicAppName, | ||
| location: data.location, | ||
| newResourceGroupName: data.isCreatingNewResourceGroup ? data.resourceGroup : undefined, | ||
| resourceGroup: data.isCreatingNewResourceGroup ? undefined : { name: data.resourceGroup }, | ||
| newPlanName: data.isCreatingNewAppServicePlan ? data.appServicePlan : undefined, | ||
| plan: data.isCreatingNewAppServicePlan ? undefined : { id: data.appServicePlan }, | ||
| appServicePlanSku: data.appServicePlanSku || 'WS1', | ||
| newStorageAccountName: data.isCreatingNewStorageAccount ? data.storageAccount : undefined, | ||
| storageAccount: data.isCreatingNewStorageAccount ? undefined : { id: data.storageAccount }, | ||
| createAppInsights: data.createAppInsights, | ||
| newAppInsightsName: data.appInsightsName, | ||
| }; | ||
|
|
||
| // Create the Logic App using the wizard-free method | ||
| const node: SlotTreeItem = await createLogicAppWithoutWizard( | ||
| createContext, | ||
| data.subscriptionId, | ||
| true // Skip notification since we're deploying next | ||
| ); | ||
|
|
||
| // Now deploy to the newly created Logic App | ||
| await deploy(actionContext, node.fullId, node.fullId); | ||
| } else { | ||
| // Deploy to existing Logic App | ||
| await deploy(actionContext, data.logicAppId, data.logicAppId); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Missing input validation in deployViaWebview handler. The data object should be validated before being used to create resources. Required fields like subscriptionId, newLogicAppName, location, resourceGroup should be checked for existence and validity to prevent runtime errors.
Comment on lines
+318
to
+341
| // Check storage account name availability (debounced) | ||
| useEffect(() => { | ||
| if (!newStorageAccountName || !isCreatingNew || !isCreatingNewStorageAccount || !selectedSubscription || storageAccountNameError) { | ||
| setStorageNameUnavailable(false); | ||
| setStorageNameMessage(''); | ||
| return; | ||
| } | ||
|
|
||
| setIsCheckingStorageName(true); | ||
| const timeoutId = setTimeout(async () => { | ||
| try { | ||
| const result = await apiService.checkStorageAccountNameAvailability(selectedSubscription, newStorageAccountName); | ||
| setStorageNameUnavailable(!result.available); | ||
| setStorageNameMessage(result.message || ''); | ||
| } catch (error) { | ||
| console.error('Error checking storage account name availability:', error); | ||
| } finally { | ||
| setIsCheckingStorageName(false); | ||
| } | ||
| }, 500); // 500ms debounce | ||
|
|
||
| return () => clearTimeout(timeoutId); | ||
| }, [newStorageAccountName, isCreatingNew, isCreatingNewStorageAccount, selectedSubscription, storageAccountNameError, apiService]); | ||
|
|
There was a problem hiding this comment.
Duplicate useEffect for storage account name availability check. Lines 294-316 and 318-340 contain identical logic. This will cause the availability check to run twice for every state change, doubling the API calls and potentially causing race conditions.
Suggested change
| // Check storage account name availability (debounced) | |
| useEffect(() => { | |
| if (!newStorageAccountName || !isCreatingNew || !isCreatingNewStorageAccount || !selectedSubscription || storageAccountNameError) { | |
| setStorageNameUnavailable(false); | |
| setStorageNameMessage(''); | |
| return; | |
| } | |
| setIsCheckingStorageName(true); | |
| const timeoutId = setTimeout(async () => { | |
| try { | |
| const result = await apiService.checkStorageAccountNameAvailability(selectedSubscription, newStorageAccountName); | |
| setStorageNameUnavailable(!result.available); | |
| setStorageNameMessage(result.message || ''); | |
| } catch (error) { | |
| console.error('Error checking storage account name availability:', error); | |
| } finally { | |
| setIsCheckingStorageName(false); | |
| } | |
| }, 500); // 500ms debounce | |
| return () => clearTimeout(timeoutId); | |
| }, [newStorageAccountName, isCreatingNew, isCreatingNewStorageAccount, selectedSubscription, storageAccountNameError, apiService]); |
Comment on lines
+198
to
+203
| // Generate a simple GUID-like suffix | ||
| const generateGuid = () => { | ||
| return Math.random().toString(36).substring(2, 10); | ||
| }; | ||
|
|
||
| const guid = generateGuid(); |
There was a problem hiding this comment.
The random GUID generation using Math.random() is not cryptographically secure and may produce collisions. Consider using a proper UUID library or crypto.randomUUID() for generating unique identifiers for Azure resources.
Suggested change
| // Generate a simple GUID-like suffix | |
| const generateGuid = () => { | |
| return Math.random().toString(36).substring(2, 10); | |
| }; | |
| const guid = generateGuid(); | |
| // Generate a simple GUID-like suffix using cryptographically secure random values | |
| const generateSecureSuffix = () => { | |
| const bytes = new Uint8Array(8); | |
| crypto.getRandomValues(bytes); | |
| return Array.from(bytes, (b) => b.toString(36).padStart(2, '0')).join('').substring(0, 8); | |
| }; | |
| const guid = generateSecureSuffix(); |
| * @param skipNotification - If true, skips the completion notification | ||
| * @returns The created Logic App tree item | ||
| */ | ||
| export async function createLogicAppWithoutWizard( |
Contributor
There was a problem hiding this comment.
Shouldn't SubscriptionTreeItem.createChild already skip prompts if the values are provided in context? And if not, would it make sense to modify that method to skip prompts when not needed instead of adding this one?
| }; | ||
|
|
||
| // Create the Logic App using the wizard-free method | ||
| const node: SlotTreeItem = await createLogicAppWithoutWizard( |
Contributor
There was a problem hiding this comment.
Isn't createLogicApp called from deploy if the logic app doesn't exist? Can this be refactored so that the logic app is either always created prior to deploy or always created within deploy function?
…sting logic apps and new logic apps
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Type
Risk Level
What & Why
This moves the deploy command from using vscode wizards to a custom webview which allows more control and validation from the extension. We also display all relevant information so that users don't have to remember what they had input previously.
Impact of Change
Users now see all content before deploying, and allow naming of resources when a new logic app is created.
Test Plan
Contributors
Screenshots/Videos