chore: modify preview deployment script to work with forks #13404

dannykopping · 2024-05-30T06:37:43Z

I maintain a fork of coder at dannykopping/coder.

When a branch exists in a fork only, the current preview deployment scripts break.
This PR modifies the script & action to reference the coder/coder repo explicitly for the PR.

I also changed to pr status because pr view does not work with --repo.

… coder/coder repo Signed-off-by: Danny Kopping <danny@coder.com>

matifali · 2024-05-30T06:53:11Z

Does this make it work with forks and community contributors? We are using a few repo secrets in the deployment workflow, and I am not sure if this workflow allows forks to use the secret values.

dannykopping · 2024-05-30T06:59:12Z

Does this make it work with forks and community contributors? We are using a few repo secrets in the deployment workflow, and I am not sure if this workflow allows forks to use the secret values.

I don't think so (at least that was not my intent), it just modified so that PRs created using forks would work.
I'm adding a clause to the action that only members of the coder org can trigger this action.

Signed-off-by: Danny Kopping <danny@coder.com>

matifali · 2024-05-30T07:11:09Z

.github/workflows/pr-deploy.yaml

+      - name: Check if actor is a member
+        run: |
+          set -euo pipefail
+          response=$(curl -s -o /dev/null -w "%{http_code}" -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" "https://api.github.com/orgs/coder/members/${{ github.actor }}")


Are you sure this works in each case? For example, https://api.github.com/orgs/coder/members/matifali returns

{ "message": "User does not exist or is not a public member of the organization", "documentation_url": "https://docs.github.com/rest/orgs/members#check-public-organization-membership-for-a-user" }

It seems like it works for private members:

$ curl -s -H "Authorization: token $(gh auth token)" "https://api.github.com/orgs/coder/members/matifali" -v ... < HTTP/2 204 ...

I'm using https://docs.github.com/en/rest/orgs/members?apiVersion=2022-11-28#check-organization-membership-for-a-user

Yes, all works great. I was testing without the token and always getting a 302. Thank you for adding the check.

It would be nice to test by deploying the current PR.

I tried that in #13280 (where I originally had this fix) but GH didn't seem to use the new workflow. Is there a timeout maybe? I'll give it a try though.

Damn, I tried to execute this but I got:

$ ./scripts/deploy-pr.sh -d -b -y branchName: dk/fix-preview prNumber: 13404 experiments: build: true deploy: true could not create workflow dispatch event: HTTP 422: No ref found for: dk/fix-preview (https://api.github.com/repos/coder/coder/actions/workflows/60960476/dispatches)

I think the branch has to be in the main repo.
Strange that I didn't get this before, though.

Oh, looks like I must've pushed dk/verify-agent (the branch I was testing with previously) to the main repo somehow?

I am noticing the same issue. It's deploying the previous branch I was working on. node-20
#13404 (comment)

dannykopping · 2024-05-30T07:21:22Z

I've opened #13405 to validate.

matifali · 2024-05-30T07:24:14Z

Thank you. This is a good improvement. I plan to make this flow better sometimes #11331

matifali · 2024-05-30T07:29:19Z

I tried deploying this PR but its detecting the wrong branch and PR number

gh pr checkout 13404
remote: Enumerating objects: 12, done.
remote: Counting objects: 100% (12/12), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 12 (delta 10), reused 12 (delta 10), pack-reused 0
Unpacking objects: 100% (12/12), 1.71 KiB | 877.00 KiB/s, done.
From https://github.com/coder/coder
 * [new ref]             refs/pull/13404/head -> dk/fix-preview
Switched to branch 'dk/fix-preview'

git status
On branch dk/fix-preview
nothing to commit, working tree clean

./scripts/deploy-pr.sh
Are you sure you want to deploy? (y/n) y
branchName: node-20
prNumber: 12921
experiments:
build: false
deploy: false
✓ Created workflow_dispatch event for pr-deploy.yaml at node-20

To see runs for this workflow, try: gh run list --workflow=pr-deploy.yaml

dannykopping · 2024-05-30T07:31:00Z

I tried deploying this PR but its detecting the wrong branch and PR number

gh pr checkout 13404
remote: Enumerating objects: 12, done.
remote: Counting objects: 100% (12/12), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 12 (delta 10), reused 12 (delta 10), pack-reused 0
Unpacking objects: 100% (12/12), 1.71 KiB | 877.00 KiB/s, done.
From https://github.com/coder/coder
 * [new ref]             refs/pull/13404/head -> dk/fix-preview
Switched to branch 'dk/fix-preview'

git status
On branch dk/fix-preview
nothing to commit, working tree clean

./scripts/deploy-pr.sh
Are you sure you want to deploy? (y/n) y
branchName: node-20
prNumber: 12921
experiments:
build: false
deploy: false
✓ Created workflow_dispatch event for pr-deploy.yaml at node-20

To see runs for this workflow, try: gh run list --workflow=pr-deploy.yaml

Weird, it works for me:

$ gh pr status --repo=coder/coder --json headRefName,number --jq '.createdBy[0]' | cat
{"headRefName":"dk/fix-preview","number":13404}

matifali · 2024-05-30T07:33:39Z

Strangely for me

gh pr view --json headRefName | jq -r .headRefName
dk/fix-preview
gh pr view --json number | jq -r .number
13404

but

gh pr status --repo=coder/coder --json headRefName,number --jq '.createdBy[0]' | cat
{"headRefName":"node-20","number":12921}

dannykopping · 2024-05-30T07:34:57Z

Looking into it, pr list seems like a better option.

dannykopping · 2024-05-30T07:35:41Z

Can you try this @matifali?

$ gh pr list --repo=coder/coder -H $(git rev-parse --abbrev-ref HEAD) --json number --jq '.[].number' | cat
13404

dannykopping · 2024-05-30T07:36:07Z

Alternatively we should skip the convenience and just accept a PR number as an argument.

matifali · 2024-05-30T07:37:44Z

gh pr list --repo=coder/coder -H $(git rev-parse --abbrev-ref HEAD) --json number --jq '.[].number' | cat
13404
Also
gh pr status --json headRefName,number --jq '.currentBranch' | cat
{"headRefName":"dk/fix-preview","number":13404}

.github/workflows/pr-deploy.yaml

dannykopping · 2024-05-30T07:40:17Z

I don't trust pr status anymore, I'm seeing different results:

$ gh pr status --repo=coder/coder --json headRefName,number
{"createdBy":[{"headRefName":"dk/fix-preview","number":13404},{"headRefName":"dk/verify-agent","number":13280}],"needsReview":[]}

I think the script should just accept a PR as input. WDYT?

scripts/deploy-pr.sh

matifali · 2024-05-30T07:42:33Z

@dannykopping I think as the branch is actually not on the coder/coder repo, that's why we are not able to get it. Skipping --repo=coder/coder seems to work fine along with .currentBranch

dannykopping · 2024-05-30T07:43:26Z

@dannykopping I think as the branch is actually not on the coder/coder repo, that's why we are not able to get it. Skipping --repo=coder/coder seems to work fine along with .currentBranch

Not so for me:

$ gh pr status --json headRefName,number --jq '.currentBranch' | cat

dannykopping · 2024-05-30T07:48:15Z

pr list is reliable, and works irrespective of forks.

$ ./scripts/deploy-pr.sh -d -n -y                                                                          
dry run
branchName: dk/verify-agent
prNumber: 13280
experiments: 
build: false
deploy: true
$ git co dk/fix-preview 
Switched to branch 'dk/fix-preview'
$ ./scripts/deploy-pr.sh -d -n -y
dry run
branchName: dk/fix-preview
prNumber: 13404
experiments: 
build: false
deploy: true

Signed-off-by: Danny Kopping <danny@coder.com>

matifali · 2024-05-30T07:50:54Z

@dannykopping OK lets use pr list push your changes, and I can try to run it from coder/coder

dannykopping · 2024-05-30T07:51:52Z

@dannykopping OK lets use pr list push your changes, and I can try to run it from coder/coder

$ ./scripts/deploy-pr.sh -d                                                                              
Are you sure you want to deploy? (y/n) y
branchName: dk/fix-preview
prNumber: 13404
experiments: 
build: false
deploy: true
could not create workflow dispatch event: HTTP 422: No ref found for: dk/fix-preview (https://api.github.com/repos/coder/coder/actions/workflows/60960476/dispatches)

Same issue as before (not that I expected this to be fixed with this change, but worth noting).

matifali · 2024-05-30T07:52:47Z

It works fine now in dry run. Can you try deploying from the fork for real? So that we can test if all secrets are accessible, too.

matifali · 2024-05-30T07:54:23Z

@dannykopping OK lets use pr list push your changes, and I can try to run it from coder/coder
$ ./scripts/deploy-pr.sh -d                                                                              
Are you sure you want to deploy? (y/n) y
branchName: dk/fix-preview
prNumber: 13404
experiments: 
build: false
deploy: true
could not create workflow dispatch event: HTTP 422: No ref found for: dk/fix-preview (https://api.github.com/repos/coder/coder/actions/workflows/60960476/dispatches)
Same issue as before (not that I expected this to be fixed with this change, but worth noting).

Same issue for me. Can not deploy.

matifali · 2024-05-30T07:55:00Z

I think we need to allow running this workflow on forks in repo settings.

dannykopping · 2024-05-30T07:56:54Z

@dannykopping OK lets use pr list push your changes, and I can try to run it from coder/coder
$ ./scripts/deploy-pr.sh -d                                                                              
Are you sure you want to deploy? (y/n) y
branchName: dk/fix-preview
prNumber: 13404
experiments: 
build: false
deploy: true
could not create workflow dispatch event: HTTP 422: No ref found for: dk/fix-preview (https://api.github.com/repos/coder/coder/actions/workflows/60960476/dispatches)
Same issue as before (not that I expected this to be fixed with this change, but worth noting).
Same issue for me. Can not deploy.

I think this is actually a good thing.

gh workflow run in the deploy script is trying to reference the workflow script in a branch which is outside the main repo. I don't think we should enable this.

I think we'll have to close this PR.

If the my branch from #13280 was not present in coder/coder (like I was expecting), then I would've gotten the 422 as above. So this whole PR was fruit of the poisonous tree.

matifali · 2024-05-30T07:58:58Z

gh workflow run in the deploy script is trying to reference the workflow script in a branch which is outside the main repo. I don't think we should enable this.

Yes we do have the protection to check for membership but that check is part of the same workflow so not safe at all.

github-actions bot assigned dannykopping May 30, 2024

Modify preview deployment script to work with forks by specifying the…

100433f

… coder/coder repo Signed-off-by: Danny Kopping <danny@coder.com>

dannykopping force-pushed the dk/fix-preview branch from fb5f588 to 100433f Compare May 30, 2024 06:38

dannykopping changed the title ~~Modify preview deployment script to work with forks~~ chore: modify preview deployment script to work with forks May 30, 2024

dannykopping requested a review from matifali May 30, 2024 06:49

matifali approved these changes May 30, 2024

View reviewed changes

Restrict action to members of the org

d87fc2b

Signed-off-by: Danny Kopping <danny@coder.com>

matifali reviewed May 30, 2024

View reviewed changes

.github/workflows/pr-deploy.yaml Outdated Show resolved Hide resolved

matifali reviewed May 30, 2024

View reviewed changes

scripts/deploy-pr.sh Outdated Show resolved Hide resolved

Use "pr list" instead

c1e0a11

Signed-off-by: Danny Kopping <danny@coder.com>

dannykopping closed this May 30, 2024

github-actions bot locked and limited conversation to collaborators May 30, 2024

chore: modify preview deployment script to work with forks #13404

chore: modify preview deployment script to work with forks #13404

Uh oh!

Conversation

dannykopping commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matifali commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matifali May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

matifali commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matifali commented May 30, 2024

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

matifali commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

matifali commented May 30, 2024

Uh oh!

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

Uh oh!

matifali commented May 30, 2024

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

matifali commented May 30, 2024

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

matifali commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matifali commented May 30, 2024

Uh oh!

matifali commented May 30, 2024

Uh oh!

dannykopping commented May 30, 2024

Uh oh!

matifali commented May 30, 2024

Uh oh!

Uh oh!

dannykopping commented May 30, 2024 •

edited

Loading

matifali commented May 30, 2024 •

edited

Loading

matifali May 30, 2024 •

edited

Loading

matifali commented May 30, 2024 •

edited

Loading

matifali commented May 30, 2024 •

edited

Loading

matifali commented May 30, 2024 •

edited

Loading