[DevOps] PR Builds: PR deployment and cleanup workflows#2033
[DevOps] PR Builds: PR deployment and cleanup workflows#2033
Conversation
Creates two GitHub Actions workflows for per-PR template deployments:
ftk-pr-deploy.yml:
- Triggers on PRs that change src/templates/** files
- Parses PR body checkboxes to determine deployment variants
- Supports: ADX (managed), Fabric (manual), manual, no-data, workbooks, alerts
- Each variant deploys in parallel with isolated resource groups (pr-{number}-{variant})
- Posts PR comments with deployment status and portal links
- Supports fork PRs via "Needs: Deployment" label + pull_request_target
- Verifies fork PRs don't modify .github/ files
ftk-pr-cleanup.yml:
- Triggers on PR close (merged or not)
- Deletes all resource groups matching pr-{number}-*
- Posts cleanup confirmation comment
- Creates GitHub issue assigned to PR author if cleanup fails
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds GitHub Actions workflows to support per-PR Azure template deployments (FinOps hubs + variants, workbooks, alerts) and automatic cleanup of PR-specific resource groups, aligning with the toolkit’s CI automation goals for validating src/templates/** changes.
Changes:
- Introduces
ftk-pr-deploy.ymlto parse PR-body deployment checkboxes and run selected deployments with PR-scoped resource groups. - Introduces
ftk-pr-cleanup.ymlto delete PR-scoped resource groups on PR close and notify via PR comment / issue on failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
.github/workflows/ftk-pr-deploy.yml |
New per-PR deployment workflow (checkbox parsing, fork label gating, parallel deployment jobs, PR comments). |
.github/workflows/ftk-pr-cleanup.yml |
New cleanup workflow to remove pr-{number}-* resource groups and report failures. |
- Fix PR body script injection: use env var instead of inline interpolation - Escape literal + in regex patterns for checkbox matching - Fix azure/login: use @v2 release tag instead of branch ref - Fix job name typo: "Hubs + (manual)" → "Hubs (manual)" - Remove unused any-selected output Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Tighten pull_request_target condition to only fire on Needs: Deployment label - Expand fork safety check to block src/scripts/ changes - Add Fabric URI validation before deploy (fails fast as first step) - Change cleanup trigger to pull_request_target for fork PR secret access - Add -ErrorAction Stop to Remove-AzResourceGroup for proper error handling - Fix assignee fallback for external contributors on cleanup failure issues 🤖 Generated with [Claude Code](https://claude.ai/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
🤖 [AI][Claude] PR Update Summary Addressed: 9 thread(s)
Changes:
Push-backs (code already correct):
|
- Fix fabric-uri script injection: pass via env var instead of direct interpolation
- Add environment approval gate comment for fork PR security
- Add timeout-minutes: 60 to all deployment jobs
- Centralize Az module versions as workflow-level env vars
- Cache Az modules in check-options, restore in deployment jobs
- Update Deploy-Hub calls to use -PR {number} -Name "{variant}"
🤖 Generated with [Claude Code](https://claude.ai/claude-code)
Co-Authored-By: RolandKrummenacher <RolandKrummenacher@users.noreply.github.com>
Co-Authored-By: Claude <noreply@anthropic.com>
|
🤖 [AI][Claude] PR Update Summary Addressed: 5 thread(s)
Also updated all |
There was a problem hiding this comment.
Pending environment setup and testing...
@MSBrett I need your help here. Let's chat.
MSBrett
left a comment
There was a problem hiding this comment.
Security review — fork PR deployment safety
CI is green and the workflow design is solid, but we found a critical security gap in the fork PR safety check that needs addressing before merge.
Blocker: fork PRs can deploy arbitrary code via src/templates/ modifications
File: .github/workflows/ftk-pr-deploy.yml (~line 67-80)
The pull_request_target safety check blocks .github/ and src/scripts/ but not src/templates/. Since deploy jobs check out github.event.pull_request.head.sha, a fork PR can modify Bicep templates and inject a Microsoft.Resources/deploymentScripts resource that runs arbitrary PowerShell with the hub managed identity / OIDC credentials.
The workflow comment states that the ftk-pr environment requires reviewers, but the environment currently has zero protection rules (verified via gh api repos/microsoft/finops-toolkit/environments/ftk-pr --jq ".protection_rules").
Suggested fix
Replace the existing fork safety step with one that also blocks src/templates/ modifications and verifies environment protection:
- name: Verify fork PR safety
if: github.event.pull_request.head.repo.full_name != github.repository
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
# 1. Block fork PRs from modifying privileged paths
CHANGED_FILES=$(gh pr diff ${{ github.event.pull_request.number }} --name-only)
if echo "$CHANGED_FILES" | grep -E '^(\.github/|src/scripts/|src/templates/)' > /dev/null; then
echo '::error::Fork PRs may not modify .github/, src/scripts/, or src/templates/ when using PR deployment workflows.'
exit 1
fi
# 2. Refuse to deploy if ftk-pr environment lacks required reviewers
protection=$(gh api repos/${{ github.repository }}/environments/ftk-pr --jq '.protection_rules | length')
if [ "$protection" = '0' ]; then
echo '::error::ftk-pr environment has no protection rules. Configure required reviewers before fork PR deployment is permitted.'
exit 1
fi
Alternative (preferred): configure the ftk-pr GitHub Environment with required reviewers — that single setting eliminates the blocker without code changes. The workflow comment already documents this requirement; it just is not enforced. The code fix above is defense-in-depth.
Non-blocking recommendations
- Add Fabric URI format validation before passing to Deploy-Hub
- Add concurrency block to
ftk-pr-cleanup.yml
Clean checks
- Permissions least-privilege ✓
- PR body parsing safe (env passthrough, no shell interpolation) ✓
- RG naming safe ✓
- Cleanup author assignment verified ✓
- Targets dev ✓, size within limits (2 files, ~522 LOC) ✓
|
@@flanakin: you have some new feedback! Please review and resolve all comments and I'll let reviewers know by removing the |
🛠️ Description
Adds two GitHub Actions workflows for per-PR template deployment CI:
ftk-pr-deploy.ymlsrc/templates/**files (excluding docs/tests)pr-{number}-{variant}):Needs: Deploymentlabel +pull_request_target(verifies no.github/modifications)ftk-pr-cleanup.ymlpr-{number}-*Dependencies
-PR,-Scope,-ManagedExportsparametersThis is PR D (final) of a multi-PR effort to add per-PR deployment CI.
📋 Checklist
🔬 How did you test this change?
📦 Deploy to test?
🙋♀️ Do any of the following that apply?
📑 Did you update
docs/changelog.md?📖 Did you update documentation?