Skip to content

Enable Build Provenance for Nightly Builds #59013

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.

Sign up for GitHub

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
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Enable Build Provenance for Nightly Builds #59013

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d98be4
Enable Build Provenance for Nightly Builds
weswigham Jun 25, 2024
7c86376
Update .github/workflows/nightly.yaml
weswigham Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/nightly.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ on:

permissions:
contents: read
id-token: write

# Ensure scripts are run with pipefail. See:
# https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference
Expand Down Expand Up @@ -60,6 +61,10 @@ jobs:
npx hereby configure-nightly
npx hereby LKG
node ./scripts/addPackageJsonGitHead.mjs package.json
npm publish --tag next
npm --version
# update npm to latest for build provenance feature on publish
npm install -g npm
npm --version
npm publish --provenance --access public --tag next
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will work; CI uses the version of npm in package.json, which is v8, and it doesn't seem like it has that flag:

Publish a package

Usage:
npm publish <package-spec>

Options:
[--tag <tag>] [--access <restricted|public>] [--dry-run] [--otp <otp>]
[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
[-ws|--workspaces] [--include-workspace-root]

Run "npm help publish" for more info

Copy link
Member

Choose a reason for hiding this comment

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

(we're pinned to v8 since we need to ensure the lockfile stays at v2 so that older node in CI can install the right stuff, but I suppose we could instead do a node version swap in those jobs after install)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we have a reason we can't bump it for this task?

Copy link
Member Author

Choose a reason for hiding this comment

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

Rather, I mean: we could just install latest npm just for the publish line here.

Copy link
Member

Choose a reason for hiding this comment

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

Then we'd be publishing with an untested version of node; we've seen that the files array globbing rules changed previously and broke a published package, so I'm moderately nervous about that. I think I'd sooner bump us to a newer npm and then just do "use node 20, install, switch to node 14, test"

Copy link
Member Author

@weswigham weswigham Jun 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The node versions are definitely all still tested, it's moreso the npm publish result isn't - which is fair, but we also don't test that. At all. That'd be like a post-publish API test watchdog, which is like... a couple levels deeper an integration test than we bother with right now, and even then that would be an after-the-fact thing (though you could make the argument the playground fills this purpose) - it's just impossible for us to know how npm is going to roundtrip the package (though we probably can have reasonable expectations) through the publishing operation (though npm pack is a... partial preview?). And honestly, if npm@latest breaks what we publish, I'd rather know sooner (in the nightly, which can break from time to time, and if it does, people and infra complain) than later (whenever we get around to updating the frozen version string).

Copy link
Member

Choose a reason for hiding this comment

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

That's fair, though I also think it'd be odd for our nightly releases to have providence but not our stable builds... 😄

Copy link
Member Author

@weswigham weswigham Jun 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's just easy to turn on to try in our nightlies and tweak until it's right (at hopefully minimal cost to us), and then we have another month or so before we even make our next stable build to consider if we can/it's worth finally fully migrating off of azure pipelines for releases (provenance as is right now is only works on github actions and gitlab runners - presumably because those are the runners where npm knows how to find the build steps for the publish (not that build steps alone get you a truly reproducible build, but that's why it's "provenance" and not "reproducible builds" - it's a weaker guarantee)).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if we use Actions, then I'd probably even go to the highest level, e.g. https://github.com/jakebailey/hereby/blob/main/.github/workflows/release.yml#L33 (which works well).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather just stick with what npm provides for now - it's far more likely to become defacto standard than the bespoke actions are.

env:
NODE_AUTH_TOKEN: ${{secrets.npm_token}}