-
Notifications
You must be signed in to change notification settings - Fork 119
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
Protected branch with PR requirement prevents release #175
Comments
Hmm that’s odd, I did not run into it myself. Does the configured GITHUB_TOKEN have write access to the repository? Do you have any custom configuration for semantic-release in your setup? |
We are facing exactly the same issue. |
@gr2m even though you might have write access for protected branches you need to pass all the required checks (like having a PR and having, 2 approvers, etc) in order to be able to merge to master unless you are admin. We don't want our bot to have such privilege so if instead of committing directly to the repo it would be nice if we could create a PR. That should fix this situation. |
I think what might be possible is that you have configured something which requires the creation of additional commits, hence my question about custom configuration |
@gr2m you are right. The one trying to commit is actually the |
I’m not sure if these features are implemented in this plugin or somewhere else? I’m not sure how where the feature would need to be implemented, but if you want to give it a go I can help review it |
@Ninerian, the situation we have which I think it's similar to yours is this one:
You will still get the issue you described before.
In the perfect world, we would be able to tell Github, please bypass this type of checking for specific non-admin users (like a The way we workaround this was by assigning the The good thing is that the GITHUB_TOKEN we generated for doing this kind of commit doesn't have any crazy permission like deleting repo or such so we should be OK. Hope this is useful to you too and helps u with |
After some research I have to admit that I had the wrong package installed. I used semantic-release/git. after changing to this plugin, the release works flawless. |
In case others come here with the same issue - I was having this problem, but fixed it by un-checking "Include administrators" in the branch protection rule. |
How were you able to fix this issue? |
@Ninerian Great that you figured out how to solve this 🎉 Can you maybe shed some light on how to combine the |
I was also able to resolve the protection issue using GIT_CREDENTIALS as my environment variable. But instead of using my password i used a Personal Access Token of an admin account with push access to the repos |
Hey @hashim-sohail 👋 Thanks for the quick reply! How do you protect these credentials against malicious PRs who read your environment variables and send them somewhere? Oh, and have you enabled branch protection? If so, did you setup the token with administrative privileges? |
PS: I'm asking since https://github.com/semantic-release/semantic-release/blob/f645547f2f30eb59179f1d22da13612ba5090f38/docs/recipes/github-actions.md#pushing-packagejson-changes-to-a-master-branch warns not to use personal access tokens. I'm a little bit puzzled here. On the one hand, the docs advise you to use a personal access token, on the other hand they tell you not to do as it imposes a security risk... |
We have a different workflow for protected branches, which run only after the PR is merged after review into these branches. The secret is only exposed in these workflows.
Yes, branch protection is active. The token is generated from an admin account. The admin accounts are privileged to push directly to our protected branches. |
I went through that link too. But there is no proper workaround right now to implement this. So had to do with a custom workaround, playing with the secrets |
Great, thanks 👍
I did some additional research, too, and GitHub Actions mention that secrets are never given to actions that run as result of a PR of a fork. So, everything should be safe 😊 Thanks for your help 👍 |
@cgadam Could you please clarify for me; by adding a |
@Duchynko the |
I'm still getting this error. This is my release config in
We have a bot user and it is responsible to build and publish. I've assigned the
|
I ran this script locally on my machine and I am able to release the next version and bump the package version automatically using Why the same configuration is not working on CI? Check my config release config and logs here release.yml
One more thing I observed is that in my tokens page, the generated token has |
hey @cgadam, how did you assign the bot as an owner? I have created a token but then the release is done under my name, that is not ideal since our team would like to keep the action done in the github-action[bot] name. I have tried the plugin and configuration as minimal as the example in the thanks in advance for any help! |
a "bot" user in this case is simply another github user account that your team uses only for automated tasks like this one. you can then create a token for that account instead of your own. that user account then just needs to have a permission level that is allowed to push to the protected branch. |
@travi We did the same as you said but still we are getting the error. |
@0xc0d3r the above detail is key. do you allow admins to push, even though the branch is restricted. the "include administrators" option would need to be unchecked and the bot user would need to have admin or higher permissions. |
I finally pieced this all together as well, and it was a chore. Let me save the next guy some time: If you are absolutely allergic to using a PAT for this, we created an org-level Github App specifically for dealing with the limitations of GITHUB_TOKEN. Follow this documentation to create a Github App. Let's call our Github App Once you have your GIthub App, configure its permissions. We gave ours a repository-level permissions -- for the purposes of this use-case, I believe Next create a Private Key. This will download a PEM file -- don't lose it. Also take note of your App's App ID. Now install it to your organization. We chose to install it to just the repo that I'm working with, but I see no reason you couldn't install it org-wide if you're okay with the security implications. Now that we have that set up, we can go to our repo and set up some secrets. I added a Next, we go to branch protection rules. I'm assuming if you're reading this, you're going to have We can finally write our workflow
And, for completeness, my .releaserc (I use the YAML format):
Hope this saves the next guy about 8hrs of time. 🤦🏼 |
That's the best answer in this thread! Works like a charm!!! 💪 |
## Purpose of the changes In order to improve the maintainability and velocity of this project, I suggested we implement an automatic release pipeline. To support this automation feature, I've set up the repository to follow the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) formatting standard. This allows any commit to be parsed programmatically, and the release type (major, minor, patch) associated with a group of new commits to be determined automatically. For more information on the different commit types and their associated releases, please check [the table listing from the configuration preset](https://github.com/insurgent-lab/conventional-changelog-preset#commit-types). Once those changes are merged to `master`: - new commits on `master` will automatically be linted (locally with a pre-commit hook) to ensure they follow the formatting standard. - commits on other branches won't be linted, instead, the PR title will be linted (removing overhead for contributors). If the PR title isn't compliant, an explanation message will automatically be posted on the PR. - any commit pushed to the `master` branch will trigger a Test & Release workflow (unless `[skip ci]` is present in the commit title) that will analyze the commits since the last version, automatically determine the necessary version bump, and handle the whole release process (updating `package.json` & `package-lock.json`, generating new `CHANGELOG.md` entries, publishing Github tag & release, then publishing the new version to npm). ## Todo before merging the PR: - [x] create a git tag for v2.3.1 (pointing to 988f7f2) - [x] only allow squash merging PRs (cf screenshot below) - go to https://github.com/kelektiv/node-cron/settings#merge-button-settings - check "Allow squash merging" and set the below dropdown to "Default to pull request title" - uncheck "Allow merge commits" & "Allow rebase merging" - [x] setup branch protection for `master` (cf screenshot below) - go to https://github.com/kelektiv/node-cron/settings/branches and create/update branch protection for `master` with (at least) the following options: - "Require a pull request before merging" - "Dismiss stale pull request approvals when new commits are pushed" - "Require status checks to pass before merging" - "Require branches to be up to date before merging" > then find and select all "lint-and-test" workflows (should be 9 of them + "Lint PR title" + "security/snyk"). - "Require conversation resolution before merging" - "Require linear history" - make sure "Do not allow bypassing the above settings" is **unchecked**, else the automatic release commits won't work - [x] setup Github PAC for the repository (required because of [limitations with the default access token when branch protection is enabled](semantic-release/github#175)) - go to https://github.com/settings/personal-access-tokens/new - set Expiration in one year (custom) - select the right resource owner (should be "kelektiv") - under "Repository" access, select "Only select repositories" and select only "node-cron" - under "Permissions", set "Contents", "Issues" and "Pull requests" to "Read and write" - the "Overview" section should look like this (cf screenshot below) - copy the generated token to a new `CI_GITHUB_TOKEN` repository secret on the Github repository - [x] setup npm token for the package - go to https://www.npmjs.com/settings/{NPM_USERNAME}/tokens - "Generate New Token" > "Granular Access Token" - set Expiration to "365 days" - set Permissions to "Read and write", then click on "Only select packages and scopes" and select only "cron" - copy the generated token to a new `NPM_TOKEN` repository secret on the Github repository ## Screenshots <details> <summary>disable merge commits in PR, only allow squash merging</summary> ![image](https://github.com/kelektiv/node-cron/assets/11234273/96e8eb3e-125a-4ce5-9a45-8289a0cdfc19) </details> <details> <summary>setup branch protection for `master` *(required checks are not exactly the same in the screenshot)*</summary> ![image](https://github.com/kelektiv/node-cron/assets/11234273/bdb712b0-9a62-4391-a85b-6ee762797232) </details> <details> <summary>Github PAC > the "Overview" section should look like this</summary> ![image](https://github.com/kelektiv/node-cron/assets/11234273/0fb4b9ef-8674-42c2-af4a-bf89776239a4) </details> ## Notes - The "Squash and Merge" button should be the only one available when trying to merge this PR.
This is a very important and not really obvious information! Its a pity that this is not written down somewhere in the semantic-release docs. Apart from that I additionally had to add my admin account on GitHub to the list of the actors allowed to bypass the branch protection. I don't really understand why, because in that box there is a small hint, telling that admin accounts are always allowed to bypass the pull request requirement. Thus I think it should not be necessary to add the account to that list. |
## Purpose of the changes In order to improve the maintainability and velocity of this project, I suggested we implement an automatic release pipeline. To support this automation feature, I've set up the repository to follow the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) formatting standard. This allows any commit to be parsed programmatically, and the release type (major, minor, patch) associated with a group of new commits to be determined automatically. For more information on the different commit types and their associated releases, please check [the table listing from the configuration preset](https://github.com/insurgent-lab/conventional-changelog-preset#commit-types). Once those changes are merged to `master`: - new commits on `master` will automatically be linted (locally with a pre-commit hook) to ensure they follow the formatting standard. - commits on other branches won't be linted, instead, the PR title will be linted (removing overhead for contributors). If the PR title isn't compliant, an explanation message will automatically be posted on the PR. - any commit pushed to the `master` branch will trigger a Test & Release workflow (unless `[skip ci]` is present in the commit title) that will analyze the commits since the last version, automatically determine the necessary version bump, and handle the whole release process (updating `package.json` & `package-lock.json`, generating new `CHANGELOG.md` entries, publishing Github tag & release, then publishing the new version to npm). ## Todo before merging the PR: - [x] create a git tag for v2.3.1 (pointing to 988f7f24e3dde2e8bfe72890fdcc9a899b49f9b5) - [x] only allow squash merging PRs (cf screenshot below) - go to https://github.com/kelektiv/node-cron/settings#merge-button-settings - check "Allow squash merging" and set the below dropdown to "Default to pull request title" - uncheck "Allow merge commits" & "Allow rebase merging" - [x] setup branch protection for `master` (cf screenshot below) - go to https://github.com/kelektiv/node-cron/settings/branches and create/update branch protection for `master` with (at least) the following options: - "Require a pull request before merging" - "Dismiss stale pull request approvals when new commits are pushed" - "Require status checks to pass before merging" - "Require branches to be up to date before merging" > then find and select all "lint-and-test" workflows (should be 9 of them + "Lint PR title" + "security/snyk"). - "Require conversation resolution before merging" - "Require linear history" - make sure "Do not allow bypassing the above settings" is **unchecked**, else the automatic release commits won't work - [x] setup Github PAC for the repository (required because of [limitations with the default access token when branch protection is enabled](semantic-release/github#175)) - go to https://github.com/settings/personal-access-tokens/new - set Expiration in one year (custom) - select the right resource owner (should be "kelektiv") - under "Repository" access, select "Only select repositories" and select only "node-cron" - under "Permissions", set "Contents", "Issues" and "Pull requests" to "Read and write" - the "Overview" section should look like this (cf screenshot below) - copy the generated token to a new `CI_GITHUB_TOKEN` repository secret on the Github repository - [x] setup npm token for the package - go to https://www.npmjs.com/settings/{NPM_USERNAME}/tokens - "Generate New Token" > "Granular Access Token" - set Expiration to "365 days" - set Permissions to "Read and write", then click on "Only select packages and scopes" and select only "cron" - copy the generated token to a new `NPM_TOKEN` repository secret on the Github repository ## Screenshots <details> <summary>disable merge commits in PR, only allow squash merging</summary> ![image](https://github.com/kelektiv/node-cron/assets/11234273/96e8eb3e-125a-4ce5-9a45-8289a0cdfc19) </details> <details> <summary>setup branch protection for `master` *(required checks are not exactly the same in the screenshot)*</summary> ![image](https://github.com/kelektiv/node-cron/assets/11234273/bdb712b0-9a62-4391-a85b-6ee762797232) </details> <details> <summary>Github PAC > the "Overview" section should look like this</summary> ![image](https://github.com/kelektiv/node-cron/assets/11234273/0fb4b9ef-8674-42c2-af4a-bf89776239a4) </details> ## Notes - The "Squash and Merge" button should be the only one available when trying to merge this PR.
I had to add the Issues permission for my app, otherwise the @semantic-release/github plugin fails to search issues. Hope this helps others. |
You can use the
|
I would start by asking if you truly need to make a commit as part of your release process. If you choose not to make a commit, the need to push a commit no longer exists. There is a reason we do not include the git plugin in core. |
@travi Is the recommended approach to create a commit as a part of pull request before being merged? |
To cover the most common reasons folks choose to make commits during a release, the recommended approach is to use GitHub releases (or similar) for release notes rather than updating a changelog file and rely on the registry when determining the latest version rather than updating the version in the package.json file. Avoid making a commit altogether for those sorts of details. Not recommending making the updates manually. If you decide you still want those changes as part of a release, let semantic-release automate them for you, but know that having a commit adds significant complexity that you could avoid. |
Update the token used by Semantic release to allow bypassing branch protections rules. See [this discussion](semantic-release/github#175 (comment)) for more details.
Change the token used by Semantic release to allow bypassing branch protection rules. See [this discussion](semantic-release/github#175 (comment)) for more details.
Change the token used by Semantic release to allow bypassing branch protection rules. See [this discussion](semantic-release/github#175 (comment)) for more details.
Change the token used by Semantic release to allow bypassing branch protection rules. See [this discussion](semantic-release/github#175 (comment)) for more details.
Change the token used by Semantic release to allow bypassing branch protection rules. See [this discussion](semantic-release/github#175 (comment)) for more details.
Change the token used by Semantic release to allow bypassing branch protection rules. See [this discussion](semantic-release/github#175 (comment)) for more details.
- name: 👾 Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
persist-credentials: false |
Hello we integrated semantic release in our tool chain, but as we tried to release to master the first time, the branch protection of master prevented the release.
The error message from travis:
How could I keep the PR rule and enable semantic release?
The text was updated successfully, but these errors were encountered: