fix: Refactor scripts for WAF changes and improve clarity#889
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the post-deployment image build/deploy scripts to better support WAF/private ACR scenarios, and updates the infra templates to simplify ACR permissions and improve Jumpbox VM identity/login configuration.
Changes:
- Refactors
build_and_deploy_imagesscripts (bash + PowerShell) to support config discovery via azd env or deployment outputs, temporarily open/close ACR public access in WAF mode, and ensure MI-based ACR pull config is restored after updating the web app container. - Simplifies ACR role assignments to only grant
AcrPullto managed identities (removing deployerAcrPushassignment parameters/logic). - Updates Jumpbox VM deployment to generate a default admin password when not provided, enable system-assigned identity, and configure Entra ID join + “Virtual Machine Administrator Login” role assignment; also adjusts Jumpbox DCR stream naming/flow settings.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/scripts/build_and_deploy_images.sh | Major refactor: config resolution options, WAF ACR open/relock, backend ACI recreation + frontend update/restore. |
| infra/scripts/build_and_deploy_images.ps1 | PowerShell parity with bash refactor: same WAF ACR handling, config resolution, and deployment steps. |
| infra/modules/container-registry.bicep | Removes AcrPush role assignment support; keeps only MI AcrPull assignments. |
| infra/main.bicep | Removes deployerType wiring for ACR push, changes Jumpbox password/MI/AAD join/role assignment, updates DCR streams/flows. |
| infra/main.json | Regenerates/updates ARM JSON reflecting the Bicep changes (hash + template content updates). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Roopan-Microsoft
approved these changes
Jul 7, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose
This pull request removes support for assigning the
AcrPushrole to the deployer for the Azure Container Registry (ACR), simplifying the deployment by only assigningAcrPullpermissions to managed identities. It also enhances the Jumpbox VM configuration by ensuring a secure, default admin password is generated if not provided, enables system-assigned managed identity (required for Entra ID login), and adds automatic Entra ID (AAD) join and administrator role assignment for the deployer. Additionally, it updates monitoring resources to align with new naming and stream conventions.Container Registry Role Assignment Simplification:
AcrPushrole assignments, includingpushPrincipalIds,deployerType, and theAcrPushrole definition, frominfra/modules/container-registry.bicep,infra/main.bicep, andinfra/main.json. Now onlyAcrPullpermissions are assigned to managed identities. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Jumpbox VM Security and Identity Improvements:
extensionAadJoinConfig. [1] [2]Monitoring and Data Collection Updates:
securityEventLogsDataSource/Microsoft-SecurityEventtoSecurityAuditEvents/Microsoft-Event, and adds new KQL transform/output stream settings. [1] [2] [3] [4]Minor Logic Cleanup:
Template Metadata:
infra/main.jsonto reflect the changes. [1] [2]Does this introduce a breaking change?
Golden Path Validation
Deployment Validation