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

Verify that PR creators are members of the website-write team #6959

Closed
wants to merge 8 commits into from

Conversation

ajb176
Copy link
Member

@ajb176 ajb176 commented Jun 5, 2024

Fixes #3906

What changes did you make?

  • Added an extra job to the pr-instructions.yml workflow which makes up to three API calls to verify team membership, close the pull request, and add the requested comment.
  • The octokit membership API call returns a 404 error if the member is not found on the website-write team, in which case the catch block makes the API calls to close the PR and add the comment.
  • If the PR author is found on the website-write team (the expected case), nothing will happen except a comment logged that the author was successfully verified.

Why did you make the changes (we will use this info to test)?

  • To ensure only members of the website-write team can create pull requests.

Instructions for Reviewers:

  • 1. Add - 'verify-pr-author-3906' on line 8 aligned with the other two branches
  • 2. Replace the repo-token with github-token on line 43 and replace the current token with a user-generated token (Instructions here)
  • 3. Change the owner assignment on lines 57 and 63 from 'hackforla' to your own github handle
  • 4. Save, commit and push the changes to your local verify-pr-author-3906 branch
  • 5. Checkout a new branch from this one (testcase1, for example), make any minor change (add a letter a comment for example), then commit and push it
  • 6. Go to your github website fork and try to merge testcase1 into the verify-pr-author-3906 branch
  • 7. The github actions should start running (assuming the base is the verify-pr-author-3906 branch edited from steps 1-4)
  • 8. There should be a checkmark if the action was successful, click details, dropdown the RunActions/GithubScript and it should say 'Successfully verified!'
  • 9. For testing the fail condition, navigate to the verify-pr-author branch and change the username assignment from prAuthor in line 51 to a random string or github handle of someone not on the website-write team.
  • 10. Repeat steps 4-8 (for step 5 you can call the new branch testcase2), this time the pull request should automatically close and post a comment from your handle

Make sure to push the necessary changes to the verify-pr-author branch before using checkout to create the testcase branches. The testcase branches should be one commit past the verify-pr branch to avoid headaches when trying to merge the testcase branch into the base branch. If you'd rather not mess with the original branch, you can also checkout a new branch immediately after pulling the verify-pr-author-3906 branch, and follow the instructions while using the new branch name in steps 1, 4, 6, 7 and 9.

@github-actions github-actions bot added role: back end/devOps Tasks for back-end developers Complexity: Large Status: Updated No blockers and update is ready for review Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly automation for manulal github board maintenance actions that are going to be automated size: 3pt Can be done in 13-18 hours labels Jun 5, 2024
@ajb176
Copy link
Member Author

ajb176 commented Jun 5, 2024

I couldn't find information on repo tokens so I just used the HACKFORLA_BOT_PA_TOKEN I saw in other yml files, the code should be testable but I'll need to add a token with the right privileges (admin:org specifically) before it can be merged. Or maybe the privileges are fine but I need to call it a 'repo-token' instead of a 'github-token' like in the other files? I had to switch it to github-token to test it, I'll try switching it back.

I'm now seeing a 403 error in the workflow logs, so at least the API call is being made but it seems like the token lacks privileges. @t-will-gillis any idea on which token may work here?

@ajb176 ajb176 closed this Jun 5, 2024
@ajb176 ajb176 reopened this Jun 5, 2024
@t-will-gillis
Copy link
Member

t-will-gillis commented Jun 5, 2024

Hi @ajb176

I couldn't find information on repo tokens so I just used the HACKFORLA_BOT_PA_TOKEN I saw in other yml files, the code should be testable but I'll need to add a token with the right privileges (admin:org specifically) before it can be merged. Or maybe the privileges are fine but I need to call it a 'repo-token' instead of a 'github-token' like in the other files? I had to switch it to github-token to test it, I'll try switching it back.

The info about tokens is in a difficult to find place due to the Wiki still not being updated. The current version of the Hack for LA's GitHub Actions is in this link, and if you scroll down to Tip 7 the token scopes are listed (see Details):

Screenshot 2024-06-05 130313

I'm now seeing a 403 error in the workflow logs, so at least the API call is being made but it seems like the token lacks privileges. @t-will-gillis any idea on which token may work here?

If it is an issue with a lack of scopes, the TEAMS token might be enough. Regarding "Check Team Membership", this may or may not be what you need, however did you check out issue-trigger.yml and the tpascoal/get-user-teams-membership automation?

Additionally: you could try adding in:

permissions: 
  issues: write
  pull-requests: write

to your yml

@ajb176
Copy link
Member Author

ajb176 commented Jun 6, 2024

Thanks @t-will-gillis

I think the admin token might work, I'll do some trial and error and see if I can get it working.

It seems like the issue might have been the fact that the trigger is pull_request instead of pull_request_target, in which case the workflow wouldn't be granted access to tokens in the base repository. I've tested a personal access token with the same scope as the admin token locally, and I added a new yml file with the pull_request_target trigger and it seems like it's working. I'm pretty confident this was the problem but I'll make a new PR tomorrow and check.

@ajb176 ajb176 closed this Jun 6, 2024
@ajb176 ajb176 reopened this Jun 6, 2024
@ajb176 ajb176 closed this Jun 7, 2024
@ajb176 ajb176 deleted the verify-pr-author-3906 branch June 21, 2024 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation for manulal github board maintenance actions that are going to be automated Complexity: Large Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly role: back end/devOps Tasks for back-end developers size: 3pt Can be done in 13-18 hours Status: Updated No blockers and update is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a Github Action to verify Pull Requests are created by website team members
2 participants