Skip to content

Feature/OIDC acr migration#1001

Merged
BenjaminMichaelis merged 3 commits intomainfrom
feature/oidc-acr-migration
Apr 22, 2026
Merged

Feature/OIDC acr migration#1001
BenjaminMichaelis merged 3 commits intomainfrom
feature/oidc-acr-migration

Conversation

@BenjaminMichaelis
Copy link
Copy Markdown
Member

Description

Describe your changes here.

Fixes #Issue_Number (if available)

Ensure that your pull request has followed all the steps below:

  • Code compilation
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary

Benjamin Michaelis added 3 commits April 21, 2026 16:17
- Replace docker/login-action (admin credentials) with az acr login via
  OIDC session (azure/login with AZURE_CLIENT_ID)
- Add --registry-identity and --user-assigned flags to az containerapp up
  so identity attachment and registry config happen atomically on create
  and update, eliminating the chicken-and-egg ordering issue
- Replace az config set extension.use_dynamic_install with explicit
  az extension add --name containerapp --upgrade to ensure the extension
  version supports --registry-identity on az containerapp up
- Remove --registry-username/--registry-password from containerapp up
- Update Assign MI step to use WEB_UAMI_RESOURCE_ID and WEB_UAMI_CLIENT_ID
…pdate

- Add --image to az containerapp update so image and env vars are set
  atomically in a single revision (dev and prod)
- Remove separate containerapp up which required CAE-level permissions;
  identity assign + registry set + update --image are CA-scoped only
  (plus managedEnvironments/read, covered by Container Apps Contributor
  role on the CAE granted to deploy UAMIs)
…_APP_ENVIRONMENT env var

- Change deploy-production cancel-in-progress from true to false to
  prevent broken state if deploy is cancelled mid-flight (secrets set
  but revision not yet updated)
- Remove unused CONTAINER_APP_ENVIRONMENT env var from both dev and prod
  Assign MI steps (vestigial from az containerapp up era)
Copilot AI review requested due to automatic review settings April 22, 2026 03:30
@BenjaminMichaelis BenjaminMichaelis merged commit e49c5c0 into main Apr 22, 2026
4 checks passed
@BenjaminMichaelis BenjaminMichaelis deleted the feature/oidc-acr-migration branch April 22, 2026 03:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the GitHub Actions deployment workflow to use Azure OIDC + managed identity for Azure Container Registry (ACR) auth and Container App registry configuration, removing the need for ACR username/password secrets.

Changes:

  • Switch Azure login to use a standardized AZURE_CLIENT_ID secret (env-scoped).
  • Replace docker/login-action (ACR username/password) with az acr login, and configure Container App identity + registry via az containerapp ....
  • Change production deploy concurrency to allow overlapping runs (cancel-in-progress: false).

az containerapp identity assign --name $CONTAINER_APP_NAME --resource-group $RESOURCEGROUP --user-assigned ${{ secrets.WEB_UAMI_RESOURCE_ID }}
az containerapp registry set --name $CONTAINER_APP_NAME --resource-group $RESOURCEGROUP --server $CONTAINER_REGISTRY --identity ${{ secrets.WEB_UAMI_RESOURCE_ID }}

- name: Assign Managed Identity to Container App and Set Secrets and Environment Variables
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

The step name says it assigns a managed identity, but the identity assignment was moved to the prior step. Rename this step to reflect what it now does (setting Key Vault-backed secrets and updating Container App env vars/image) to avoid confusion during ops/debugging.

Suggested change
- name: Assign Managed Identity to Container App and Set Secrets and Environment Variables
- name: Set Key Vault-backed secrets and update Container App environment variables and image

Copilot uses AI. Check for mistakes.
concurrency:
group: deploy-production
cancel-in-progress: true
cancel-in-progress: false
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

Setting cancel-in-progress: false for the production deploy concurrency group allows multiple production deployments to run at the same time. Because both runs mutate the same Container App (identity/registry/secrets/image), overlapping runs can race and leave production on an unexpected image or env-var set. Consider reverting to cancel-in-progress: true (or use a more granular concurrency group keyed by environment/app name if parallelism is needed).

Suggested change
cancel-in-progress: false
cancel-in-progress: true

Copilot uses AI. Check for mistakes.
az containerapp identity assign --name $CONTAINER_APP_NAME --resource-group $RESOURCEGROUP --user-assigned ${{ secrets.WEB_UAMI_RESOURCE_ID }}
az containerapp registry set --name $CONTAINER_APP_NAME --resource-group $RESOURCEGROUP --server $CONTAINER_REGISTRY --identity ${{ secrets.WEB_UAMI_RESOURCE_ID }}

- name: Assign Managed Identity to Container App and Set Secrets and Environment Variables
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

The step name says it assigns a managed identity, but the identity assignment was moved to the prior step. Rename this step to reflect what it now does (setting Key Vault-backed secrets and updating Container App env vars/image) to avoid confusion during ops/debugging.

Suggested change
- name: Assign Managed Identity to Container App and Set Secrets and Environment Variables
- name: Set Key Vault-backed Secrets and Update Container App Environment Variables and Image

Copilot uses AI. Check for mistakes.
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.

2 participants