Migrate engine deploy auth to Azure OIDC - #40
Conversation
📝 WalkthroughWalkthroughAdds Azure Container App runtime infrastructure, GitHub OIDC authentication with a user-assigned managed identity, production workflow deployment wiring, updated infrastructure naming, and workflow configuration path changes. ChangesAzure runtime deployment
Workflow configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant AzureLogin
participant ContainerRegistry
participant AzureContainerApp
GitHubActions->>AzureLogin: Request OIDC login with production variables
GitHubActions->>ContainerRegistry: Authenticate and push image
GitHubActions->>AzureContainerApp: Validate configured target and update image
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 500711452e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --role Contributor ` | ||
| --scope $deploymentRgId | ||
|
|
||
| $subject = "repo:$GitHubOwner/$GitHubRepository:environment:$GitHubEnvironment" |
There was a problem hiding this comment.
When this helper is run with the default repository values, this double-quoted string does not append a literal :environment: after $GitHubRepository: PowerShell treats a variable followed by : as a scoped variable reference, so the federated credential subject is malformed instead of matching GitHub's repo:<owner>/<repo>:environment:production subject (PowerShell quoting docs). Following the setup instructions can therefore create a credential that azure/login will not match; build the string with ${GitHubRepository}:environment:${GitHubEnvironment} or a format expression.
Useful? React with 👍 / 👎.
5007114 to
753000a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.azure/pipeline-setup.md:
- Around line 23-25: Update the setup instructions surrounding
scripts/setup-azure-auth-for-pipeline.ps1 to document that the signed-in Azure
CLI identity needs Microsoft.Authorization/roleAssignments/write on the
deployment resource group or subscription. Explicitly identify Owner, User
Access Administrator, or Role-Based Access Control Administrator as suitable
roles, and avoid implying that Contributor access is sufficient.
In @.github/workflows/deploy-autopr-engine.yml:
- Around line 156-157: Update the failure branch around the az containerapp show
check so its message does not claim the Container App is missing for every CLI
failure. Replace the provisioning-specific guidance with neutral wording asking
the pipeline identity to verify the app’s existence and access in the specified
resource group.
In `@scripts/setup-azure-auth-for-pipeline.ps1`:
- Line 25: Update the setup script after az account set to fetch the selected
account’s tenantId via az account show, reject or fail when it differs from the
caller-provided $TenantId, and export the validated subscription-backed tenant
ID as AZURE_TENANT_ID instead of using $TenantId directly.
- Around line 23-58: Ensure every Azure CLI invocation in the setup script fails
immediately on a nonzero exit code instead of relying only on
$ErrorActionPreference. Apply the existing or proposed Invoke-Az throwing helper
consistently to az account set, az group create, az identity create, az group
show, az role assignment create, and az identity federated-credential create,
while preserving their current arguments and output handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9840c1e0-d28a-4726-904a-94ced9973374
📒 Files selected for processing (3)
.azure/pipeline-setup.md.github/workflows/deploy-autopr-engine.ymlscripts/setup-azure-auth-for-pipeline.ps1
7b1f9a8 to
fd1467d
Compare
fd1467d to
4943b0f
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
orchestration/infrastructure/terraform/runtime/main.tf (1)
15-23: 🔒 Security & Privacy | 🔵 TrivialDefine the production ACR security controls explicitly.
The registry leaves public network access at its default and does not define image scanning, quarantine, or trusted-image controls. Confirm the external scanning/signing and network-isolation plan; disabling public access would also require private connectivity for the GitHub-hosted build runner.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestration/infrastructure/terraform/runtime/main.tf` around lines 15 - 23, Update the azurerm_container_registry.runtime resource to explicitly define the production security controls: set the intended public network access behavior and configure image scanning, quarantine, and trusted-image settings according to the approved external scanning/signing and network-isolation plan. If public access is disabled, also provision the private connectivity required by the GitHub-hosted build runner.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy-autopr-engine.yml:
- Line 151: Update the build-and-push job to expose the exact image version from
steps.meta.outputs.version as its image_tag output, then change the deploy job’s
image reference to use needs.build-and-push.outputs.image_tag instead of
github.ref_name. Preserve the existing registry and repository components.
- Around line 81-82: Update the “Checkout code” step using actions/checkout@v4
to set persist-credentials to false, ensuring the workflow does not retain
GITHUB_TOKEN credentials in the local Git configuration.
In `@orchestration/infrastructure/terraform/runtime/main.tf`:
- Around line 104-107: Update the ENVIRONMENT value in the runtime env block
from staging to production, using the repository’s established production
environment variable if one exists; leave the surrounding Container App
configuration unchanged.
- Around line 78-80: Update the azurerm_container_app.api image ownership so
Terraform does not overwrite workflow or manual image deployments: either route
post-bootstrap image changes through Terraform, or add a lifecycle
ignore_changes rule targeting template[0].container[0].image for runtime-managed
deployments. Preserve var.initial_image for initial provisioning.
In `@orchestration/infrastructure/terraform/runtime/variables.tf`:
- Around line 53-56: Update the initial_image configuration used by the
Container App bootstrap so fresh deployments do not depend on the stack-created
pvcprodcodeacr registry or a nonexistent master tag. Use a guaranteed
pre-existing bootstrap image, or explicitly stage image publication before
Container App creation, while preserving the override path for later deployment
images.
In `@orchestration/infrastructure/terraform/website/variables.tf`:
- Line 14: Before applying the new defaults for resource_group_name and
static_web_app_name, verify whether production Terraform state already contains
differently named resources. If so, migrate or import the existing resources
into the addresses used by website/main.tf, then review the plan to ensure no
resource group, Static Web App, or custom domain is destroyed; proceed with the
defaults only for confirmed fresh state.
In `@scripts/setup-azure-auth-for-pipeline.ps1`:
- Around line 71-75: Update the role assignment flow around the Contributor
assignment to resolve the Container App resource ID for pvc-prod-codeflow-api
and grant the GitHub identity Container Apps Contributor scoped to that resource
instead of Contributor scoped to the deployment resource group. Preserve the
existing AcrPush assignment at the Azure Container Registry scope.
---
Nitpick comments:
In `@orchestration/infrastructure/terraform/runtime/main.tf`:
- Around line 15-23: Update the azurerm_container_registry.runtime resource to
explicitly define the production security controls: set the intended public
network access behavior and configure image scanning, quarantine, and
trusted-image settings according to the approved external scanning/signing and
network-isolation plan. If public access is disabled, also provision the private
connectivity required by the GitHub-hosted build runner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 95af2636-038a-42a9-9a12-57874fb53a01
📒 Files selected for processing (16)
.azure/pipeline-setup.md.github/workflows/deploy-autopr-engine.yml.github/workflows/pr-comment-handler.yml.github/workflows/validate-templates.yml.gitignoreorchestration/infrastructure/README.mdorchestration/infrastructure/terraform/runtime/.terraform.lock.hclorchestration/infrastructure/terraform/runtime/README.mdorchestration/infrastructure/terraform/runtime/main.tforchestration/infrastructure/terraform/runtime/outputs.tforchestration/infrastructure/terraform/runtime/providers.tforchestration/infrastructure/terraform/runtime/variables.tforchestration/infrastructure/terraform/website/.terraform.lock.hclorchestration/infrastructure/terraform/website/main.tforchestration/infrastructure/terraform/website/variables.tfscripts/setup-azure-auth-for-pipeline.ps1
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/validate-templates.yml
- .azure/pipeline-setup.md
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files .github/workflows/deploy-autopr-engine.yml || true
echo
echo "Workflow around checkout and later steps:"
if [ -f .github/workflows/deploy-autopr-engine.yml ]; then
nl -ba .github/workflows/deploy-autopr-engine.yml | sed -n '1,140p'
fi
echo
echo "Search for git commands and checkout arguments in workflow:"
if [ -f .github/workflows/deploy-autopr-engine.yml ]; then
rg -n "git |checkout@v4|persist-credentials|GITHUB_TOKEN|github_token|gitlab|npm|make|cargo|docker|gh " .github/workflows/deploy-autopr-engine.yml || true
fiRepository: phoenixvc/codeflow-engine
Length of output: 297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Workflow content:"
awk '{ printf "%6d\t%s\n", NR, $0 }' .github/workflows/deploy-autopr-engine.yml
echo
echo "Search relevant commands:"
grep -Enio "git[ /]|checkout@v4|persist-credentials|GITHUB_TOKEN|github_token|npm|make|cargo|docker|gh |curl" .github/workflows/deploy-autopr-engine.yml || trueRepository: phoenixvc/codeflow-engine
Length of output: 7357
Disable persisted checkout credentials.
Only Docker builds and Azure updates follow this checkout, with no authenticated Git access needed. Set persist-credentials: false so GITHUB_TOKEN is not stored in local Git config during the build.
Proposed fix
- name: Checkout code
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 81-82: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/deploy-autopr-engine.yml around lines 81 - 82, Update the
“Checkout code” step using actions/checkout@v4 to set persist-credentials to
false, ensuring the workflow does not retain GITHUB_TOKEN credentials in the
local Git configuration.
Source: Linters/SAST tools
| env: | ||
| RESOURCE_GROUP: ${{ vars.AZURE_RESOURCE_GROUP }} | ||
| CONTAINER_APP: ${{ vars.AZURE_CONTAINER_APP }} | ||
| IMAGE: ${{ vars.AZURE_CONTAINER_REGISTRY }}.azurecr.io/${{ env.IMAGE_REPOSITORY }}:${{ github.ref_name }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file =="
wc -l .github/workflows/deploy-autopr-engine.yml || true
echo "== relevant workflow lines =="
sed -n '1,240p' .github/workflows/deploy-autopr-engine.yml || true
echo "== refs to deploy/build jobs and metadata =="
rg -n "metadata|IMAGE_REPOSITORY|build-and-push|needs\.|github\.ref_name|branches|INPUT_REF|pull_request|push|manual|workflow_dispatch|meta\.outputs" .github/workflows/deploy-autopr-engine.ymlRepository: phoenixvc/codeflow-engine
Length of output: 7210
🌐 Web query:
docker/metadata-action v5 type=ref,event=branch manual workflow_dispatch outputs.version
💡 Result:
In the docker/metadata-action (v5), the type=ref configuration is used to generate tags based on Git references for events such as branch, tag, and pull_request [1][2]. Crucially, workflow_dispatch is a GitHub event, not a valid ref type for the type=ref configuration [3]. While workflow_dispatch events are sometimes referenced in documentation tables to show how they behave when they trigger a workflow (e.g., they may associate with a branch reference), you cannot use type=ref,event=workflow_dispatch [3][4]. If you are attempting to use a manual input (such as an inputs.version from a workflow_dispatch event) as a tag, you should use the type=raw configuration with a custom value, rather than type=ref [3]. Example of using a manual input for tagging: tags: | type=raw,value=${{ github.event.inputs.version }} Alternatively, if you are using semver patterns with a manual version input, you can use the value attribute [5]: tags: | type=semver,pattern={{major}}.{{minor}}.{{patch}},value=${{ github.event.inputs.version }} The outputs.tags and outputs.labels generated by the action can then be accessed in subsequent steps, such as in the docker/build-push-action, using ${{ steps.meta.outputs.tags }} [1][6].
Citations:
- 1: https://github.com/docker/metadata-action/blob/v5/README.md
- 2: https://github.com/marketplace/actions/docker-metadata-action
- 3: Invalid event for type=ref,event=workflow_dispatch docker/metadata-action#378
- 4: Confusing documentation about tags docker/metadata-action#366
- 5: Version metadata not working in v5.8.0 docker/metadata-action#543
- 6: https://docs.docker.com/build/ci/github-actions/manage-tags-labels/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== metadata action outputs/tags relevant docs =="
python3 - <<'PY'
from urllib.request import urlopen, Request
url = "https://raw.githubusercontent.com/docker/metadata-action/v5/README.md"
req = Request(url, headers={"User-Agent": "CodeRabbit-review-verification"})
text = urlopen(req, timeout=20).read().decode()
lines = text.splitlines()
for i, line in enumerate(lines):
if "outputs.tags" in line or "outputs.version" in line or "type=ref,event=branch" in line:
start=max(0, i-3); end=min(len(lines), i+8)
print(f"--- around line {i+1} ---")
for j in range(start,end):
print(f"{j+1:4}: {lines[j]}")
print()
PYRepository: phoenixvc/codeflow-engine
Length of output: 2806
🌐 Web query:
github docker metadata-action v5 source generate tags refs branch manual run ref_name
💡 Result:
When using docker/metadata-action in a GitHub Actions workflow triggered by a manual event like workflow_dispatch, you cannot directly use type=ref,event=branch (or other ref-based types) to automatically derive tags from the manual run's reference, as these types are designed for specific Git-event triggers (push, pull_request, etc.) [1][2][3]. To generate tags during a manual workflow_dispatch run, use type=raw and explicitly pass the reference name via the value attribute [4][3]. Example implementation: 1. Define an input in your workflow for the tag or branch name [3]. 2. Use type=raw in your metadata-action step to consume that input [3]. workflow configuration snippet: on: workflow_dispatch: inputs: tag_name: description: 'Tag or branch name to build' required: true default: 'main' jobs: docker: runs-on: ubuntu-latest steps: - name: Docker meta id: meta uses: docker/metadata-action@v5 with: images: name/app tags: | type=raw,value=${{ github.event.inputs.tag_name }} # Or use github.ref_name if the manual run is triggered on a specific branch # type=raw,value=${{ github.ref_name }} This approach bypasses the restrictions on ref-based tag generation by manually providing the value that the action should use for the image tag [4][3]. If you do not have a specific input, you can use ${{ github.ref_name }} directly as the value for type=raw, provided the workflow was triggered against the intended branch or tag context [1][3].
Citations:
- 1: https://github.com/docker/metadata-action
- 2: https://github.com/docker/metadata-action?tab=readme-ov-file
- 3: Invalid event for type=ref,event=workflow_dispatch docker/metadata-action#378
- 4: `push tag` event to write semver tags isn't triggered because tags are pushed using `GITHUB_TOKEN` docker/metadata-action#503
🌐 Web query:
site:github.com/docker/metadata-action docker-metadata-action src tags branch refs manual branch sanitize
💡 Result:
The docker/metadata-action automatically handles tag sanitization to ensure compliance with Docker image tag specifications [1][2]. When generating tags—particularly from dynamic sources like branches or pull request references (via type=ref)—the action performs the following sanitization steps: 1. Character Replacement: It replaces sequences of invalid characters with a hyphen (-) [1][3]. While Docker tag specifications allow for lowercase/uppercase letters, digits, underscores, periods, and dashes, they have specific restrictions on positioning [1][4]. 2. Leading Character Stripping: The action strips leading dots (.) and hyphens (-) from tags, as these are not permitted as the first character of a Docker tag [4]. 3. Lowercasing: Although image names are strictly lowercased, the action typically preserves the casing of the input source (like branch names) unless specific configurations or behaviors apply [5]. Users sometimes encounter issues if the source reference (e.g., a branch name with uppercase letters) results in a tag that is rejected by a downstream registry or tool that expects strictly lowercase repository names [5]. For manual tag generation or when using raw inputs (e.g., type=raw), the action applies these same sanitization rules to ensure the resulting metadata is valid [4]. If a raw input sanitizes to an empty string, the action generally avoids creating an empty tag, often falling back to other defined values [4]. In summary, the action does not require manual sanitization of branch names or refs; it automatically processes them to meet Docker's syntax requirements [1][3]. If you are experiencing issues with tag validity in downstream processes (such as image pushes), it is often due to registry-specific requirements (like strict lowercasing) rather than a failure of the action's built-in sanitization logic [5].
Citations:
- 1: https://github.com/docker/metadata-action/blob/v5/README.md
- 2: https://github.com/docker/metadata-action
- 3: Tags for branch names containing "#" parsed incorrectly ignoring the "#" docker/metadata-action#199
- 4: fix: strip leading dots and hyphens from raw tags docker/metadata-action#646
- 5: refs with uppercase characters trigger an error in buildx (branch names with uppercase characters) docker/metadata-action#405
🌐 Web query:
docker metadata-action README outputs.version type ref github.ref_name workflow_dispatch
💡 Result:
In the docker/metadata-action GitHub Action, the version output is a string representing the generated Docker image version [1][2][3]. This version is derived based on the event that triggered the workflow and the Git reference provided [4][5]. Regarding the interaction between github.ref_name and workflow_dispatch: 1. Version Derivation: The docker/metadata-action automatically extracts metadata from the GitHub context, including the branch or tag name [5]. When a workflow is triggered by workflow_dispatch, it typically runs on the branch specified in the dispatch event (or the default branch if none is specified), and the action uses the corresponding Git reference to generate the version string [5]. 2. Event Behavior: For a workflow_dispatch event, the reference (github.ref_name) is the name of the branch from which the workflow was triggered [5]. The metadata-action uses this reference to help construct the image tags and the version output [5]. 3. Configuration: While the action automatically infers the version, you can influence how it is generated using the flavor and tags inputs in your workflow configuration to specify exactly how you want the version string or image tags to be formatted [1][3]. You can access the version output in your workflow using ${{ steps.<your_action_id>.outputs.version }} [1][3]. If you need more complex data, the json output provides a full object containing tags and labels that you can parse using fromJSON() [1][3].
Citations:
- 1: https://github.com/docker/metadata-action?tab=readme-ov-file
- 2: https://github.com/docker/metadata-action/blob/c94c54c4e6ef3734c17f3f3aa488848d7f9b50d9/action.yml
- 3: https://github.com/docker/metadata-action/blob/v5/README.md
- 4: https://github.com/docker/metadata-action
- 5: https://github.com/marketplace/actions/docker-metadata-action?version=v4.5.0
Deploy the exact tag emitted by the build job.
This workflow builds with steps.meta.outputs.tags but deploys by reconstructing the image tag from github.ref_name in later jobs. For manual dispatches or branch names containing characters that should be rewritten for Docker tag safety, the deploy can point at an image that was not built. Export steps.meta.outputs.version from build-and-push and use needs.build-and-push.outputs.image_tag in the deploy job, or at least tag all workflow_dispatch runs explicitly with the same type=ref,event=branch behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/deploy-autopr-engine.yml at line 151, Update the
build-and-push job to expose the exact image version from
steps.meta.outputs.version as its image_tag output, then change the deploy job’s
image reference to use needs.build-and-push.outputs.image_tag instead of
github.ref_name. Preserve the existing registry and repository components.
| container { | ||
| name = "codeflow-engine" | ||
| image = var.initial_image |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline/size =="
wc -l orchestration/infrastructure/terraform/runtime/main.tf 2>/dev/null || true
echo "== relevant main.tf lines =="
sed -n '1,140p' orchestration/infrastructure/terraform/runtime/main.tf 2>/dev/null || true
echo "== search initial_image and workflow/image refs =="
rg -n "initial_image|workflow|image|ContainerApp|containerregistry|azure\.containerapps|azuredevops|github\.actions" orchestration/infrastructure -S || true
rg -n "initial_image|workflow-deploy|production dispatch|dispatch|ref|image" . -g '!**/.git/**' -g '!**/node_modules/**' -S | head -200
echo "== git diff stat if available =="
git diff --stat HEAD 2>/dev/null || true
git diff -- orchestration/infrastructure/terraform/runtime/main.tf 2>/dev/null | sed -n '1,220p' || trueRepository: phoenixvc/codeflow-engine
Length of output: 42293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files matching deploy-codeflow-engine or similar =="
git ls-files .github orchestration/infrastructure -z | tr '\0' '\n' | rg '(\.github/workflows/.*(deploy-codeflow-engine|deploy).*\.ya?ml|deploy-codeflow-engine)' || true
echo "== deploy workflow outline/content =="
fd -p '.github/workflows/deploy-codeflow-engine.yml' . -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {} 2>/dev/null || true
echo "== terraform lifecycle directives =="
rg -n "lifecycle|ignore_changes|azurerm_container_app|initial_image" orchestration/infrastructure/terraform -S || true
echo "== runtime variables/docs relevant =="
sed -n '1,90p' orchestration/infrastructure/terraform/runtime/variables.tf
sed -n '1,30p' orchestration/infrastructure/terraform/runtime/README.mdRepository: phoenixvc/codeflow-engine
Length of output: 4235
Prevent Terraform from reverting workflow-deployed images.
azurerm_container_app.api still defaults the image to var.initial_image, so any workflow/manual update to the same Container App can be overwritten by the next terraform apply. Give one system ownership of the image: route post-bootstrap deploys through Terraform, or add lifecycle { ignore_changes = [template[0].container[0].image] } for runtime-managed deploys.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestration/infrastructure/terraform/runtime/main.tf` around lines 78 - 80,
Update the azurerm_container_app.api image ownership so Terraform does not
overwrite workflow or manual image deployments: either route post-bootstrap
image changes through Terraform, or add a lifecycle ignore_changes rule
targeting template[0].container[0].image for runtime-managed deployments.
Preserve var.initial_image for initial provisioning.
| env { | ||
| name = "ENVIRONMENT" | ||
| value = "staging" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Set the production runtime environment correctly.
The resources, tags, documentation, and deployment workflow target production, but the Container App sets ENVIRONMENT=staging. This can select staging behavior in the production deployment; use production or an explicit production variable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestration/infrastructure/terraform/runtime/main.tf` around lines 104 -
107, Update the ENVIRONMENT value in the runtime env block from staging to
production, using the repository’s established production environment variable
if one exists; leave the surrounding Container App configuration unchanged.
| variable "initial_image" { | ||
| description = "Codeflow Engine image deployed by default. Override only for bootstrap scenarios." | ||
| type = string | ||
| default = "pvcprodcodeflowacr.azurecr.io/codeflow-engine:master" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== file list ==="
git ls-files | rg '^orchestration/infrastructure/terraform/|workflows|pipeline|cd|deploy|yaml' || true
echo "=== target variables ==="
sed -n '1,140p' orchestration/infrastructure/terraform/runtime/variables.tf 2>/dev/null || true
echo "=== main.tf relevant ==="
sed -n '1,260p' orchestration/infrastructure/terraform/runtime/main.tf 2>/dev/null || true
echo "=== search initial_image usage ==="
rg -n 'initial_image|codeflow-engine|master|ContainerApp|container_app|codeflow acr|acr|codeflowprod|pvcprodcodeflowacr|azurecr\.io|workflow|push|build' orchestration/infrastructure/terraform -S || true
echo "=== workflows likely locations ==="
fd -t f '(\.ya?ml|\.tf|tfvars|github_actions|actions|container|docker|acr|deploy|release)' . | sed -n '1,200p'Repository: phoenixvc/codeflow-engine
Length of output: 25850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== infrastructure deploy workflow ==="
sed -n '1,260p' orchestration/infrastructure/.github/workflows/deploy.yml 2>/dev/null || true
echo "=== terraform runtime README bootstrap instructions ==="
sed -n '1,220p' orchestration/infrastructure/terraform/runtime/README.md 2>/dev/null || true
echo "=== root terraform vars relevant to runtime/image ==="
sed -n '1,220p' orchestration/infrastructure/terraform/variables.tf 2>/dev/null || true
echo "=== terraform outputs/dependencies if any in parent terraform ==="
sed -n '1,180p' orchestration/infrastructure/terraform/outputs.tf 2>/dev/null || true
echo "=== workflow references to runtime image/image push/container app ==="
rg -n 'runtime|initial_image|acr/login|docker/login|docker build|docker push|master:|codeflow-engine|container app|az containerapp|deploy.yml|workflow_dispatch|workflow_run|rerun' orchestration/infrastructure/.github/workflows orchestration/pipeline orchestration/deploy orchestration/scripts -S || true
echo "=== provider terraform version constraints ==="
rg -n 'terraform|azure|registry\.terraform\.io/hashicorp/azurerm|version|required_providers' orchestration/infrastructure/terraform -S || trueRepository: phoenixvc/codeflow-engine
Length of output: 20356
Make the Container App bootstrap order explicit.
initial_image defaults to pvcprodcodeflowacr.azurecr.io/codeflow-engine:master, where pvcprodcodeflowacr is created by runtime/main.tf in the same Terraform stack. On a fresh deploy with no existing image tag, Container App creation can fail unless a guaranteed bootstrap image is used first, or image push and app creation are split into explicit stages.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestration/infrastructure/terraform/runtime/variables.tf` around lines 53
- 56, Update the initial_image configuration used by the Container App bootstrap
so fresh deployments do not depend on the stack-created pvcprodcodeacr registry
or a nonexistent master tag. Use a guaranteed pre-existing bootstrap image, or
explicitly stage image publication before Container App creation, while
preserving the override path for later deployment images.
| description = "Resource group for Codeflow website hosting." | ||
| type = string | ||
| default = "prod-rg-san-codeflow" | ||
| default = "pvc-prod-codeflow-rg" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '^orchestration/infrastructure/terraform/website/(variables\.tf|main\.tf|.*\.tf)$' || true
echo "== variables.tf =="
if [ -f orchestration/infrastructure/terraform/website/variables.tf ]; then
cat -n orchestration/infrastructure/terraform/website/variables.tf
fi
echo "== main.tf outline/snippets =="
if [ -f orchestration/infrastructure/terraform/website/main.tf ]; then
wc -l orchestration/infrastructure/terraform/website/main.tf
ast-grep outline orchestration/infrastructure/terraform/website/main.tf || true
echo "== resource usages =="
rg -n 'resource_group|static_web|domain|var\.(resource|webapp|website).*|azure(azurerm)_|name\s*=' orchestration/infrastructure/terraform/website/main.tf
fi
echo "== terraform website references/imports =="
rg -n 'resource_group_name\s*=' -S . --glob 'orchestration/infrastructure/terraform/website/*.tf' || true
rg -n 'resource_group_name|website|static_app|domains|azure_static_web_app|azurerm_.*domain|import |terraform import|moved ' orchestration/infrastructure/terraform/website || true
echo "== git diff stat and relevant variables/diffs =="
git diff --stat || true
git diff -- orchestration/infrastructure/terraform/website/variables.tf || true
git diff -- orchestration/infrastructure/terraform/website/main.tf || trueRepository: phoenixvc/codeflow-engine
Length of output: 6237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status =="
git status --short || true
echo "== tfstate and module references =="
rg -n 'pvc-prod-codeflow-rg|prod-codeflow|static_app|webapp|azure_static_web_app|azurerm_resource_group|azurerm_domain|location\s*=' -S orchestration/infrastructure/terraform/website --glob '*.tf' --glob '*.tfvars' --glob 'terraform.tfstate' --glob '*.tfstate' || true
echo "== provider versions/features =="
for f in $(git ls-files 'orchestration/infrastructure/terraform/website/*.tf'); do
echo "--- $f ---"
rg -n 'terraform|required_providers|azurerm|provider "azurerm"|for_each|for_each_metadata|lifecycle|prevent_destroy|ignore_changes|create_before_destroy|count\s*=|resource\s+"' "$f" || true
doneRepository: phoenixvc/codeflow-engine
Length of output: 2059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re, hashlib
root = Path('orchestration/infrastructure/terraform/website')
vars_text = (root/'variables.tf').read_text() if (root/'variables.tf').exists() else ''
main_text = (root/'main.tf').read_text() if (root/'main.tf').exists() else ''
rgs = [(r'variable\s+"([^"]+)"\s*\{', vars_text),
(r'variable\s+"([^"]+)"\s*\{', vars_text),
(r'default\s*=\s*"([^"]+)"', vars_text)]
decls = re.findall(r'variable\s+"(\w+)"\s*\{\s*default\s*=\s*"([^"]+)"', vars_text, re.S)
uses = [(m.start(), 'var.' + g) for m in re.finditer(r'\bvar\.(\w+)', main_text) for g in [m.group(1)]]
print('declared_defaults=', decls)
print('var_usages_by_count=', {g: len(m.group(0) for m in re.finditer(r'\bvar\.('+g+')\b', main_text)) for g in set(g for _,g in uses)})
for name, default in decls:
if name in set(g for _,g in uses):
print(f'{name}: default={default!r} used {sum(1 for g,_ in uses if g==name)} times')
PYRepository: phoenixvc/codeflow-engine
Length of output: 351
Migrate existing website resources before changing these defaults.
website/main.tf declares azurerm_resource_group.website.name = var.resource_group_name and azurerm_static_web_app.website.name = var.static_web_app_name. If production state contains different resource group or Static Web App names, applying these defaults can replace those resources and may destroy the existing resource group, SWA, and custom domain. Confirm this is a fresh state or run an explicit Terraform import/state migration and review the destroy plan before applying.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestration/infrastructure/terraform/website/variables.tf` at line 14,
Before applying the new defaults for resource_group_name and
static_web_app_name, verify whether production Terraform state already contains
differently named resources. If so, migrate or import the existing resources
into the addresses used by website/main.tf, then review the plan to ensure no
resource group, Static Web App, or custom domain is destroyed; proceed with the
defaults only for confirmed fresh state.
| Invoke-Az role assignment create ` | ||
| --assignee-object-id $identity.principalId ` | ||
| --assignee-principal-type ServicePrincipal ` | ||
| --role Contributor ` | ||
| --scope $deploymentRgId | Out-Null |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
az role definition list \
--name "Container Apps Contributor" \
--query '[0].permissions[0].actions' \
--output jsoncRepository: phoenixvc/codeflow-engine
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file exists and relevant lines =="
if [ -f scripts/setup-azure-auth-for-pipeline.ps1 ]; then
nl -ba scripts/setup-azure-auth-for-pipeline.ps1 | sed -n '1,140p'
else
echo "scripts/setup-azure-auth-for-pipeline.ps1 not found"
fd -a 'setup-azure-auth-for-pipeline\.ps1$' .
fi
echo
echo "== search for relevant Azure auth/role assignments =="
rg -n "Invoke-Az role assignment create|AcrPush|Container Apps|Container App|deploymentRgId|DeploymentResourceGroup|show identity" . -S --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: phoenixvc/codeflow-engine
Length of output: 240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
find . -name 'setup-azure-auth-for-pipeline.ps1' -print
echo
echo "== relevant file excerpt =="
awk '{ printf("%6d %s\n", NR, $0) }' scripts/setup-azure-auth-for-pipeline.ps1 | sed -n '1,140p'
echo
echo "== search relevant terms =="
grep -RIn -E 'Invoke-Az role assignment create|AcrPush|Container Apps|Container App|deploymentRgId|DeploymentResourceGroup|assignee-object-id|\$identity' . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: phoenixvc/codeflow-engine
Length of output: 4201
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files mentioning Container App or auth env vars =="
find . -type f \
\( -path './.workflow*/*' -o -path './.github/workflows/*' -o -name '*.yml' -o -name '*.yaml' -o -name '*.ps1' \) \
-print | sort | while read -r f; do
if grep -RInE 'AZURE_CONTAINER_APP|AZURE_RESOURCE_GROUP|AZURE_CONTAINER_REGISTRY|AZURE_TENANT_ID|AZURE_CLIENT_ID|Container App|ContainerApps|az containerapp|az container app|github/codeql|codeql|Azure CLI' "$f" >/dev/null 2>&1; then
echo "--- $f ---"
grep -RInE 'AZURE_CONTAINER_APP|AZURE_RESOURCE_GROUP|AZURE_CONTAINER_REGISTRY|AZURE_TENANT_ID|AZURE_CLIENT_ID|Container App|ContainerApps|az containerapp|container app|github/codeql|Azure CLI' "$f" || true
fi
done | sed -n '1,240p'
echo
echo "== exact references to container app/resource group =="
grep -RInE 'pvc-prod-codeflow-api|pvc-prod-codeflow-rg|pvcprodcodeflowacr|Azure/container apps|Configure Azure Credentials' . --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build | sed -n '1,240p'Repository: phoenixvc/codeflow-engine
Length of output: 8922
🌐 Web query:
Microsoft Learn Container Apps Contributor built-in role actions container apps show update az containerapp role
💡 Result:
The Container Apps Contributor built-in role provides full management access to Azure Container Apps resources [1][2]. Its primary permitted actions include the ability to create, update, delete, and read Container Apps, as well as interact with associated managed environments [3]. Key Actions for the Container Apps Contributor Role (ID: 358470bc-b998-42bd-ab17-a7e34c199c0f): - Microsoft.App/containerApps//action, read, write, delete: Grants full lifecycle management for Container Apps [3]. - Microsoft.App/managedEnvironments/join/action: Allows joining a container app or job to a managed environment [3]. - Microsoft.App/managedEnvironments/read: Grants read access to managed environments [3]. - Microsoft.App/connectedEnvironments//read: Grants read access to connected environments [3]. - Microsoft.Authorization//read: Allows reading roles and role assignments [3]. - Microsoft.Insights/alertRules/: Permits creation and management of alert rules [3]. Regarding the az containerapp role command: There is no native az containerapp role command in the Azure CLI. The Azure CLI command group for managing RBAC roles is az role assignment. If you are looking to list or manage role assignments for a container app, you should use: - To list role assignments for a specific container app: az role assignment list --scope /subscriptions/{subscriptionId}/resourceGroups/{rgName}/providers/Microsoft.App/containerApps/{appName} Technical Note: While the Container Apps Contributor role grants broad management permissions, users may sometimes encounter issues when updating specific configurations (e.g., networking or secrets) that are scoped to the parent Managed Environment [4]. In such cases, additional permissions (such as Managed Environments Contributor or network-related permissions) may be required depending on the scope of the operation [4].
Citations:
- 1: https://learn.microsoft.com/en-gb/azure/role-based-access-control/built-in-roles
- 2: https://learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
- 3: https://www.azadvertizer.net/azrolesadvertizer/358470bc-b998-42bd-ab17-a7e34c199c0f.html
- 4: Contributor role on container app is not enough to update containers environment variables microsoft/azure-container-apps#1414
Scope deployment access to the Container App.
This grants the GitHub identity Contributor over all of pvc-prod-codeflow-rg, while the workflow only updates pvc-prod-codeflow-api and pulls from ACR. Resolve the app resource ID and assign Container Apps Contributor at that scope; keep AcrPush at the registry scope.
Proposed direction
+$containerAppId = Invoke-Az containerapp show `
+ --name "pvc-prod-codeflow-api" `
+ --resource-group $DeploymentResourceGroup `
+ --query id `
+ --output tsv
+
Invoke-Az role assignment create `
--assignee-object-id $identity.principalId `
--assignee-principal-type ServicePrincipal `
- --role Contributor `
- --scope $deploymentRgId | Out-Null
+ --role "Container Apps Contributor" `
+ --scope $containerAppId | Out-Null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Invoke-Az role assignment create ` | |
| --assignee-object-id $identity.principalId ` | |
| --assignee-principal-type ServicePrincipal ` | |
| --role Contributor ` | |
| --scope $deploymentRgId | Out-Null | |
| $containerAppId = Invoke-Az containerapp show ` | |
| --name "pvc-prod-codeflow-api" ` | |
| --resource-group $DeploymentResourceGroup ` | |
| --query id ` | |
| --output tsv | |
| Invoke-Az role assignment create ` | |
| --assignee-object-id $identity.principalId ` | |
| --assignee-principal-type ServicePrincipal ` | |
| --role "Container Apps Contributor" ` | |
| --scope $containerAppId | Out-Null |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/setup-azure-auth-for-pipeline.ps1` around lines 71 - 75, Update the
role assignment flow around the Contributor assignment to resolve the Container
App resource ID for pvc-prod-codeflow-api and grant the GitHub identity
Container Apps Contributor scoped to that resource instead of Contributor scoped
to the deployment resource group. Preserve the existing AcrPush assignment at
the Azure Container Registry scope.
1 participant
Summary
Validation
Deployment note
This PR does not provision Azure resources and should not be treated as the production deploy itself. The current production deploy blocker remains external: the GitHub production environment does not exist yet, the target Codeflow resource group was not visible in the active Azure subscription, and the legacy AZURE_CREDENTIALS secret could not authenticate to any subscription.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation