-
Notifications
You must be signed in to change notification settings - Fork 22
Reusable workflows for npm publish #443
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| # Adopting shared & reusable GitHub Actions for publishing pipelines | ||
|
|
||
| ## Summary | ||
|
|
||
| This RFC proposes standardizing all **publish and release automation** workflows across the Express.js organization using **shared, reusable GitHub Actions**. These workflows will handle packaging and secure publication to npm, ensuring consistent and reliable release processes across all repositories. This workflows will be centrally maintained, versioned, and consumed by individual repositories to ensure consistency, reliability, and easier maintenance. | ||
|
|
||
| ## Motivation | ||
|
|
||
| Currently, each repository in the Express.js organization maintains its own GitHub Actions workflows. This leads to: | ||
|
|
||
| - Inconsistent CI/CD practices (different Node versions, test commands, caching, verification steps, etc.) | ||
| - Duplicated effort across repositories | ||
| - Increased probability of misconfiguration or security vulnerabilities | ||
| - Difficulty onboarding new contributors and maintainers | ||
| - Harder organization-wide upgrades (e.g., Node.js version updates or introducing new security validations) | ||
|
|
||
| By moving to shared reusable workflows, we expect to: | ||
|
|
||
| - Ensure consistent release standards across all repositories | ||
| - Reduce maintenance overhead and simplify updates | ||
| - Improve security by enforcing centralized best practices | ||
| - Enable easier shared improvements for all repositories | ||
| - Support future capabilities such as provenance and trusted publisher | ||
|
|
||
| ## Detailed Explanation | ||
|
|
||
| ### Proposal | ||
|
|
||
| - Create and maintain centralized reusable workflows under a repository such as: | ||
| - `expressjs/ci-workflows` (repository name to be decided later) | ||
|
|
||
| - This repository will include: | ||
| - `release.yml` — publish to npm and create GitHub releases (optional approval gates) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some packages need to be built before publish, which requires installation of dev dependencies. Also, it'd probably be good to add some verification that the |
||
|
|
||
| ```yaml | ||
| - uses: step-security/wait-for-secrets@v1 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we comfortable adding a third-party dependency here? It would require expending the "trusted actions" of the organization too, which isn't in this proposal. It also appears that it might not be maintained with 19 open PRs and no work in almost a year. |
||
| id: wait-for-secrets | ||
| with: | ||
| secrets: | | ||
| OTP: | ||
| name: 'OTP to publish package' | ||
| description: 'NPM 2FA' | ||
|
|
||
| - name: publish | ||
| run: | | ||
| export NODE_AUTH_TOKEN=${{ secrets.NPM_PUBLISH }} | ||
| npm publish --otp ${{ steps.wait-for-secrets.outputs.OTP }} --access public | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why 2FA instead of trusted publishing? Should we wait for secrets for both the token and 2FA, to promote people creating one off tokens as needed instead of having to keep dozens of granular tokens up to date and rotated?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Who can fill in the information for the |
||
| ``` | ||
|
|
||
| - Each repository will consume them using `workflow_call`, for example: | ||
|
|
||
| ```yaml | ||
| name: Publish package | ||
| on: | ||
| release: | ||
| types: [released] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use tags instead of releases? It might be confusing to create a release and have people try and install it before it's available. |
||
|
|
||
| permissions: | ||
| id-token: write | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this if we aren't using trusted publishing? |
||
| contents: read | ||
|
|
||
| jobs: | ||
| publish: | ||
| uses: expressjs/ci-workflows/.github/workflows/release.yaml@v1 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's a repo of different workflows, should versioning by prefixed by the "workflow"? |
||
| secrets: | ||
| NPM_PUBLISH: ${{ secrets.NPM_PUBLISH }} | ||
| ``` | ||
|
|
||
| - Customization will still be possible using workflow inputs and conditionals. | ||
| - Workflows will be versioned (`v1`, `v1.1`, etc.) to ensure stability and exact sha can be used. | ||
|
|
||
| ### Migration | ||
|
|
||
| 1. Create shared workflow repository and initial pipeline definitions. | ||
| 2. Document how to consume workflows, expected defaults, and available inputs. | ||
| 3. Migrate a few core repositories (`express`, `router`, `body-parser`) as a pilot. | ||
| 4. Expand adoption across the organization. | ||
| 5. Deprecate custom pipelines once migration is completed. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have custom pipelines already? |
||
|
|
||
| ## Rationale and Alternatives | ||
|
|
||
| | Approach | Pros | Cons | | ||
| |----------|------|------| | ||
| | **Shared reusable workflows (proposed)** | Consistent CI/CD, easier maintenance, improved security, one update applies to all repos | Requires initial setup and governance | | ||
| | **Status quo: per-repository workflows** | Maximum flexibility per repository | Inconsistent behavior, duplicated code, high maintenance, increased risk of outdated CI | | ||
|
|
||
| This proposal offers the best balance of maintainability, consistency, and openness for the Express.js ecosystem. | ||
|
|
||
| ## Implementation | ||
|
|
||
| ### Affected Areas | ||
|
|
||
| - All active repositories using GitHub Actions under the `expressjs/` organization. | ||
| - All active repositories using GitHub Actions under the `pillarjs/` organization. | ||
| - All active repositories using GitHub Actions under the `jshttp/` organization. | ||
| - No runtime code changes required; only CI configuration updates. | ||
|
|
||
| ### Actions Required | ||
|
|
||
| - Create new organization repository. | ||
| - Implement and document reusable workflows. | ||
| - Configure organization-wide secrets (npm token, GitHub token, etc.) if required. | ||
| - Roll out reusable workflows in prioritized repositories. | ||
| - Establish contribution guidelines and versioning strategy for workflow updates. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would/could this repo that's created operate under the existing "captain" model? |
||
|
|
||
| ### Technical Considerations | ||
|
|
||
| - Use `workflow_call` and workflow permissions properly. | ||
| - Use only organization-level secrets, not per-repo secrets for shared steps. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would this work for the NPM token? Which user is publishing? Do we need to create a new shared user if we're using tokens? That feels like it opens up a new vulnerable space. |
||
| - Release workflows should include safeguards (e.g., manual approval for npm publish). | ||
|
|
||
| ## Prior Art | ||
|
|
||
| - GitHub officially recommends this pattern for large organizations. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you say "this pattern" do you mean a shared workflow, or using NPM tokens and publishing from CI? Some of the doc seems to refer to creating a shared workflow repo which I think is not contentious, but other parts are specifically referring to the publish script, which is a bit more contentious. |
||
|
|
||
| ## Unresolved Questions and Bikeshedding | ||
|
|
||
| - What should the shared workflow repository be named? | ||
| - Should `npm audit` or CodeQL scanning be mandatory by default? | ||
| - How should versions of workflows be managed - tagged releases (`v1`) or branch references (`main`)? | ||
| - Should automatic npm releases be allowed, or manual approvals required? | ||
| - Do we want to pass the npm token using `export NODE_AUTH_TOKEN=${{ secrets.NPM_PUBLISH }}` or use the `.npmrc` file for that? | ||
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.
Why one repo for everything instead of as an action that does one thing?