Skip to content

Add a release workflow #9

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

Merged
merged 20 commits into from
Sep 24, 2021
Merged

Add a release workflow #9

merged 20 commits into from
Sep 24, 2021

Conversation

mattwynne
Copy link
Member

@mattwynne mattwynne commented Sep 21, 2021

Adds an automated release workflow. So far this only works for publishing a Ruby gem, and will need to be extended to publish other language/platform packages.

See cucumber/common#1688

Checklist:

  • Generate a new RubyGems API token and add it as RUBYGEMS_API_KEY secret in the Release environment.
  • Generate a new NPM API token and add it as NPM_TOKEN secret in the Release environment.
  • Add Go publishing
  • Add Java publishing
  • Add JavaScript publishing
  • Re-organise things so we have a way to re-run a publish job if it failed.
  • Factor out our own publish-go action?
  • Factor out our own publish-mvn action?

@aslakhellesoy
Copy link
Contributor

aslakhellesoy commented Sep 21, 2021

Great work @mattwynne. I have many questions.

  • Where in the process is a git tag created?
  • What user is the git tag created by?
  • Will that git tag be signed?
  • Will the git tag be on a commit on main or on releases/*?
  • Do we want to be able to use the Merge Button to merge unsigned PRs?
  • Are we ok with unsigned commits on main?
  • Do we want to make sure all commits on releases/* are signed? If so, do we need to squash-merge from main?
  • The workflow uses various cucumber-actions/* - let's add some links to https://github.com/cucumber-actions in the scripts. (Can you invite me again to that org @mattwynne)
  • Where is the release version number picked up from? The branch name? If so, what happens when a 2nd commit is pushed to the same releases/* branch?

I feel that the tag should be created locally by the person triggering the release, and that the release action should only be triggered when a new tag is pushed to the releases/* branch (rather than for every commit pushed to the branch).

@aslakhellesoy
Copy link
Contributor

Another thing to consider is retries. The package managers are occasionally down or unresponsive (especially sonatype, where maven artifacts are published). If a tag is pushed, and the release action fails, we should be able to re-trigger the action manually via GitHub's web interface.

@aslakhellesoy
Copy link
Contributor

Would it make sense to have separate release-java.yml, release-ruby.yml etc workflows? That way we can manually rerun just the ones that have failed, and not the others.

@aurelien-reeves
Copy link
Contributor

@mattwynne we have made some tests here: https://github.com/cucumber/release-tests

@mattwynne
Copy link
Member Author

@mattwynne we have made some tests here: https://github.com/cucumber/release-tests

@aurelien-reeves I like the approach of splitting the release / publish workflows and using the release published event to trigger the package publishing!

Let's pair / ensemble on this on Wednesday?

@mattwynne
Copy link
Member Author

mattwynne commented Sep 21, 2021

Great work @mattwynne. I have many questions.

You do indeed! It's great to have your attention on this.

  • Where in the process is a git tag created?

Right now, the tag is created by GitHub when we create the Release here in the create-release action.

  • What user is the git tag created by?
  • Will that git tag be signed?

I think github just creates non-annotated / lightweight tags, so there's no user info / signing associated with the tagging, as far as I can tell.

  • Will the git tag be on a commit on main or on releases/*?

It's currently made on the HEAD commit where the workflow runs, so the head of the release/* branch. Ideally, I think, we'll fast-forward commits onto the release branch from main so the commit being release will be on both branches.

  • Do we want to be able to use the Merge Button to merge unsigned PRs?
  • Are we ok with unsigned commits on main?
  • Do we want to make sure all commits on releases/* are signed? If so, do we need to squash-merge from main?

I just dunno about all this. I think that for streamlining the release process we should try to push commit signing as far upstream as we can, but we obviously don't want to put too many barriers in the way of contributions either, so there's a trade-off.

I think we should insist on signed commits on the release/* branches for now, and if that means we need to manually squash merges into those branches in order to sign commits, we should live with that for a while and see how much of a pain it is.

The main pain I see with that scenario is that the commit being released (the squashed commit) is not on main and so we have a puzzle about which commit to tag - the one on main or the one that was squashed onto release/*.

Yes. The idea of factoring out these actions is so that we have a little copy/paste as possible when implementing this automation in the various cucumber repos. Actions are the lowest level of re-use you get in GitHub Actions, so the workflows themselves want to be as minimal as possible.

I've sent you another invite. Using that org is not cast in stone - we could move those actions over to the cucumber org if we want to. I just wanted to try and keep them all in one place while we developed them.

Where did you want to put the links? The workflows themselves have links in them. Did you mean link to them in the docs?

  • Where is the release version number picked up from? The branch name? If so, what happens when a 2nd commit is pushed to the same releases/* branch?

No, the branch name is not relevant at all. We just happen to use the release number as a way to keep each release branch unique, as they're basically throwaway artifacts.

The release number/tag is picked up from the top heading in the CHANGELOG, using the changelog latest command which I contributed back. I like the idea of using the changelog as the "source of truth" during the release process, and I think that CLI tool is really useful both for manual and automated tasks.

After the feedback from @davidjgoss that the original automated release process for cucumber-build was quite onerous, I did some work on an action to create a PR for each release automatically. This works pretty nicely in most cases, but I think we should keep that on the back burner until we're settled on the actual release end of the process.

I feel that the tag should be created locally by the person triggering the release, and that the release action should only be triggered when a new tag is pushed to the releases/* branch (rather than for every commit pushed to the branch).

I had the same instinct initially. The trouble with doing this is that we have to find another way to signal that the release is "done". So far, I've been using the CHANGELOG as the signal to request a new release, and the tag as the signal of what has been released. This enables us to know when a release is needed which is what enables the pre-release PR workflow.

@mattwynne
Copy link
Member Author

I've added an NPM publish action and the secret for it, so Ruby and NPM are good to go.

- uses: actions/setup-node@v2
with:
node-version: '16'
- uses: cucumber-actions/[email protected]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use https://github.com/marketplace/actions/npm-publish instead? Less stuff for us to maintain?

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea would be to not use external actions to be able to check a security option to github, thus to have total control over the release pipeline

Eventually we could fork the action

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair point. I would prefer to fork a community one rather than maintaining ones we've written from scratch. It has a lower cost of ownership - we can merge in upstream bugfixes as needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree
But we should be able to understand the code and what it does

(yes, I may be a little bit paranoid 😅)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hacking a widely used release plugin could provide attackers with an attack-surface where they can distribute malicious code to a lot of people. It's good to be paranoid about this.

These actions are fairly small and I think the effort required to understand them is smaller than the effort required to write them.

Copy link
Member Author

@mattwynne mattwynne Sep 22, 2021

Choose a reason for hiding this comment

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

I think that this 3rd party one is way over complex for what we need. It has many features we don't need, where ours is about fifteen lines of bash. i like ours better.

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 totally agree about the paranoia though!

with:
ruby-version: 3.0.2
bundler-cache: true
- uses: cucumber-actions/[email protected]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use https://github.com/marketplace/actions/publish-to-rubygems instead? Less stuff for us to maintain?

Copy link
Member Author

Choose a reason for hiding this comment

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

@aurelien-reeves pointed out that including 3rd party actions in our release chain is a potential supply-chain attack vector. For simple, stable, stuff like this I think it makes sense to roll our own.

I reviewed that particular one before deciding to write our own, and it's lame. It doesn't actually do the gem publish, it just invokes a rake task. It also writes the token to disk, and needs a github token for a reason I don't understand.

So I like ours better.

@aslakhellesoy
Copy link
Contributor

Thanks for the explanations @mattwynne - makes a lot more sense to me now.

For Maven publishing we can try https://github.com/marketplace/actions/action-maven-publish (there are more here)

For Go releases we just need to create and push a go/v$version git tag.

If one of the publish actions fail, I don't think we have an automated way to retry. That worries me.

@aslakhellesoy
Copy link
Contributor

The version numbers in the various package decriptors (and go code) will have to be bumped before we start the release process. We should figure out how to automate this. Maybe we can reuse some of the scripts from common.

@aslakhellesoy
Copy link
Contributor

We need to add several secrets to the Release environment. We should document how to do this, and where to find the secrets. I assume we can get most of them from the secrets repo, but the NPM_TOKEN is not in that repo.

@mattwynne I assume you added a new one in https://www.npmjs.com/settings/cukebot/tokens - but once added they cannot be read. If we want a token per repo, that's fine, but then the tokens must be named. Right now there are way too many tokens in there - we should delete as many as possible and start fresh.

@aslakhellesoy
Copy link
Contributor

We need to merge cucumber/action-create-github-release#6 and make a new release of that action before we can merge this PR. Then I think we can test the release process.

aslakhellesoy and others added 2 commits September 22, 2021 16:30
Co-authored-by: Aurélien Reeves <[email protected]>
Co-authored-by: Aurélien Reeves <[email protected]>
@mattwynne
Copy link
Member Author

We need to add several secrets to the Release environment. We should document how to do this, and where to find the secrets. I assume we can get most of them from the secrets repo, but the NPM_TOKEN is not in that repo.

Yes, we need to document how to do it. Where would be a good place do you think?

No, they've all been generated fresh. I think this is good practice.

@mattwynne I assume you added a new one in https://www.npmjs.com/settings/cukebot/tokens - but once added they cannot be read. If we want a token per repo, that's fine, but then the tokens must be named. Right now there are way too many tokens in there - we should delete as many as possible and start fresh.

As far as I can tell there's no facility for naming tokens in npmjs.com 🤷 FWIW the one I generated for this repo is the one starting 1604. The other recent one (starting 95d2) is used by the tests for the https://github.com/cucumber-actions/publish-npm/ action.

I have no idea what the others are for, or why there are so many. I would assume that once we've finished automating all the releases of our javascript packages we can just delete all the older ones.

needs: create-release
runs-on: ubuntu-latest
steps:
- name: Create go/* tag
Copy link
Member Author

Choose a reason for hiding this comment

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

Even though it's trivial, I suggest we factor this out into an action too. That way we keep symmetry with the other publish jobs, and we also have a single lever we can move if we need to make cross-project modifications about how Go packages are released (such as publishing binaries etc.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, but I'd prefer to extract it after we're sure it works. It's easier to iterate that way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, but I'd prefer to extract it after we're sure it works. It's easier to iterate that way.

Mmmyeah. Remember how hard these are to test though - really the only way to test it is to run it and make a release. If we have it in a separate action we can unit test that at least, which gives us a bit more confidence when we bolt the whole thing together.

I've run out of time today but I can do this tomorrow.

@mattwynne
Copy link
Member Author

If one of the publish actions fail, I don't think we have an automated way to retry. That worries me.

Do you mean you want to automatically retry if the publish fails? Or you want to be able to manually re-run the automated release workflow? I expect you mean the latter.

I think we have a couple of approaches we could take:

  1. Make sure that the publish jobs are idempotent, so we can just re-run the whole workflow if one of them fails.
  2. Split the different platform publish jobs into different workflows, so we can easily re-run individual ones.

I have a slight preference for (1) since making them idempotent feels sensible anyway in case something gets triggered unintentionally, but (2) is probably a quicker win. I suggest we do that for now.

I liked what you did in https://github.com/cucumber/release-tests to chain the actual publish jobs off of the release published event. Maybe we could do that?

run: |-
git config user.name "cukebot"
git config user.email "[email protected]"
git tag "go/${{ steps.create-release.outputs.version }}"
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 could also use the https://github.com/cucumber-actions/versions action as a prior step in this workflow.

Or, if we factor out our own action for this we can make it self-sufficient about looking up the version.

@aslakhellesoy
Copy link
Contributor

If one of the publish actions fail, I don't think we have an automated way to retry. That worries me.

Do you mean you want to automatically retry if the publish fails? Or you want to be able to manually re-run the automated release workflow? I expect you mean the latter.

I meant manually retrying a publish for just a single language.

I think we have a couple of approaches we could take:

1. Make sure that the publish jobs are idempotent, so we can just re-run the whole workflow if one of them fails.

The whole workflow also includes create-release, so that would have to be idempotent too. If we want to make them idempotent and still error when something is wrong is to check if the release has already been made and then do nothing. That means querying github releases, rubygems, npm, nexus/sonatype (maven) etc.

They are hopefully all easy to query, but I'm worried about nexus/sonatype - not sure if it's possible there.

2. Split the different platform publish jobs into different workflows, so we can easily re-run individual ones.

This would be my preference. @aurelien-reeves and I experimented with this yesterday in https://github.com/cucumber/release-tests/tree/main/.github/workflows (a throwaway repo).

We tried to make the publish-* workflows trigger on the on.release event, but couldn't make it work. I'm not even sure we can make it work - we still want to make sure the job only runs in the Release environment, and I don't see how that could be activated unless the event is a on.push to a release/* branch.

I have a slight preference for (1) since making them idempotent feels sensible anyway in case something gets triggered unintentionally, but (2) is probably a quicker win. I suggest we do that for now.

I liked what you did in https://github.com/cucumber/release-tests to chain the actual publish jobs off of the release published event. Maybe we could do that?

Not sure - see comment above.

@mattwynne
Copy link
Member Author

I'm not even sure we can make it work - we still want to make sure the job only runs in the Release environment, and I don't see how that could be activated unless the event is a on.push to a release/* branch.

Ah yes. We don't have enough security around Releases - they could be created manually by anyone with the commit bit.

So we need to stick to triggering these when there's a push to the protected branch.

@mattwynne
Copy link
Member Author

mattwynne commented Sep 22, 2021

The whole workflow also includes create-release, so that would have to be idempotent too. If we want to make them idempotent and still error when something is wrong is to check if the release has already been made and then do nothing. That means querying github releases, rubygems, npm, nexus/sonatype (maven) etc.

I've actually done this already in the tests for the rubygems and npm publish actions. When I was testing the NPM one the other day I noticed it won't let you push the same package name / version a second time, so it would just fail if you tried to run it again.

They are hopefully all easy to query, but I'm worried about nexus/sonatype - not sure if it's possible there.

Yeah that's a risk until we've done it.

curl https://search.maven.org/solrsearch/select\?q\=cucumber\&wt\=json looks pretty good (from here)?

@mattwynne
Copy link
Member Author

I liked what you did in https://github.com/cucumber/release-tests to chain the actual publish jobs off of the release published event. Maybe we could do that?

Not sure - see comment above.

I reckon we can have several separate workflows all going off the push event on the release/* branches, which should give us the best of both worlds.

@mattwynne mattwynne dismissed aurelien-reeves’s stale review September 24, 2021 01:05

I think we've implemented the suggestions.

@mattwynne
Copy link
Member Author

I think this is good to go, eh? We can easily / safely factor out a publish-go action later.

@aurelien-reeves
Copy link
Contributor

I guess you have heavily tested the releases workflows as part of https://github.com/cucumber/release-tests?

If so, it looks good to me :)

@aslakhellesoy aslakhellesoy merged commit 1ba7f9e into main Sep 24, 2021
@aslakhellesoy aslakhellesoy deleted the add-release-workflow branch September 24, 2021 09:06
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.

3 participants