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

Create manually triggered Github Action for running CI on behalf of forks #6399

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RamIdeas
Copy link
Contributor

@RamIdeas RamIdeas commented Aug 1, 2024

This PR introduces a GIthub Action which can be triggered manually to clone a fork to a new branch and open a draft PR with the "e2e" label which will (hopefully) run the e2e jobs.

Inputs:

  • PR number type: string (the PR to clone from)
  • Reviewed type: boolean (to confirm the user has reviewed the PR for usage/leakage of secrets)

Also:

  • The user triggering the action is enforced to be on the wrangler team

@RamIdeas RamIdeas requested a review from a team as a code owner August 1, 2024 16:11
Copy link

changeset-bot bot commented Aug 1, 2024

⚠️ No Changeset found

Latest commit: f00298a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Comment on lines 19 to 23
- name: Check user for team affiliation
uses: tspascoal/get-user-teams-membership@v2
id: teamAffiliation
with:
GITHUB_TOKEN: ${{ secrets.GH_ACCESS_TOKEN }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it safe to pass this token to a third-party action?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good point. The source code is "obfuscated" in some sense because dist is built and committed to the repo, so there is a security issue there.

We might look instead at copy/pasting the source into our own source code and calling it directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But forking that repo into our org might be the safest option

Copy link
Contributor

github-actions bot commented Aug 1, 2024

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-wrangler-6399

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6399/npm-package-wrangler-6399

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-wrangler-6399 dev path/to/script.js
Additional artifacts:
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-create-cloudflare-6399 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-cloudflare-kv-asset-handler-6399
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-miniflare-6399
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-cloudflare-pages-shared-6399
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-cloudflare-vitest-pool-workers-6399
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-cloudflare-workers-editor-shared-6399
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-cloudflare-workers-shared-6399
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11939308871/npm-package-cloudflare-workflows-shared-6399

Note that these links will no longer work once the GitHub Actions artifact expires.


[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241106.0
workerd 1.20241106.1 1.20241106.1
workerd --version 1.20241106.1 2024-11-06

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

GITHUB_TOKEN: ${{ secrets.GH_ACCESS_TOKEN }}
username: ${{ github.actor }}
team: wrangler
- name: Stop workflow if user is no member
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Stop workflow if user is no member
- name: Stop workflow if user is not a "wrangler" team member

?

run: gh pr checkout ${{ inputs.pr-number }}

- name: "Create Draft PR"
run: git checkout -b run-ci-on-hehalf-of-${{ inputs.pr-number }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we run a branch -D on this branch first? Unlikely conflict, but possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was considering this needs to be repeatable. I was thinking a force push or something once I confirmed this all worked but branch -D would also work – do we need to delete the remote branch too? and close the existing PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

If that's possible then it seems like a good idea, to avoid needing any potential manual cleanup.

@andyjessop
Copy link
Contributor

One more thing we might consider is to update this section of CONTRIBUTING.md, explaining that some tests may fail and that it requires a wrangler team member to run the action.

@CarmenPopoviciu CarmenPopoviciu added the awaiting reporter response Needs clarification or followup from OP label Aug 15, 2024
@penalosa penalosa removed the awaiting reporter response Needs clarification or followup from OP label Aug 21, 2024
@penalosa penalosa requested review from a team as code owners November 20, 2024 16:35
@workers-devprod workers-devprod added the e2e Run e2e tests on a PR label Nov 20, 2024
@penalosa penalosa added skip-pr-description-validation Skip validation of the required PR description format and removed e2e Run e2e tests on a PR labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-pr-description-validation Skip validation of the required PR description format
Projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

5 participants