Skip to content

ci(tf-integration): trigger on PR approve, not every push - #1272

Open
LeonardCareer wants to merge 1 commit into
mainfrom
leonarddu/merge-gate-tf-integration
Open

ci(tf-integration): trigger on PR approve, not every push#1272
LeonardCareer wants to merge 1 commit into
mainfrom
leonarddu/merge-gate-tf-integration

Conversation

@LeonardCareer

Copy link
Copy Markdown
Collaborator

What

Move the Terraform Integration workflow from being triggered on every push (pull_request_target) to being triggered only when a reviewer submits an approval (pull_request_review).

Why

Per scrum discussion — every fork PR push currently spams every environment reviewer with a deployment review requested email (one per matrix job, tens of emails per push). Moving the trigger to "review submitted" means reviewers only get the approval request once, after they've read the code.

Tradeoff Lei raised: tf plan results are no longer visible pre-review. Consensus was to try this and see how it goes.

How

  • Trigger: pull_request_targetpull_request_review: types: [submitted]
  • New gate job with two conditions:
    • if: github.event.review.state == 'approved' — skip on comment/request-changes
    • Uses gh api to fetch PR file list, sets run=true only if any file matches .tf$, .tfvars$, modules/terraform/**, or scenarios/*/terraform-(inputs|test-inputs)/**
  • terraform-test and setup-matrix now needs: gate + if: needs.gate.outputs.run == 'true'
  • terraform-plan chain unchanged (inherits from setup-matrix)

Test plan

  • Non-tf PR, reviewer approves → gate=false, no privileged jobs run
  • tf PR, reviewer comments → gate skipped (state != approved), no privileged jobs run
  • tf PR, reviewer approves → gate=true, terraform-test + 62 plan jobs queue behind single environment approval
  • tf PR, reviewer approves then author pushes new commit → workflow does NOT re-trigger (known tradeoff; author needs a re-approve)

Related: MSRC #127928.

Adds a gate job that only lets terraform-test/plan proceed when the review
is approved AND the PR touches tf files. Cuts approval-email noise.

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

Adjusts the Terraform Integration GitHub Actions workflow to reduce privileged-environment approval spam by triggering the workflow on PR review submission and adding a gating job that only enables the terraform jobs when terraform-related files are touched.

Changes:

  • Switch workflow trigger from pull_request_target (path-filtered) to pull_request_review (types: [submitted]).
  • Add a gate job that uses gh api to detect whether the PR touches terraform-related paths and outputs run=true/false.
  • Make terraform-test and setup-matrix depend on gate and run only when gate outputs run == 'true'.

Comment on lines +18 to +23
gate:
if: github.event.review.state == 'approved'
runs-on: ubuntu-latest
outputs:
run: ${{ steps.check.outputs.run }}
steps:
Comment on lines 6 to +8
on:
pull_request_target:
paths:
- '**.tf'
- '**.json'
- '**.tfvars'
- 'modules/terraform/**'
pull_request_review:
types: [submitted]

jobs:
gate:
if: github.event.review.state == 'approved'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@liyu-ma, here is the dilemma. Suppose I'm a reviewer, I can't approve before I feel confident the change is correct, then I need to check the tf change is working. If I already checked it, the test is not useful anymore.
Alternatively. If this remains a pr check, a reviewer can trigger it before he starts reviewing and check back the result later.

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