Skip to content
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

Automate CLA checking #11783

Merged
merged 16 commits into from
Feb 23, 2024
Merged

Automate CLA checking #11783

merged 16 commits into from
Feb 23, 2024

Conversation

siddheshranade
Copy link
Contributor

@siddheshranade siddheshranade commented Jan 23, 2024

Description

Whenever a contributor opens a new PR on this repo, we want to check that they've signed an individual/corporate CLA, and automate writing a comment on the PR accordingly.

Cesium concierge performed this automation before, but we stopped using it and hence need to write a new GitHub Action on this repo to continue checking for CLAs.

Issue number and link

Issue in engineering repo.

Testing plan

  1. Open any new pull request on this repo.
  2. This should trigger a new workflow called "CLA Checking", which should run successfully.
  3. The pull request should now have a new comment written by the github-actions bot, either asking the author to sign a CLA, or confirming that they have done so.
  4. If no new comment appears on a new PR the workflow has failed. If a comment appears with an error the workflow has failed.

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have update the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

@siddheshranade
Copy link
Contributor Author

@ggetz

This code is currently stable and configured to write comments on this PR on any git push. (See example comment.)

Need your help with the following to proceed forward:

A. Google Sheets

  1. We need to get Ids for both the Individual and Corporate CLA Google Sheets
  2. Add them to this repo's Secrets
  3. Remove the hardcoded Ids from this line and retrieve from Secrets

B. Google account configuration

  1. Need to get a JSON file for a Google Service account credentials (docs) and store it in Secrets again
  2. Need to retrive/parse JSON from Secrets when authorizing the Google API here
  3. Need to remove TempGoogleJSON of my personal Service Account

C. Contributors

  1. Is this the correct link to the contributors list to add to the comment?

@ggetz
Copy link
Contributor

ggetz commented Jan 24, 2024

Thanks @siddheshranade!

This code is currently stable and configured to write comments on #11781

I'm curious? Why the other PR instead of this one? And I assume we want to change the trigger to an opened PR instead of push, correct?

Need your help with the following to proceed forward

Sure, I'll sync up with you offline to get the proper values and I'll configure them in the repo settings.

@siddheshranade
Copy link
Contributor Author

siddheshranade commented Jan 24, 2024

@ggetz

Why the other PR instead of this one?

Just wanted to keep things on this PR clean, and make it easy to test as many times as we want. Will remove the hardcoded PR Id eventually so the comment appears on whatever PR triggers the action.

I assume we want to change the trigger to an opened PR instead of push, correct?

Yes, correct, trigger on push is only temporary. If you see the code, I've added a bunch of TODOs for all temporary logic that we want to change before testing and merging this.

Also - because I've replaced all API keys with dummy values, for now this workflow will fail every time someone does a push.

@CesiumGS CesiumGS deleted a comment from github-actions bot Feb 5, 2024
@CesiumGS CesiumGS deleted a comment from github-actions bot Feb 5, 2024
@siddheshranade
Copy link
Contributor Author

@ggetz

Ignore my previous (now deleted) comment. We can use the Google Service Account for the API by adding its JSON file as a GitHub secret, and then writing the JSON to a temporary file that's accessed at runtime. (This is how Concierge did it as well.)

  1. PR is ready, please review when you can.

Once we have the Service Account from Chris (from this issue):

  1. Can you please share both Google Sheets to the Service Account email? Otherwise the API wont' be able to read from the sheets.

  2. The workflow also won't currently work - can you please add the following Repository Secrets to this repo:

Secret Name Value
GOOGLE_KEYS The entire JSON from the "API Keys" tab of the Google Service Account, copy-pasted as is into this secret.
INDIVIDUAL_CLA_SHEET_ID The Id of the Google Sheet which lists the individual CLAs.
CORPORATE_CLA_SHEET_ID The Id of the Google Sheet which lists the corporate CLAs.

Thanks!

@siddheshranade siddheshranade marked this pull request as ready for review February 5, 2024 16:21
@siddheshranade siddheshranade requested a review from ggetz February 5, 2024 16:21
Copy link
Contributor

@ggetz ggetz left a comment

Choose a reason for hiding this comment

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

This is looking great @siddheshranade! I'll add the secrets to Github as soon as I get the info.

@@ -0,0 +1,26 @@
name: CLA Checking
on:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

We just want the open action to trigger this, right? By default, pull_request includes other triggers as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, my bad! I think I didn't catch this because it works similar to how we want by default:

For example, if no activity types are specified, the workflow runs when a pull request is opened or reopened or when the head branch of the pull request is updated.

But good to have this only when a new PR created. Fixing in next commit.

@cesiumgs-admin
Copy link

@siddheshranade I was able to create a new service account, share the spreadsheets with that account, and set the repository secrets. Please confirm and let me know if we're good to go.

Copy link

🔴 There was an error checking the CLA! If this is your first contribution, please send in a Contributor License Agreement.

    Error: Google Sheets API has not been used in project 698491282566 before or it is disabled. Enable it by visiting https://console.developers.google.com/apis/api/sheets.googleapis.com/overview?project=698491282566 then retry. If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry.

@siddheshranade
Copy link
Contributor Author

@ggetz

Since you have the Service Account access can you resolve the error github-actions is giving above? It says:

Error: Google Sheets API has not been used in project 698491282566 before or it is disabled. Enable it by visiting https://console.developers.google.com/apis/api/sheets.googleapis.com/overview?project=698491282566 then retry. If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry.

This SO answer should help if required.

@ggetz
Copy link
Contributor

ggetz commented Feb 23, 2024

@siddheshranade This should now be enabled!

@siddheshranade
Copy link
Contributor Author

@ggetz

Thanks for your patience - everything works as expected now. See: latest test comment on a new PR.

Feel free to merge as you wish.

@ggetz
Copy link
Contributor

ggetz commented Feb 23, 2024

Awesome! Thanks for adding this @siddheshranade!

@ggetz ggetz merged commit f3a3dfa into main Feb 23, 2024
9 checks passed
@ggetz ggetz deleted the cla-checking-final branch February 23, 2024 20:44
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