As a distribution merger, I can't merge https://github.com/mage-os/mageos-composer-root-update-plugin/pull/2

mage-os / terraform

Terraform files for managing the organization repository permissions.

4 stars 8 forks source link

As a distribution merger, I can't merge https://github.com/mage-os/mageos-composer-root-update-plugin/pull/2 #31

Closed damienwebdev closed 1 year ago

sprankhub commented 2 years ago

From https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request:

Merge a pull request into the upstream branch when work is completed. Anyone with push access to the repository can complete the merge.

Currently, no one but the tech lead (and the mage-os-ci user) has push access to a repo.

We need to give the respective merger team push access to the repository. According to the docs, "Write" access is the role with the least privileges including push access.

sprankhub commented 2 years ago

I think the following should take care of this:

https://github.com/mage-os/terraform/blob/582a53b480a8265af79fe4ed4c3c3303e43fd160/main.tf#L131-L143

I checked that you, @damienwebdev are in the distribution team and that this team has write access to the repository, so this should theoretically work. Since the PR is merged now, could you create & check a new example?

damienwebdev commented 1 year ago

Vinai commented 1 year ago

After a bit of exploration yesterday, it turns out writers are not allowed to merge, if merge restrictions are in place. This means that we have to unset push_restrictions "repositories": https://github.com/mage-os/terraform/blob/main/main.tf#L113 I don't know if an empty array is correct, or no declaration at all.

sprankhub commented 1 year ago

But wouldn't that mean that even mage-os-ci can only push through PR merges? How do we ensure that this user can simply push without a PR then?

In general, https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection#push_restrictions says push_restrictions is optional, so if we change this, we can simply remove it.

damienwebdev commented 1 year ago

@sprankhub push_restrictions limits who can push to the branch. By default, since we have branch protections (with pull requests) on, no one can directly push to the branch.

If the CI bot needs to be able to push directly to 2.4-develop we can specifically allow it to override the permissions instead of limiting pushes to just that user.

I would instead suggest: