-
Notifications
You must be signed in to change notification settings - Fork 9
Add link to new member playbook to auto-generated PRs #319
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
Open
tim-schilling
wants to merge
2
commits into
main
Choose a base branch
from
tim-schilling-patch-2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,35 +28,41 @@ jobs: | |
|
|
||
| - name: Checkout code | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Get username to add | ||
| id: get_username | ||
| run: | ||
| python -c "print('USERNAME='+'${{ github.event.issue.title }}'.split(' - ')[1].strip().lstrip('@'))" >> $GITHUB_ENV | ||
| python -c "print('USERNAME='+'${GITHUB_EVENT_ISSUE_TITLE}'.split(' - ')[1].strip().lstrip('@'))" >> $GITHUB_ENV | ||
| env: | ||
| GITHUB_EVENT_ISSUE_TITLE: ${{ github.event.issue.title }} | ||
|
|
||
| - name: Validate add user request | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| ISSUE_USER: ${{ github.event.issue.user.login }} | ||
| ISSUE_USER: ${{ env.ISSUE_USER }} | ||
| USERNAME: ${{ env.USERNAME }} | ||
| ISSUE_NUMBER: ${{ env.ISSUE_NUMBER }} | ||
| run: | | ||
| # Check whether the user exists | ||
| set +e | ||
| gh api /users/${{ env.USERNAME }} > /dev/null | ||
| gh api /users/${USERNAME} > /dev/null | ||
| if [ $? -ne 0 ]; then | ||
| gh issue comment ${{ env.ISSUE_NUMBER }} --body "User ${{ env.USERNAME }} does not exist." | ||
| gh issue comment ${ISSUE_NUMBER} --body "User ${USERNAME} does not exist." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Check if the username is in the title is the same as the user who opened the issue | ||
| if [ "${{ env.USERNAME }}" != "${{ env.ISSUE_USER }}" ]; then | ||
| gh issue comment ${{ env.ISSUE_NUMBER }} --body "If you want to add a different user, please create a PR for it" | ||
| if [ "${USERNAME}" != "${ISSUE_USER}" ]; then | ||
| gh issue comment ${ISSUE_NUMBER} --body "If you want to add a different user, please create a PR for it" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Check if the user is already a member of the org | ||
| gh api /orgs/django-commons/members/${{ env.USERNAME }} > /dev/null | ||
| gh api /orgs/django-commons/members/${USERNAME} > /dev/null | ||
| if [ $? -eq 0 ]; then | ||
| gh issue comment ${{ env.ISSUE_NUMBER }} --body "User ${{ env.USERNAME }} is already a member of django-commons." | ||
| gh issue comment ${ISSUE_NUMBER} --body "User ${USERNAME} is already a member of django-commons." | ||
| exit 1 | ||
| fi | ||
|
|
||
|
|
@@ -66,38 +72,50 @@ jobs: | |
| git config user.email [email protected] | ||
|
|
||
| - name: Create branch | ||
| run: git checkout -b ${{ env.BRANCH_NAME }} | ||
| run: git checkout -b ${BRANCH_NAME} | ||
| env: | ||
| BRANCH_NAME: ${{ env.BRANCH_NAME }} | ||
|
|
||
| - name: Check if user wants to become a designer | ||
| id: check_designer | ||
| continue-on-error: true | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| ISSUE_NUMBER: ${{ env.ISSUE_NUMBER }} | ||
| USERNAME: ${{ env.USERNAME }} | ||
| run: | | ||
| ADD_TO_DESIGNERS=$(gh issue view ${{ env.ISSUE_NUMBER }} --json body | jq -e '.body | contains("[x] Do you wish to join the designers team?")') | ||
| ADD_TO_DESIGNERS=$(gh issue view ${ISSUE_NUMBER} --json body | jq -e '.body | contains("[x] Do you wish to join the designers team?")') | ||
| if [ "$ADD_TO_DESIGNERS" = "true" ]; then | ||
| python scripts/add_member.py ${{ env.USERNAME }} designers | ||
| python scripts/add_member.py ${USERNAME} designers | ||
| else | ||
| echo "User does not want to join the designers team." | ||
| fi | ||
|
|
||
| - name: Add user to the list | ||
| run: | | ||
| python scripts/add_member.py ${{ env.USERNAME }} members | ||
| python scripts/add_member.py ${USERNAME} members | ||
| env: | ||
| USERNAME: ${{ env.USERNAME }} | ||
|
|
||
| - name: Commit changes | ||
| run: | | ||
| git add terraform/production/org.tfvars | ||
| git commit -m "Add ${{ env.USERNAME }} to django-commons" | ||
| git push origin ${{ env.BRANCH_NAME }} | ||
| git commit -m "Add ${USERNAME} to django-commons" | ||
| git push origin ${BRANCH_NAME} | ||
| env: | ||
| USERNAME: ${{ env.USERNAME }} | ||
| BRANCH_NAME: ${{ env.BRANCH_NAME }} | ||
|
|
||
| - name: Create pull request | ||
| run: | | ||
| gh pr create \ | ||
| --title "Add ${{ env.USERNAME }} to django-commons" \ | ||
| --body "Fix #${{ env.ISSUE_NUMBER }}" \ | ||
| --title "Add ${USERNAME} to django-commons" \ | ||
| --body "Fix #${ISSUE_NUMBER}\n[New Member Playbook](https://github.com/django-commons/controls?tab=readme-ov-file#new-member-playbook)" \ | ||
| --base main \ | ||
| --head ${{ env.BRANCH_NAME }} \ | ||
| --head ${BRANCH_NAME} \ | ||
| --label "New member" | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.TERRAFORM_MANAGEMENT_GITHUB_TOKEN }} | ||
| USERNAME: ${{ env.USERNAME }} | ||
| ISSUE_NUMBER: ${{ env.ISSUE_NUMBER }} | ||
| BRANCH_NAME: ${{ env.BRANCH_NAME }} | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Something seems off here. Either we don't need to make these re-assignments, or we're no longer capturing the proper data.
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'm not sure I understand. What zizmor was pointing out in was that the environment variables could be exposed to injection via shell expansion. By refactoring this was that risk is mitigated. I'm seeing all of the variables being used below ... feels like I'm missing something now 😄
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.
This seems correct to me. By makings these reassignments and using them like this, it stops injection attacks.
It does look a bit weird and unnecessary on first glance, but it is absolutely on purpose 😄