Skip to content

Conversation

@sagudev
Copy link
Member

@sagudev sagudev commented Jul 1, 2025

This PR adds Prepare new release workflow which needs to be manually dispatched (available only to contributors) to create new release PR, that one can edit/change before merging.
Then there is also Publish release workflows, which is triggered by creating a tag (we should restrict those using ruleset to release managers) and publishes release to crates.io and creates GitHub release from tag.

Based on https://release-plz.ieni.dev/docs/github/quickstart.

Needs admin to follow instructions on https://release-plz.ieni.dev/docs/github/quickstart#1-change-github-actions-permissions for secrets and workflow permissions.

sagudev added 2 commits July 1, 2025 07:41
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@jschwe
Copy link
Member

jschwe commented Jul 2, 2025

So to summarize the workflow is as follows, right?

  1. Someone with contributor permissions or higher triggers the "prepare new release" workflow, which creates a PR
  2. The PR is merged by someone with maintainer permissions or higher (after potential manual edits)
  3. The release to crates.io is triggered by someone creating a tag (after merging), which we (well, an admin) can restrict to a new role "release manager“ by adding an appropriate group + ruleset.

command: release
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a comment here, clarifying the permissions the token should have? I'm thinking we will probably be copying this code to more repos in the future, so it could be helpful to specify the requirement here.
In my opinion that would be:

  • the token should only allow updates, not publishing new crates or yanking.
  • Ideally it would be scoped to the crate, although that wouldn't work with workspaces.

Copy link
Member Author

Choose a reason for hiding this comment

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

We might want publish if we introduce new crates. release plz suggest: https://release-plz.ieni.dev/docs/github/quickstart#2-set-the-cargo_registry_token-secret

Copy link
Member

Choose a reason for hiding this comment

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

Publishing new crates shouldn't happen very often, so I feel like it would be fine to have a human due to that when necessary.
on the other hand, it doesn't matter too much from a security perspective, since permissions to update a crate (that is potentially used by many others) is more valuable than permissions to publish a new crate (which would be unused by default).
I guess in any case we would need to make sure that we monitor the crates.io notification emails for suspicious activity.

@sagudev
Copy link
Member Author

sagudev commented Jul 2, 2025

So to summarize the workflow is as follows, right?

1. Someone with contributor permissions or higher triggers the "prepare new release" workflow, which creates a PR

2. The PR is merged by someone with maintainer permissions or higher (after potential manual edits)

3. The release to crates.io is triggered by someone creating a tag (after merging), which we (well, an admin) can restrict to a new role "release manager“ by adding an appropriate group + ruleset.

Yes. One can also manually do step 1 (for current version we already have this: #45). So now we would only need step 3.

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev sagudev requested a review from jschwe July 2, 2025 06:37
@jschwe
Copy link
Member

jschwe commented Jul 2, 2025

LGTM, but in any case this will need a review from an admin

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