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

Migrate to pnpm, publish via GitHub Actions with provenance #122

Closed
dcroote opened this issue Nov 29, 2023 · 14 comments · Fixed by #137
Closed

Migrate to pnpm, publish via GitHub Actions with provenance #122

dcroote opened this issue Nov 29, 2023 · 14 comments · Fixed by #137
Assignees
Labels
enhancement New feature or request

Comments

@dcroote
Copy link
Contributor

dcroote commented Nov 29, 2023

A nice to have, see the discussion as well as the blog post on npm package provenance.

@dcroote dcroote added the enhancement New feature or request label Nov 29, 2023
@bbenligiray
Copy link
Member

As a note, the "publish package" steps in our processes are becoming a significant pain point

@dcroote
Copy link
Contributor Author

dcroote commented Feb 20, 2024

It took a handful of commits, but my fork now has an automated Actions workflow that publishes a package to npm with provenance (scroll to the bottom of the linked page) and releases to GitHub.

It uses changesets and works like so:

  • Every commit to main will trigger the changeset action to look at the changesets since the last release
  • It will aggregate these changesets and decide on the next appropriate version (e.g. if there was a few patches, then patch; if a minor then minor, etc.)
  • It will create a PR
  • If we don't want to release, then we just don't merge the PR and it hangs around, getting updated automatically on each main push
  • If we do merge, then it releases to npm and GitHub

Other notes:

  • I needed to add a repository url to package.json
  • I chose changeset as the release flow option because we use changesets in other repos and so it seemed like the better option than trying to change behavior and force folks to use conventional commit messages, which is necessary for other release workflows
  • I replaced yarn with pnpm

Thoughts @andreogle @bbenligiray ? Shall I create a PR?

@andreogle
Copy link
Member

I think I like it 👍🏻 My first thought was that having a separate release branch would be better than using main, but if the PR gets auto-updated and we just need to hit "merge" to release, then that sounds quite nice.

force folks to use conventional commit messages

this is never going to work 😂 changeset sounds good and seems to work well

replaced yarn with pnpm

It seems like we may have crossed the threshold where this is the preferred tool for API3 repos (?) Sounds great to me if so

@bbenligiray
Copy link
Member

I can see a few cases that this doesn't support. For example, if we have a major and a minor change lined up on main and we want to release only the minor change, the automatic PR will want us to release the major change as well. Also, it doesn't seem to support patching previous major/minor versions. I guess to be able to support these, minor-specific release branches as in Airnode (https://github.com/api3dao/airnode/tree/v0.3, https://github.com/api3dao/airnode/tree/v0.4, etc.) are needed. That being said, I think OIS releases have been exclusively linear so this workflow would work here in practice.

Is it possible to require a number of approvals on the release PR?

@hiletmis can you check the provenance part out? The point is that the releases that do it has a green tick.

@bbenligiray
Copy link
Member

Would provenance also work here?

@hiletmis
Copy link

@bbenligiray tried on logos.

I will do the necessary changes on the other repos as well

thanks @dcroote for the feature

@dcroote
Copy link
Contributor Author

dcroote commented Feb 23, 2024

For example, if we have a major and a minor change lined up on main and we want to release only the minor change, the automatic PR will want us to release the major change as well.

Correct, though I've struggled to think of when we'd have both merged to main but then not want to release the major. Did you have a situation in mind?

Also, it doesn't seem to support patching previous major/minor versions

As you suggest, I would create a branch (if one didn't already exist), but then add the branch to the workflow trigger here so that changesets releases the appropriate version against that branch containing the patch. The fallback could always be manual release.

Would provenance also work here?

Quite possibly. The Airnode docker build steps are a bit complicated, but the api3-dao-dashboard already uses the docker/build-push-action GitHub Action that automatically adds provenance attestations

Is it possible to require a number of approvals on the release PR?

I'm not sure this can be done based on GitHub's branch protection rules. The number of reviewer approvals can be set, but only at the branch level. Right now this repo requires: PRs to merge, 1 approval, and status checks before merging. There is the option of "Require review from Code Owners" that might get at what you want.

thanks @dcroote for the feature

Sure, and @hiletmis if you want the PRs and commits to have the release version see this step and how it's incorporated.

@bbenligiray
Copy link
Member

Correct, though I've struggled to think of when we'd have both merged to main but then not want to release the major. Did you have a situation in mind?

It's common to bundle breaking changes together to reduce the number of major releases. Breaking changes also typically take a longer time to implement. Of multiple breaking changes, one will hit main first, and in the case that we want to bundle all breaking changes together, we would have to wait until all breaking changes are completed before being able to make a new release, which would potentially take a long time. Again though, I'm thinking generally, OIS may not be affected by this in practice.

As you suggest, I would create a branch (if one didn't already exist), but then add the branch to the workflow trigger here so that changesets releases the appropriate version against that branch containing the patch. The fallback could always be manual release.

We probably wouldn't need manual release at all in this case. This may be overkill for something like OIS that historically had a linear release path (no backporting) though.

Quite possibly. The Airnode docker build steps are a bit complicated, but the api3-dao-dashboard already uses the docker/build-push-action GitHub Action that automatically adds provenance attestations

Documented in api3dao/airnode#1956

I'm not sure this can be done based on GitHub's branch protection rules. The number of reviewer approvals can be set, but only at the branch level. Right now this repo requires: PRs to merge, 1 approval, and status checks before merging. There is the option of "Require review from Code Owners" that might get at what you want.

It looks like we can require an arbitrary number of approvals or approval of code owners, but this may be annoying on main. Having a special release branch (or multiple for different versions) would allow stricter requirements to be practical.

@dcroote
Copy link
Contributor Author

dcroote commented Mar 7, 2024

OK- after a bunch of exploration the following can work, which I believe satisfies everything. Note it is slightly more laborious because of the multiple PRs, but I think that is inescapable if we want stricter release protections.

  • We have a release branch with stricter branch protection rules (required PR before merging, number of reviewers, a code owner). This is nice because we don't have to enforce stricter branch protections on main and main development can continue in parallel with a release.
  • When we decide to make a release: we open a PR from main to release. In my fork: Merge main into release dcroote/ois#8
  • Since the PR is against release it requires the aforementioned approvals
  • The PR is merged, which triggers the changeset release Actions workflow which runs on push to release.
  • The Actions workflow sees changesets and creates a release PR against the release branch. Release v2.3.8 dcroote/ois#9
  • Since the PR is against release it requires the aforementioned approvals
  • The PR is merged, which publishes to npm with provenance and to GitHub Releases
  • A PR from release to main is created and merged. Merge release into main dcroote/ois#10
  • Repeat

What is nice is while the release process is occurring, development can continue on main without issue (I tested by pushing a commit to main after publishing but before merging release back to main).

Note that I believe merging of PRs to and from release should be done with "Create a merge commit" to preserve the commit history, which is particularly important for preserving the tagged release commit. I think the Actions PR itself can be squashed or rebased to avoid one extra merge commit.

@andreogle
Copy link
Member

Sounds simple enough to me.

@bbenligiray
Copy link
Member

Sounds good, this is mostly what we're doing atm with the data feed operations (except the release branch isn't protected yet)

A PR from release to main is created and merged. dcroote#10

I assume you opened this PR manually. I wonder if it can be opened automatically after the release.

@dcroote
Copy link
Contributor Author

dcroote commented Mar 11, 2024

I assume you opened this PR manually. I wonder if it can be opened automatically after the release.

Indeed it can be, good thought.

@bbenligiray
Copy link
Member

@hiletmis see above. We tend to merge production back to main manually, it would be nice if a PR that does this opened automatically after production publishes

@hiletmis
Copy link

@dcroote

Adding following to the release workflow should do the job;

      - name: Create pull request
        if: steps.changesets.outputs.published == 'true'
        run: gh pr create -B main -H production --title 'Merge production into main' --body 'Merges production into main'
        env:
            GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants