-
Notifications
You must be signed in to change notification settings - Fork 8
Update CLA checker workflow #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
d0af05e
fe826dc
e78dd86
fc10ca3
2bd6f12
ff15ce5
1181bdb
e2f37e7
48dd503
4e23060
548cddf
dcf2c97
a455d67
2415cc9
8e563d5
0eeacb5
49f528b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| name: CLA | ||
| name: CLA Checker | ||
| on: | ||
| workflow_call: | ||
| inputs: | ||
|
|
@@ -18,50 +18,53 @@ jobs: | |
| steps: | ||
| # --- Step 1: Check Base Branch --- | ||
| - name: Checkout base branch and check for contributor status | ||
| uses: actions/checkout@v5 | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| ref: ${{ github.event.pull_request.base.sha }} | ||
| ref: ${{ github.ref }} | ||
|
|
||
| - name: Determine if contributor exists in base | ||
| - name: Determine if contributor exists in base (new) | ||
| id: check_contributor_base | ||
| run: | | ||
| AUTHOR="${{ github.event.pull_request.user.login }}" | ||
| if [ -f "CONTRIBUTORS" ]; then | ||
| if grep -q "^$AUTHOR" CONTRIBUTORS; then | ||
| if [ -f "CONTRIBUTORS.md" ]; then | ||
| if grep -q "| $AUTHOR |" CONTRIBUTORS.md || grep -q "| $AUTHOR " CONTRIBUTORS.md; then | ||
| echo "on_base=true" >> $GITHUB_OUTPUT | ||
| echo "🎉 $AUTHOR has already signed the CLA on base branch." | ||
| else | ||
| echo "on_base=false" >> $GITHUB_OUTPUT | ||
| echo "⚠️ $AUTHOR not on base. Proceeding to check PR branch." | ||
| fi | ||
| else | ||
| # If CONTRIBUTORS file doesn't exist, we must check PR branch | ||
| # If CONTRIBUTORS.md file doesn't exist, we must check PR branch | ||
| echo "on_base=undefined" >> $GITHUB_OUTPUT | ||
| echo "🔴 CONTRIBUTORS file does not exist on base. Proceeding to check PR branch." | ||
| echo "🔴 CONTRIBUTORS.md file does not exist on base. Proceeding to check PR branch." | ||
| fi | ||
|
|
||
| # --- Step 2: Check PR Branch --- | ||
| - name: Checkout PR branch and check for contributor status | ||
| # Only run if contributor wasn't found on the base branch | ||
| if: steps.check_contributor_base.outputs.on_base != 'true' | ||
| # Always check PR branch to detect if contributor removed themselves | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| ref: ${{ github.event.pull_request.head.sha }} | ||
|
|
||
| - name: Determine if contributor exists in PR branch | ||
| id: check_contributor_pr | ||
| # Only run if contributor wasn't found on the base branch | ||
| if: steps.check_contributor_base.outputs.on_base != 'true' | ||
| # Always check PR branch to detect if contributor removed themselves | ||
| run: | | ||
| AUTHOR="${{ github.event.pull_request.user.login }}" | ||
| if grep -q "^$AUTHOR" CONTRIBUTORS; then | ||
| echo "signed=true" >> $GITHUB_OUTPUT | ||
| echo "✅ $AUTHOR has updated their CLA signature in CONTRIBUTORS file." | ||
| if [ -f "CONTRIBUTORS.md" ]; then | ||
| if grep -q "| $AUTHOR |" CONTRIBUTORS.md || grep -q "| $AUTHOR " CONTRIBUTORS.md; then | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above |
||
| echo "on_pr=true" >> $GITHUB_OUTPUT | ||
| echo "✅ $AUTHOR is in the CONTRIBUTORS.md file on PR branch." | ||
| else | ||
| echo "on_pr=false" >> $GITHUB_OUTPUT | ||
| echo "⚠️ $AUTHOR is not in the CONTRIBUTORS.md file on PR branch." | ||
| fi | ||
| else | ||
| echo "signed=false" >> $GITHUB_OUTPUT | ||
| echo "❌ $AUTHOR has not signed the CLA." | ||
| echo "on_pr=undefined" >> $GITHUB_OUTPUT | ||
| echo "🔴 CONTRIBUTORS.md file does not exist on PR branch." | ||
| fi | ||
|
|
||
| # --- Step 3: Manage PR Labels, Comments, and Final Status (Consolidated) --- | ||
|
|
@@ -74,44 +77,116 @@ jobs: | |
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| const signedOnBase = '${{ steps.check_contributor_base.outputs.on_base }}' === 'true'; | ||
| // Default signed status is false if the second check wasn't run | ||
| const signedOnPr = '${{ steps.check_contributor_pr.outputs.signed }}' === 'true'; | ||
| const cla_met = signedOnBase || signedOnPr; | ||
| const signedOnPr = '${{ steps.check_contributor_pr.outputs.on_pr }}' === 'true'; | ||
| const issue_number = context.issue.number; | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
| const author = context.payload.pull_request.user.login; | ||
|
|
||
| // Check if contributor was on base but removed themselves from PR | ||
| const removedFromPr = signedOnBase && !signedOnPr; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this complex enough? Once the first PR is on they'll have signed on base. But I would imagine they don't need to add themselves to the CONTRIBUTORS.md in subsequent PRs as it's already on base. This logic will fail in that scenario. I think it should be checking the diff of the CONTRIBUTORS file and looking for their username being removed.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm.. my brain was frozen at 1am. Agree, we should test different scenarios.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point and as discussed I've updated the workflow to handle the scenario where a contributor has already signed the CLA in the base branch. Here's what changed:
|
||
| const cla_met = signedOnBase && signedOnPr; | ||
|
|
||
| // Helper function to create or update a label with a specific color | ||
| async function ensureLabel(name, color, description) { | ||
| try { | ||
| await github.rest.issues.updateLabel({ owner, repo, name, color, description }); | ||
| console.log(`Updated label: ${name} with color ${color}`); | ||
| } catch (error) { | ||
| // If update fails (label doesn't exist), create it | ||
| await github.rest.issues.createLabel({ owner, repo, name, color, description }); | ||
| console.log(`Created new label: ${name} with color ${color}`); | ||
| try { | ||
| await github.rest.issues.createLabel({ owner, repo, name, color, description }); | ||
| console.log(`Created new label: ${name} with color ${color}`); | ||
| } catch (createError) { | ||
| console.log(`Error with label ${name}:`, createError.message); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Define desired colors and descriptions for consistency | ||
| const COLOR_SIGNED = '0052cc'; // Blue | ||
| const COLOR_REQUIRED = 'b60205'; // Red | ||
|
|
||
| console.log(`CLA Met: ${cla_met} (Base: ${signedOnBase}, PR: ${signedOnPr})`); | ||
| // Helper function to delete old CLA-related comments from this bot | ||
| async function deleteOldClaComments() { | ||
| try { | ||
| const comments = await github.rest.issues.listComments({ | ||
| owner, | ||
| repo, | ||
| issue_number | ||
| }); | ||
|
|
||
| // Filter comments from GitHub Actions bot that contain CLA-related content | ||
| // GitHub Actions bot username is 'github-actions[bot]' | ||
| const botComments = comments.data.filter(comment => | ||
| (comment.user.login === 'github-actions[bot]' || comment.user.type === 'Bot') && | ||
| (comment.body.includes('CLA') || | ||
| comment.body.includes('CONTRIBUTORS') || | ||
| comment.body.includes('Contributor Licence Agreement')) | ||
| ); | ||
|
|
||
| console.log(`Found ${botComments.length} old CLA comment(s) to delete`); | ||
|
|
||
| // Delete all old CLA comments | ||
| for (const comment of botComments) { | ||
| await github.rest.issues.deleteComment({ | ||
| owner, | ||
| repo, | ||
| comment_id: comment.id | ||
| }); | ||
| console.log(`Deleted old CLA comment #${comment.id} from ${comment.user.login}`); | ||
| } | ||
| } catch (error) { | ||
| console.log('Error deleting old comments:', error.message); | ||
| } | ||
| } | ||
|
|
||
| console.log(`CLA Met: ${cla_met} (Base: ${signedOnBase}, PR: ${signedOnPr}, Removed: ${removedFromPr})`); | ||
|
|
||
| // Handle case where contributor removed themselves from CONTRIBUTORS file | ||
| if (removedFromPr) { | ||
| await ensureLabel('cla-required', COLOR_REQUIRED, 'CLA signature is required for this PR.'); | ||
| console.log('⚠️ Contributor was in base branch but removed from PR branch.'); | ||
|
|
||
| // Ensure labels are correct | ||
| await Promise.allSettled([ | ||
| github.rest.issues.removeLabel({ owner, repo, issue_number, name: 'cla-signed' }), | ||
| github.rest.issues.addLabels({ owner, repo, issue_number, labels: ['cla-required'] }) | ||
| ]); | ||
|
|
||
| // Delete old CLA comments before posting new one | ||
| await deleteOldClaComments(); | ||
|
|
||
| // Post warning comment | ||
| const commentBody = `⚠️ Hello @${author}!\n\nYour CLA signature was found on the base branch, but you appear to have removed yourself from the _CONTRIBUTORS.md_ file in this PR.\n\nPlease ensure your entry remains in the _CONTRIBUTORS.md_ file. If you have already signed the CLA, you should not remove your details from the file.`; | ||
|
|
||
| await github.rest.issues.createComment({ owner, repo, issue_number, body: commentBody }); | ||
|
|
||
| // Fail the GitHub Action run | ||
| console.error("⚠️ Contributor removed themselves from CONTRIBUTORS file."); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| if (cla_met) { | ||
| await ensureLabel('cla-signed', COLOR_SIGNED, 'This contributor has signed the CLA.'); | ||
| console.log('✅ CLA condition met. Removing required label and adding signed label.'); | ||
| // Use Promise.allSettled for robust label management | ||
| await Promise.allSettled([ | ||
| github.rest.issues.removeLabel({ owner, repo, issue_number, name: 'cla-required' }), | ||
| github.rest.issues.addLabels({ owner, repo, issue_number, labels: ['cla-signed'] }) | ||
| ]); | ||
| if ( signedOnBase === false ) { | ||
| await Promise.allSettled([ | ||
| github.rest.issues.addLabels({ owner, repo, issue_number, labels: ['cla-signed'] }) | ||
| ]); | ||
| } | ||
|
|
||
| } else if (!signedOnBase && signedOnPr) { | ||
| // New contributor signing CLA for the first time | ||
| await ensureLabel('cla-signed', COLOR_SIGNED, 'This contributor has signed the CLA.'); | ||
| console.log('✅ New contributor has signed the CLA in PR branch.'); | ||
| await Promise.allSettled([ | ||
| github.rest.issues.removeLabel({ owner, repo, issue_number, name: 'cla-required' }), | ||
| github.rest.issues.addLabels({ owner, repo, issue_number, labels: ['cla-signed'] }) | ||
| ]); | ||
|
|
||
| // Delete old CLA comments since CLA is now signed | ||
| await deleteOldClaComments(); | ||
|
|
||
| } else { | ||
| await ensureLabel('cla-required', COLOR_REQUIRED, 'CLA signature is required for this PR.'); | ||
|
|
@@ -123,12 +198,15 @@ jobs: | |
| github.rest.issues.addLabels({ owner, repo, issue_number, labels: ['cla-required'] }) | ||
| ]); | ||
|
|
||
| // Delete old CLA comments before posting new one | ||
| await deleteOldClaComments(); | ||
|
|
||
| // Post CLA comment | ||
| const commentBody = `Hello @${author}! 👋\n\nThank you for your contribution. Since this is your first time contributing to this repository, we ask that you sign our Contributor Licence Agreement (CLA).\n\n📄 [You can read the CLA here](https://github.com/MetOffice/simulation-systems/blob/github_wps/Momentum-CLA.md).\n\nTo agree to the CLA, please add your details (**GitHub username**, real name, organisation, email, and date) to the _CONTRIBUTORS_ file (create one, if required) in the development branch for this PR. After signing the CLA, you won't need to do this again for future PRs.`; | ||
| const commentBody = `Hello @${author}! 👋\n\nThank you for your contribution. Since this is your first time contributing to this repository, we ask that you sign our Contributor Licence Agreement (CLA).\n\n📄 [You can read the CLA here](https://github.com/MetOffice/Momentum/blob/main/CLA.md).\n\nTo agree to the CLA, please add your details (**GitHub username**, Real Name, Affiliation, and Date) to the _CONTRIBUTORS.md_ file (create one, if required) in the development branch for this PR. After signing the CLA, you won't need to do this again for future PRs.`; | ||
|
|
||
| await github.rest.issues.createComment({ owner, repo, issue_number, body: commentBody }); | ||
|
|
||
| // Fail the GitHub Action run | ||
| console.error("⚠️ Please add yourself to the CONTRIBUTORS file to sign the CLA."); | ||
| console.error("⚠️ Please add yourself to the CONTRIBUTORS.md file to sign the CLA."); | ||
| process.exit(1); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting this as a markdown table is sensible, but I think this check shouldn't be reliant on correct formatting, particularly with whitespace.
So keep as
grep -q "$AUTHOR"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep the check for the username to be in between
|and|. Perhaps we can relax with the spaces around the username. Imagine a substring collision in another column for a different entry...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah fair enough, something like
grep -qE "|\s*$AUTHOR\s*|" CONTRIBUTORS.md(not tested!)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should work, I think we probably ned to escape the pipes?
grep -qE "\|\s*$AUTHOR\s*\|" CONTRIBUTORS.md