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

build: modfying publish action to use npm cli to include provenance in npm publish #319

Merged
merged 7 commits into from
Dec 7, 2023

Conversation

rsoberano-ld
Copy link
Contributor

@rsoberano-ld rsoberano-ld commented Nov 22, 2023

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Describe the solution you've provided

Yarn publish doesn't currently support NPM's publish package with provenance functionality. As a workaround until this is supported, we'll pack the workspace with yarn in order to guarantee we get the same package as before, and then use the npm cli to do the final publish with provenance. This also involves passing the workspace path as input parameters to the publish actions/script as npm's workspace functionality doesn't work exactly the same as yarns.

While npm's generated provenance isn't the most robust provenance attestation, it results in a verified checkmark on the npm package page, which brings the provenance closest to the consumer and makes it most useful.

Describe alternatives you've considered

Ideally we'd want yarn to support this natively, but tracking the yarn repo issues for the past couple months has shown no movement here.

Additional context

Add any other context about the pull request here.

@rsoberano-ld rsoberano-ld marked this pull request as ready for review December 1, 2023 00:45
@rsoberano-ld rsoberano-ld force-pushed the rsoberano/SESC-4384/js-core-sigstore branch from 7396b94 to 4a17644 Compare December 6, 2023 22:03
@rsoberano-ld
Copy link
Contributor Author

Compare with changes made to learn-release-please/npm-provenance-test repo during npm provenance testing: https://github.com/launchdarkly/learn-release-please/compare/7e9e9b348398ea077d789b535ef4d7b937f2aeae...rsoberano/SEC-4384/js-core-sigstore-updates

@@ -41,6 +41,9 @@ jobs:
with:
node-version: 16.x
registry-url: 'https://registry.npmjs.org'
- name: 'Install latest npm to support provenance publishing'
run: |
npm install -g npm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The npm that comes with node-version: 16.x is an older version that doesn't support the --provenance option, so adding a step here to update it to latest per https://docs.npmjs.com/downloading-and-installing-node-js-and-npm/.

Open to other suggestions of how to update npm here, though

@@ -80,6 +86,9 @@ jobs:
with:
node-version: 16.x
registry-url: 'https://registry.npmjs.org'
- name: 'Install latest npm to support provenance publishing'
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should make an action for this?

uses: ./actions/install-latest-npm

steps:
- name: 'Install latest npm to support provenance publishing'
run: |
npm install -g npm
Copy link
Member

Choose a reason for hiding this comment

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

I think you are going to need a shell: bash entry. If you have your IDE setup with the github actions schema it should show you an error.

Could we add a newline to the end of the file as well.

@rsoberano-ld rsoberano-ld merged commit 78aac46 into main Dec 7, 2023
15 checks passed
@rsoberano-ld rsoberano-ld deleted the rsoberano/SESC-4384/js-core-sigstore branch December 7, 2023 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants