From 4313c326d266c383bee5a015a5fb2fd025013318 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Tue, 28 Jan 2025 19:05:00 -0600 Subject: [PATCH 1/3] first pass --- .github/workflows/artifact-reviews.yml | 81 ++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 .github/workflows/artifact-reviews.yml diff --git a/.github/workflows/artifact-reviews.yml b/.github/workflows/artifact-reviews.yml new file mode 100644 index 00000000000..76fac7d48a3 --- /dev/null +++ b/.github/workflows/artifact-reviews.yml @@ -0,0 +1,81 @@ +# **what?** +# Enforces additional reviews when artifact or validation files are modified. + +# **why?** +# Ensure sensitive files receive proper review from designated team members. + +# **when?** +# This will run when PRs are opened, synchronized, reopened, or edited. + +name: "Enforce Additional Reviews on Artifact and Validations Changes" + +on: + pull_request: + types: [opened, synchronize, reopened, edited] + +jobs: + enforce-reviews: + name: "Enforce Additional Reviews" + runs-on: ubuntu-latest + steps: + - name: "Checkout code" + uses: actions/checkout@v4 + + - name: "Get list of changed files" + id: changed_files + run: | + CHANGED_FILES=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files | jq -r '.[].filename') + echo "Changed files:" + echo "$CHANGED_FILES" + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: "Check if any sensitive files were changed" + id: sensitive_files_changed + run: | + SENSITIVE_CHANGES=false + while IFS= read -r file; do + # TODO: validate what files we care about + if [[ "$file" == artifacts/* ]] || [[ "$file" == validation/* ]]; then + SENSITIVE_CHANGES=true + break + fi + done <<< "$CHANGED_FILES" + echo "SENSITIVE_CHANGES=$SENSITIVE_CHANGES" >> $GITHUB_OUTPUT + + - name: "Get Core Team Members" + if: ${{ steps.sensitive_files_changed.outputs.SENSITIVE_CHANGES == 'true' }} + id: core_members + run: | + gh api -H "Accept: application/vnd.github+json" \ + /orgs/dbt-labs/teams/core-group/members > core_members.json + env: + GH_TOKEN: ${{ secrets.IT_TEAM_MEMBERSHIP }} + + - name: "Verify 2 core team approvals" + id: check_approvals + if: ${{ steps.sensitive_files_changed.outputs.SENSITIVE_CHANGES == 'true' }} + run: | + + # Get all reviews + REVIEWS=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/reviews) + + # Count approved reviews from core team members + CORE_APPROVALS=0 + while IFS= read -r member; do + APPROVED=$(echo "$REVIEWS" | jq --arg user "$member" \ + '.[] | select(.user.login == $user and .state == "APPROVED") | .user.login' | wc -l) + CORE_APPROVALS=$((CORE_APPROVALS + APPROVED)) + done <<< "$CORE_MEMBERS" + + echo "CORE_APPROVALS=$CORE_APPROVALS" >> $GITHUB_OUTPUT + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # TODO: should this post a PR comment? probably... same as changelog. + - name: "Fail if not enough approvals" + if: ${{ steps.sensitive_files_changed.outputs.SENSITIVE_CHANGES == 'true' && steps.check_approvals.outputs.CORE_APPROVALS != '2' }} + run: | + echo "Error: Changes to sensitive files require at least 2 approvals from core team members" + echo "Current number of core team approvals: $CORE_APPROVALS" + exit 1 From f38f8eaaf501c6141c42e095397011cc6d2fc959 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Wed, 29 Jan 2025 15:36:57 -0600 Subject: [PATCH 2/3] resolve TODOs --- .github/workflows/artifact-reviews.yml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/artifact-reviews.yml b/.github/workflows/artifact-reviews.yml index 76fac7d48a3..813bf4fb24a 100644 --- a/.github/workflows/artifact-reviews.yml +++ b/.github/workflows/artifact-reviews.yml @@ -14,8 +14,8 @@ on: types: [opened, synchronize, reopened, edited] jobs: - enforce-reviews: - name: "Enforce Additional Reviews" + check-reviews: + name: "Validate Additional Reviews" runs-on: ubuntu-latest steps: - name: "Checkout code" @@ -35,8 +35,7 @@ jobs: run: | SENSITIVE_CHANGES=false while IFS= read -r file; do - # TODO: validate what files we care about - if [[ "$file" == artifacts/* ]] || [[ "$file" == validation/* ]]; then + if [[ "$file" == core/dbt/artifacts/* ]] ; then SENSITIVE_CHANGES=true break fi @@ -72,10 +71,10 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # TODO: should this post a PR comment? probably... same as changelog. - - name: "Fail if not enough approvals" + - name: "Notify and failif not enough approvals" if: ${{ steps.sensitive_files_changed.outputs.SENSITIVE_CHANGES == 'true' && steps.check_approvals.outputs.CORE_APPROVALS != '2' }} run: | - echo "Error: Changes to sensitive files require at least 2 approvals from core team members" - echo "Current number of core team approvals: $CORE_APPROVALS" + title="PR Approval Requirements Not Met" + message="Changes to artifact directory files requires at least 2 approvals from core team members. Current number of core team approvals: $CORE_APPROVALS " + echo "::error title=$title::$message" exit 1 From 43b2ebbc24eb2beba3bd380d6aba37e7fe8c3143 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 31 Jan 2025 12:20:50 -0600 Subject: [PATCH 3/3] updates after testing --- .github/workflows/artifact-reviews.yml | 106 ++++++++++++++++++++----- 1 file changed, 87 insertions(+), 19 deletions(-) diff --git a/.github/workflows/artifact-reviews.yml b/.github/workflows/artifact-reviews.yml index 813bf4fb24a..dba991eafad 100644 --- a/.github/workflows/artifact-reviews.yml +++ b/.github/workflows/artifact-reviews.yml @@ -1,21 +1,63 @@ # **what?** -# Enforces additional reviews when artifact or validation files are modified. +# Enforces 2 reviews when artifact or validation files are modified. # **why?** -# Ensure sensitive files receive proper review from designated team members. +# Ensure artifact changes receive proper review from designated team members. GitHub doesn't support +# multiple reviews on a single PR based on files changed, so we need to enforce this manually. # **when?** -# This will run when PRs are opened, synchronized, reopened, or edited. +# This will run when PRs are opened, synchronized, reopened, edited, or when reviews +# are submitted and dismissed. name: "Enforce Additional Reviews on Artifact and Validations Changes" on: - pull_request: + pull_request_target: types: [opened, synchronize, reopened, edited] + # retrigger check on review events + pull_request_review: + types: [submitted, dismissed] + +# only run this once per PR at a time +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +env: + required_approvals: 2 + team: "core-group" jobs: + cleanup-old-runs: + # this job is only run once per PR at a time. Since it uses two types of triggers, + # when the pull_request trigger fails, that run stays around when the pull_request_review + # triggers a new run. This job will clean up those old runs so we only end up with a single run. + name: "Cleanup Previous Runs" + runs-on: ubuntu-latest + steps: + - name: "Dismiss previous workflow runs" + run: | + # Get all check runs for this PR's SHA + checks=$(gh api repos/${{ github.repository }}/commits/${{ github.event.pull_request.head.sha }}/check-runs \ + --jq '.check_runs[] | select(.name == "Validate Additional Reviews")') + + # For each check run from this workflow (except current), dismiss it + echo "$checks" | jq -r '. | select(.id != ${{ github.run_id }}) | .id' | \ + while read -r check_id; do + echo "Dismissing check $check_id" + gh api repos/${{ github.repository }}/check-runs/$check_id \ + -X PATCH \ + -F status="completed" \ + -F conclusion="neutral" \ + -F "output[title]=Superseded" \ + -F "output[summary]=This check was superseded by a newer run" + done + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + check-reviews: name: "Validate Additional Reviews" + needs: [cleanup-old-runs] runs-on: ubuntu-latest steps: - name: "Checkout code" @@ -27,33 +69,42 @@ jobs: CHANGED_FILES=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files | jq -r '.[].filename') echo "Changed files:" echo "$CHANGED_FILES" + echo "CHANGED_FILES<> $GITHUB_OUTPUT + echo "$CHANGED_FILES" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: "Check if any sensitive files were changed" - id: sensitive_files_changed + - name: "Check if any artifact files were changed" + id: artifact_files_changed run: | - SENSITIVE_CHANGES=false + artifact_changes=false while IFS= read -r file; do - if [[ "$file" == core/dbt/artifacts/* ]] ; then - SENSITIVE_CHANGES=true + echo "Debug: Checking file: '$file'" + if [[ "$file" == "core/dbt/artifacts/"* ]] ; then + artifact_changes=true break fi - done <<< "$CHANGED_FILES" - echo "SENSITIVE_CHANGES=$SENSITIVE_CHANGES" >> $GITHUB_OUTPUT + done <<< "${{ steps.changed_files.outputs.CHANGED_FILES }}" + echo "artifact_changes=$artifact_changes" >> $GITHUB_OUTPUT - name: "Get Core Team Members" - if: ${{ steps.sensitive_files_changed.outputs.SENSITIVE_CHANGES == 'true' }} + if: ${{ steps.artifact_files_changed.outputs.artifact_changes == 'true' }} id: core_members run: | gh api -H "Accept: application/vnd.github+json" \ - /orgs/dbt-labs/teams/core-group/members > core_members.json + /orgs/dbt-labs/teams/${{ env.team }}/members > core_members.json + + # Extract usernames and set as multiline output + echo "membership<> $GITHUB_OUTPUT + jq -r '.[].login' core_members.json >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT env: GH_TOKEN: ${{ secrets.IT_TEAM_MEMBERSHIP }} - - name: "Verify 2 core team approvals" + - name: "Verify ${{ env.required_approvals }} core team approvals" id: check_approvals - if: ${{ steps.sensitive_files_changed.outputs.SENSITIVE_CHANGES == 'true' }} + if: ${{ steps.artifact_files_changed.outputs.artifact_changes == 'true' }} run: | # Get all reviews @@ -62,19 +113,36 @@ jobs: # Count approved reviews from core team members CORE_APPROVALS=0 while IFS= read -r member; do + echo "$member" + echo "$user" APPROVED=$(echo "$REVIEWS" | jq --arg user "$member" \ '.[] | select(.user.login == $user and .state == "APPROVED") | .user.login' | wc -l) CORE_APPROVALS=$((CORE_APPROVALS + APPROVED)) - done <<< "$CORE_MEMBERS" + done <<< "${{ steps.core_members.outputs.membership }}" echo "CORE_APPROVALS=$CORE_APPROVALS" >> $GITHUB_OUTPUT + echo $CORE_APPROVALS env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: "Notify and failif not enough approvals" - if: ${{ steps.sensitive_files_changed.outputs.SENSITIVE_CHANGES == 'true' && steps.check_approvals.outputs.CORE_APPROVALS != '2' }} + - name: "Notify and fail if not enough approvals" + if: ${{ steps.artifact_files_changed.outputs.artifact_changes == 'true' && steps.check_approvals.outputs.CORE_APPROVALS != env.required_approvals }} run: | title="PR Approval Requirements Not Met" - message="Changes to artifact directory files requires at least 2 approvals from core team members. Current number of core team approvals: $CORE_APPROVALS " + message="Changes to artifact directory files requires at least ${{ env.required_approvals }} approvals from core team members. Current number of core team approvals: ${{ steps.check_approvals.outputs.CORE_APPROVALS }} " echo "::error title=$title::$message" exit 1 + + - name: "Notify of sufficient approvals" + if: ${{ steps.artifact_files_changed.outputs.artifact_changes == 'true' && steps.check_approvals.outputs.CORE_APPROVALS >= env.required_approvals }} + run: | + title="Extra requirements met" + message="Changes to artifact directory files requires at least ${{ env.required_approvals }} approvals from core team members. Current number of core team approvals: ${{ steps.check_approvals.outputs.CORE_APPROVALS }} " + echo "::notice title=$title::$message" + + - name: "Notify of no extra requirements" + if: ${{ steps.artifact_files_changed.outputs.artifact_changes != 'true' }} + run: | + title="No extra requirements" + message="No additional reviews required" + echo "::notice title=$title::$message"