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

Add action to create a module release PR #66

Draft
wants to merge 1 commit into
base: v3
Choose a base branch
from
Draft

Add action to create a module release PR #66

wants to merge 1 commit into from

Conversation

bastelfreak
Copy link
Member

No description provided.

.github/workflows/prepare_release.yml Outdated Show resolved Hide resolved
.github/workflows/prepare_release.yml Outdated Show resolved Hide resolved
.github/workflows/prepare_release.yml Outdated Show resolved Hide resolved
.github/workflows/prepare_release.yml Outdated Show resolved Hide resolved
.github/workflows/prepare_release.yml Show resolved Hide resolved
inputs:
version:
description: 'Module version to be released.'
required: true
Copy link
Member

Choose a reason for hiding this comment

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

I would love it if we had some logic to automatically detect this based on labels, making it optional but when I looked at it I had trouble coming up with a good solution so this is a good first step.

Copy link
Member Author

Choose a reason for hiding this comment

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

dito. I would also love to do this automatically. but one step at a time.

@bastelfreak bastelfreak force-pushed the prep branch 2 times, most recently from eb2e843 to 672a436 Compare December 19, 2024 16:29
.github/workflows/prepare_release.yml Outdated Show resolved Hide resolved
.github/workflows/prepare_release.yml Outdated Show resolved Hide resolved
@bastelfreak bastelfreak force-pushed the prep branch 2 times, most recently from db2ad4d to 09a0cb5 Compare December 19, 2024 17:01
runs-on: ubuntu-24.04
steps:
- name: Checkout repository
uses: actions/checkout@v4
Copy link
Member

Choose a reason for hiding this comment

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

With my yellow Foreman hat on: we maintain stable branches as well and if we need to create multiple release then it would be nice if there was a parameter for the base branch. Then you should also take that into account for the target branch name

Copy link
Member Author

Choose a reason for hiding this comment

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

updated it. it defaults to master now, but you can provide a different branch name as base for the release_prep branch / for the PR.

Copy link
Member

Choose a reason for hiding this comment

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

If you don't specify any default, it should just work regardless of the default branch.

run: |
git switch --create release_prep
git commit --all --message="Release ${{ steps.get_version.outputs.version }}"
gh pr create --title "Release ${{ steps.get_version.outputs.version }}" --label skip-changelog
Copy link
Member

Choose a reason for hiding this comment

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

How does gh pr create work if a PR already exists? Does it at least push to the existing branch or just error out? You know how common it's to create a prep, fix labels and then regenerate it.

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 don't know yet. That's why I raised voxpupuli/puppet-example#81 , to test it.

Copy link
Member Author

Choose a reason for hiding this comment

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

when running it on code where a PR already exists:

$ gh pr create
a pull request for branch "prep" into branch "v3" already exists:
https://github.com/voxpupuli/gha-puppet/pull/66

bundler-cache: true
working-directory: ${{ inputs.working-directory }}
- name: Update metadata.json to new version
run: bundle exec rake module:bump_to_version[${{ inputs.version }}]
Copy link
Member

Choose a reason for hiding this comment

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

https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections recommends setting it to an environment variable instead of using shell interpolation. While the risk may be small here, I think it's still a good idea.

Perhaps we can even modify puppet-blacksmith to understand an environment variable. I'd be tempted to modify module:bump to support BLACKSMITH_NEXT_VERSION and otherwise simply bump a patch level. That would be around here: https://github.com/voxpupuli/puppet-blacksmith/blob/760c035c6b87861ca767358d96178f8ebdd6ec13/lib/puppet_blacksmith/rake_tasks.rb#L76-L88

And if you take that logic a step further, you can make the input version optional then. The workflow becomes:

  • Call release prep without arguments
  • Inspect the generated changelog
  • Fix any labels
  • Call the release workflow with the correct version number
  • Merge PR
  • Create tag

As a follow up: you could create a workflow that if the release_prep branch is merged that you create a tag automatically.

Copy link
Member

Choose a reason for hiding this comment

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

Turns out, there's already module:bump:full which uses BLACKSMITH_FULL_VERSION

Suggested change
run: bundle exec rake module:bump_to_version[${{ inputs.version }}]
run: bundle exec rake module:bump:full

Copy link
Member Author

Choose a reason for hiding this comment

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

updated it

Co-authored-by: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Comment on lines +54 to +66
- name: 'Check if a release is necessary'
id: 'check'
run: |
git diff --quiet CHANGELOG.md && echo "release=false" >> $GITHUB_OUTPUT || echo "release=true" >> $GITHUB_OUTPUT
- name: 'Commit changes'
if: ${{ steps.check.outputs.release == 'true' }}
env:
# https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
git switch --create release_prep
git commit --all --message="Release ${{ steps.get_version.outputs.version }}"
gh pr create --title "Release ${{ steps.get_version.outputs.version }}" --label skip-changelog --base ${{ inputs.base-branch }} --fill
Copy link
Member

Choose a reason for hiding this comment

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

I can only recommend using peter-evans/create-pull-request instead of wrangling all those steps manually.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok ok!

if: ${{ steps.check.outputs.release == 'true' }}
env:
# https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

One interesting side note: if that's the normal token GH generates on the fly when the action runs, the created PR will not trigger any further GH actions (like CI).

Reads docs https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#using-the-github_token-in-a-workflow

Oh, this is not the case for workflow dispatch, TIL

@ekohl ekohl added the enhancement New feature or request label Dec 19, 2024
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 this pull request may close these issues.

3 participants