ci: add org member check #14531

scotthart · 2024-07-22T17:25:41Z

This change is

codecov · 2024-07-22T17:53:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.59%. Comparing base (7603af8) to head (b071357).
Report is 20 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14531   +/-   ##
=======================================
  Coverage   93.59%   93.59%           
=======================================
  Files        2316     2316           
  Lines      207132   207132           
=======================================
+ Hits       193864   193867    +3     
+ Misses      13268    13265    -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

coryan · 2024-07-22T17:46:03Z

.github/workflows/test-runner.yml

+        if: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.author_association != 'MEMBER' }}
+        run: |
+          echo "Event not triggered by organization member."
+          exit 1


Doesn't this make the PR fail hard? How would we run the CI builds for any such PR after it failed?

coryan · 2024-07-22T18:02:55Z

.github/workflows/test-runner.yml

@@ -43,6 +55,7 @@ jobs:
          'external'
      }}
    name: Require Approval for External PRs
+    needs: [author-association-member]


I think you also need to remove the code from line 54? Or is that for a future PR?

scotthart

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @coryan)

.github/workflows/test-runner.yml line 44 at r1 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Doesn't this make the PR fail hard? How would we run the CI builds for any such PR after it failed?

Added alternate untrusted workflow.

.github/workflows/test-runner.yml line 58 at r1 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

I think you also need to remove the code from line 54? Or is that for a future PR?

Done.

coryan · 2024-07-22T20:50:56Z

.github/workflows/test-runner-untrusted.yml

+  external-account-integration:
+    name: External Account Integration
+    if: ${{ github.event.pull_request.author_association != 'MEMBER' }}
+    needs: [pre-flight]
+    uses: ./.github/workflows/external-account-integration.yml
+    with:
+      checkout-ref: ${{ needs.pre-flight.outputs.checkout-sha }}


I do not think it makes any sense to run this test on a pull request.

Suggested change

external-account-integration:

name: External Account Integration

if: ${{ github.event.pull_request.author_association != 'MEMBER' }}

needs: [pre-flight]

uses: ./.github/workflows/external-account-integration.yml

with:

checkout-ref: ${{ needs.pre-flight.outputs.checkout-sha }}

coryan · 2024-07-22T20:52:56Z

.github/workflows/test-runner-untrusted.yml

+            *)
+              exit 0
+              ;;


This will do nothing on a pull request, and will only run on pull requests.

coryan · 2024-07-22T20:54:54Z

.github/workflows/test-runner-untrusted.yml

+  notify:
+    name: Notify-Google-Chat
+    # Wait until all the other jobs have completed.
+    needs:
+      - external-account-integration
+      - macos-bazel
+      - macos-cmake
+      - windows-bazel
+      - windows-cmake
+    # Run even if the other jobs failed or were skipped.
+    if: always()
+    runs-on: ubuntu-latest
+    steps:
+      - name: Notify Google Chat
+        shell: bash
+        run: |
+          event_name="${{ github.event_name }}"
+          case "${event_name}" in
+            schedule)
+              ;;
+            push)
+              ;;
+            workflow_dispatch)
+              ;;
+            *)
+              exit 0
+              ;;
+          esac
+          failure="${{ contains(needs.*.result, 'failure') }}"
+          cancelled="${{ contains(needs.*.result, 'cancelled') }}"
+          status=""
+          # Report whether any of the jobs failed or were cancelled.
+          if [[ "${cancelled}" == "true" ]]; then status="cancelled"; fi
+          if [[ "${failure}" == "true" ]]; then status="failure"; fi
+          # Exit early if there is nothing interesting to report.
+          if [[ -z "${status}" ]]; then exit 0; fi
+          printf '{"text": "GHA Build %s %s/%s/actions/runs/%s"}' \
+            "${status}" "${{ github.server_url }}" "${{ github.repository }}" "${{ github.run_id }}" |
+          curl -fsX POST -o /dev/null -d@- -H "Content-Type: application/json; charset=UTF-8" '${{ secrets.CLOUD_CPP_BUILD_ALERTS_WEBHOOK }}'


Suggested change

notify:

name: Notify-Google-Chat

# Wait until all the other jobs have completed.

needs:

- external-account-integration

- macos-bazel

- macos-cmake

- windows-bazel

- windows-cmake

# Run even if the other jobs failed or were skipped.

if: always()

runs-on: ubuntu-latest

steps:

- name: Notify Google Chat

shell: bash

run: |

event_name="${{ github.event_name }}"

case "${event_name}" in

schedule)

;;

push)

;;

workflow_dispatch)

;;

*)

exit 0

;;

esac

failure="${{ contains(needs.*.result, 'failure') }}"

cancelled="${{ contains(needs.*.result, 'cancelled') }}"

status=""

# Report whether any of the jobs failed or were cancelled.

if [[ "${cancelled}" == "true" ]]; then status="cancelled"; fi

if [[ "${failure}" == "true" ]]; then status="failure"; fi

# Exit early if there is nothing interesting to report.

if [[ -z "${status}" ]]; then exit 0; fi

printf '{"text": "GHA Build %s %s/%s/actions/runs/%s"}' \

"${status}" "${{ github.server_url }}" "${{ github.repository }}" "${{ github.run_id }}" |

curl -fsX POST -o /dev/null -d@- -H "Content-Type: application/json; charset=UTF-8" '${{ secrets.CLOUD_CPP_BUILD_ALERTS_WEBHOOK }}'

coryan · 2024-07-22T20:55:28Z

.github/workflows/test-runner-untrusted.yml

+    uses: ./.github/workflows/macos-bazel.yml
+    with:
+      checkout-ref: ${{ needs.pre-flight.outputs.checkout-sha }}
+  windows-bazel:


Consider adding just one of these until you have finished testing, and then adding them back.

coryan · 2024-07-22T20:58:26Z

.github/workflows/test-runner.yml

+        if: >-
+          ${{
+              github.event_name == 'pull_request_target' &&
+              github.event.pull_request.author_association != 'MEMBER'
+          }}


Why not move this if: section to the pre-flight job?

scotthart

Reviewable status: 0 of 2 files reviewed, 7 unresolved discussions (waiting on @coryan)

.github/workflows/test-runner.yml line 44 at r3 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Why not move this if: section to the pre-flight job?

Done.

.github/workflows/test-runner-untrusted.yml line 52 at r3 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

I do not think it makes any sense to run this test on a pull request.

Removed

.github/workflows/test-runner-untrusted.yml line 69 at r3 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Consider adding just one of these until you have finished testing, and then adding them back.

Done.

.github/workflows/test-runner-untrusted.yml line 143 at r3 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

This will do nothing on a pull request, and will only run on pull requests.

Removed notify job

.github/workflows/test-runner-untrusted.yml line 155 at r3 (raw file):

Previously, coryan (Carlos O'Ryan) wrote…

Done.

coryan

I do not want to block you here. I need to get ready for some stuff on Wednesday.

ci: add org member check

14f57e3

scotthart requested a review from a team as a code owner July 22, 2024 17:25

scotthart temporarily deployed to internal July 22, 2024 17:25 — with GitHub Actions Inactive

coryan reviewed Jul 22, 2024

View reviewed changes

add unstrusted pull_target alternative

a55c05a

scotthart temporarily deployed to internal July 22, 2024 18:58 — with GitHub Actions Inactive

scotthart had a problem deploying to external July 22, 2024 18:58 — with GitHub Actions Error

scotthart commented Jul 22, 2024

View reviewed changes

tweak trusted environment

d5e193a

scotthart temporarily deployed to external July 22, 2024 19:07 — with GitHub Actions Inactive

scotthart temporarily deployed to internal July 22, 2024 19:07 — with GitHub Actions Inactive

add untrusted job condition

07dfab0

scotthart temporarily deployed to internal July 22, 2024 19:22 — with GitHub Actions Inactive

scotthart temporarily deployed to internal July 22, 2024 19:23 — with GitHub Actions Inactive

fix deps

a9d60e3

scotthart temporarily deployed to internal July 22, 2024 19:32 — with GitHub Actions Inactive

remove dispatch from trusted

5d5593d

scotthart temporarily deployed to internal July 22, 2024 19:34 — with GitHub Actions Inactive

coryan reviewed Jul 22, 2024

View reviewed changes

address review comments

4f63c43

scotthart commented Jul 22, 2024

View reviewed changes

scotthart temporarily deployed to internal July 22, 2024 21:11 — with GitHub Actions Inactive

fix preflight conditional

7957c20

scotthart temporarily deployed to internal July 22, 2024 21:16 — with GitHub Actions Inactive

debug

6df457a

scotthart temporarily deployed to internal July 22, 2024 21:22 — with GitHub Actions Inactive

debug 2

9fb1b17

scotthart temporarily deployed to internal July 22, 2024 21:22 — with GitHub Actions Inactive

debug 3

31dc41c

scotthart temporarily deployed to internal July 22, 2024 21:24 — with GitHub Actions Inactive

fix preflight condition

acd3851

scotthart temporarily deployed to internal July 22, 2024 21:25 — with GitHub Actions Inactive

coryan approved these changes Jul 22, 2024

View reviewed changes

remove debug

b071357

scotthart temporarily deployed to internal July 22, 2024 21:27 — with GitHub Actions Inactive

scotthart merged commit ddfe4dd into main Jul 22, 2024
104 of 105 checks passed

scotthart deleted the ci-gha-author-association branch July 22, 2024 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: add org member check #14531

ci: add org member check #14531

scotthart commented Jul 22, 2024 •

edited by coryan

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading

coryan Jul 22, 2024

coryan Jul 22, 2024

scotthart left a comment

coryan Jul 22, 2024

coryan Jul 22, 2024

coryan Jul 22, 2024

coryan Jul 22, 2024

coryan Jul 22, 2024

scotthart left a comment

coryan left a comment

ci: add org member check #14531

ci: add org member check #14531

Conversation

scotthart commented Jul 22, 2024 • edited by coryan Loading

codecov bot commented Jul 22, 2024 • edited Loading

Codecov Report

coryan Jul 22, 2024

Choose a reason for hiding this comment

coryan Jul 22, 2024

Choose a reason for hiding this comment

scotthart left a comment

Choose a reason for hiding this comment

coryan Jul 22, 2024

Choose a reason for hiding this comment

coryan Jul 22, 2024

Choose a reason for hiding this comment

coryan Jul 22, 2024

Choose a reason for hiding this comment

coryan Jul 22, 2024

Choose a reason for hiding this comment

coryan Jul 22, 2024

Choose a reason for hiding this comment

scotthart left a comment

Choose a reason for hiding this comment

coryan left a comment

Choose a reason for hiding this comment

scotthart commented Jul 22, 2024 •

edited by coryan

Loading

codecov bot commented Jul 22, 2024 •

edited

Loading