Skip to content

fix: Refactor scripts for WAF changes and improve clarity#889

Merged
Roopan-Microsoft merged 3 commits into
mainfrom
rc-acr-update-cg
Jul 7, 2026
Merged

fix: Refactor scripts for WAF changes and improve clarity#889
Roopan-Microsoft merged 3 commits into
mainfrom
rc-acr-update-cg

Conversation

@Ragini-Microsoft

Copy link
Copy Markdown
Collaborator

Purpose

This pull request removes support for assigning the AcrPush role to the deployer for the Azure Container Registry (ACR), simplifying the deployment by only assigning AcrPull permissions 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:

  • Removed all parameters, variables, and logic related to AcrPush role assignments, including pushPrincipalIds, deployerType, and the AcrPush role definition, from infra/modules/container-registry.bicep, infra/main.bicep, and infra/main.json. Now only AcrPull permissions are assigned to managed identities. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Jumpbox VM Security and Identity Improvements:

  • Automatically generates a secure default admin password for the Jumpbox VM if none is provided, improving out-of-the-box security. [1] [2]
  • Enables system-assigned managed identity on the Jumpbox VM, required for Entra ID (AAD) login. [1] [2]
  • Adds a role assignment granting the deployer the "Virtual Machine Administrator Login" role on the Jumpbox VM. [1] [2]
  • Configures the Jumpbox VM to automatically join Entra ID (AAD) and enables AAD login via the extensionAadJoinConfig. [1] [2]

Monitoring and Data Collection Updates:

  • Updates Data Collection Rule (DCR) naming and stream conventions for Windows event logs, switching from securityEventLogsDataSource/Microsoft-SecurityEvent to SecurityAuditEvents/Microsoft-Event, and adds new KQL transform/output stream settings. [1] [2] [3] [4]

Minor Logic Cleanup:

  • Simplifies the condition for deploying admin access resources by removing the dependency on the VM admin password being set. [1] [2]

Template Metadata:

  • Updates Bicep-generated template hashes in infra/main.json to reflect the changes. [1] [2]

Does this introduce a breaking change?

  • Yes
  • No

Golden Path Validation

  • I have tested the primary workflows (the "golden path") to ensure they function correctly without errors.

Deployment Validation

  • I have validated the deployment process successfully and all services are running as expected with this change.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_images scripts (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 AcrPull to managed identities (removing deployer AcrPush assignment 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.

Comment thread infra/main.bicep
Comment thread infra/main.bicep
Comment thread infra/scripts/build_and_deploy_images.sh
Comment thread infra/scripts/build_and_deploy_images.ps1
@Roopan-Microsoft Roopan-Microsoft merged commit 93b9544 into main Jul 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants