From 415b4e8e7b36450611f25289e6868f934a87fcee Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 Jan 2026 19:37:39 +0000 Subject: [PATCH 1/3] Improve fork safety: consolidate approval and add pull_request trigger - Add pull_request trigger for internal PRs (non-forks) to test workflow changes immediately - Keep pull_request_target for fork PRs that need access to secrets - Move approval step to test-all-warehouses.yml (runs once instead of per-platform) - Remove per-platform approval from test-warehouse.yml to reduce spam Fixes ELE-5221 Co-Authored-By: Itamar Hartstein --- .github/workflows/test-all-warehouses.yml | 55 ++++++++++++++++++++++- .github/workflows/test-warehouse.yml | 17 ------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/.github/workflows/test-all-warehouses.yml b/.github/workflows/test-all-warehouses.yml index 6a969c825..7cee463b1 100644 --- a/.github/workflows/test-all-warehouses.yml +++ b/.github/workflows/test-all-warehouses.yml @@ -1,5 +1,13 @@ name: Test all warehouse platforms on: + # For internal PRs (non-forks) - no approval needed, can test workflow changes immediately + pull_request: + branches: ["master"] + paths: + - elementary/** + - .github/** + - pyproject.toml + # For fork PRs - requires approval before running (has access to secrets) pull_request_target: branches: ["master"] paths: @@ -27,7 +35,52 @@ on: description: Whether to generate new data jobs: + # Determine if this is a fork PR and skip if wrong trigger is used + check-fork-status: + runs-on: ubuntu-latest + outputs: + is_fork: ${{ steps.check.outputs.is_fork }} + should_skip: ${{ steps.check.outputs.should_skip }} + steps: + - name: Check if PR is from fork + id: check + run: | + IS_FORK="false" + SHOULD_SKIP="false" + + if [[ "${{ github.event_name }}" == "pull_request" || "${{ github.event_name }}" == "pull_request_target" ]]; then + if [[ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]]; then + IS_FORK="true" + fi + fi + + # Skip if: pull_request from fork (should use pull_request_target) OR pull_request_target from non-fork (should use pull_request) + if [[ "${{ github.event_name }}" == "pull_request" && "$IS_FORK" == "true" ]]; then + SHOULD_SKIP="true" + elif [[ "${{ github.event_name }}" == "pull_request_target" && "$IS_FORK" == "false" ]]; then + SHOULD_SKIP="true" + fi + + echo "is_fork=$IS_FORK" >> $GITHUB_OUTPUT + echo "should_skip=$SHOULD_SKIP" >> $GITHUB_OUTPUT + + # Approval gate for fork PRs (only runs once for all platforms) + approve-fork: + runs-on: ubuntu-latest + needs: [check-fork-status] + if: needs.check-fork-status.outputs.should_skip != 'true' && needs.check-fork-status.outputs.is_fork == 'true' + environment: elementary_test_env + steps: + - name: Approved + run: echo "Fork PR approved for testing" + test: + needs: [check-fork-status, approve-fork] + # Run if: not skipped AND (not a fork OR fork approval succeeded) + if: | + always() && + needs.check-fork-status.outputs.should_skip != 'true' && + (needs.check-fork-status.outputs.is_fork != 'true' || needs.approve-fork.result == 'success') strategy: fail-fast: false matrix: @@ -37,7 +90,7 @@ jobs: uses: ./.github/workflows/test-warehouse.yml with: warehouse-type: ${{ matrix.warehouse-type }} - elementary-ref: ${{ inputs.elementary-ref || (github.event_name == 'pull_request_target' && github.event.pull_request.head.sha) || '' }} + elementary-ref: ${{ inputs.elementary-ref || ((github.event_name == 'pull_request_target' || github.event_name == 'pull_request') && github.event.pull_request.head.sha) || '' }} dbt-data-reliability-ref: ${{ inputs.dbt-data-reliability-ref }} dbt-version: ${{ matrix.dbt-version }} generate-data: ${{ inputs.generate-data || false }} diff --git a/.github/workflows/test-warehouse.yml b/.github/workflows/test-warehouse.yml index a2e26fc53..66b938877 100644 --- a/.github/workflows/test-warehouse.yml +++ b/.github/workflows/test-warehouse.yml @@ -58,25 +58,8 @@ env: E2E_DBT_PROJECT_DIR: ${{ github.workspace }}/elementary/tests/e2e_dbt_project jobs: - # PRs from forks require approval, specifically with the "pull_request_target" event as it contains repo secrets. - check-if-requires-approval: - runs-on: ubuntu-latest - outputs: - requires_approval: ${{ steps.set-output.outputs.requires_approval }} - steps: - - name: Set requires approval output - id: set-output - run: | - if [[ "${{ github.event_name }}" =~ ^pull_request && "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]]; then - echo "requires_approval=true" >> $GITHUB_OUTPUT - else - echo "requires_approval=false" >> $GITHUB_OUTPUT - fi - test: runs-on: ubuntu-latest - needs: [check-if-requires-approval] - environment: ${{ (needs.check-if-requires-approval.outputs.requires_approval == 'true' && 'elementary_test_env') || '' }} defaults: run: working-directory: elementary From 842db9fcb0444a3a55453fafd8439f2ff7a9d573 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 Jan 2026 20:28:02 +0000 Subject: [PATCH 2/3] Fix: use !cancelled() and check fork-status result before running tests Co-Authored-By: Itamar Hartstein --- .github/workflows/test-all-warehouses.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-all-warehouses.yml b/.github/workflows/test-all-warehouses.yml index 7cee463b1..2a22e0a16 100644 --- a/.github/workflows/test-all-warehouses.yml +++ b/.github/workflows/test-all-warehouses.yml @@ -76,9 +76,10 @@ jobs: test: needs: [check-fork-status, approve-fork] - # Run if: not skipped AND (not a fork OR fork approval succeeded) + # Run if: not cancelled, fork check succeeded, not skipped, AND (not a fork OR fork approval succeeded) if: | - always() && + ! cancelled() && + needs.check-fork-status.result == 'success' && needs.check-fork-status.outputs.should_skip != 'true' && (needs.check-fork-status.outputs.is_fork != 'true' || needs.approve-fork.result == 'success') strategy: From a5234f393dc324fe5b115e2dc14ef9381d7789ca Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 27 Jan 2026 20:53:32 +0000 Subject: [PATCH 3/3] Remove redundant comment on test job condition Co-Authored-By: Itamar Hartstein --- .github/workflows/test-all-warehouses.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test-all-warehouses.yml b/.github/workflows/test-all-warehouses.yml index 2a22e0a16..a745ee1cd 100644 --- a/.github/workflows/test-all-warehouses.yml +++ b/.github/workflows/test-all-warehouses.yml @@ -76,7 +76,6 @@ jobs: test: needs: [check-fork-status, approve-fork] - # Run if: not cancelled, fork check succeeded, not skipped, AND (not a fork OR fork approval succeeded) if: | ! cancelled() && needs.check-fork-status.result == 'success' &&