search

mage-os / terraform

Terraform files for managing the organization repository permissions.
4 stars 8 forks source link

Allow mage-os-ci user to push to remote bypassing branch protections #85

Closed gowrizrh closed 2 months ago

gowrizrh commented 3 months ago

Currently all repositories defined in variable "repositories" have branch protections turned on so all changes are submitted via pull requests. This makes sense in a regular repository but not so much in a mono-repository setup.

The mageos-async-events-sinks is a mono repository which runs an action to split the packages and push to remote as the mage-os-ci user, thus running into the problem of not being allowed to push because of said branch restrictions.

Some possible solutions that I can think of are

  1. Create a new variable "mono-repositories" which copy the same branch protection configuration AND add push_allowances to the mage-os-ci user.
  2. Add push_allowances to the mage-os-ci user on the existing branch protection rule meaning mage-os-ci could in theory bypass branch restrictions on all repositories defined by the variable.
sprankhub commented 3 months ago

I wondered that you talk about push_allowances, because we used push_restrictions until now. Turned out we need a Terraform update, which I executed in https://github.com/mage-os/terraform/pull/86/.

Your suggestions sound good to me. I'd personally say we can go with the slightly "less secure" option 2. However, I don't feel comfortable deciding this alone. Hence, I'm dragging in the @mage-os/infrastructure team, @Vinai @damienwebdev @DavidLambauer.

sprankhub commented 3 months ago

@Vinai, @damienwebdev, @DavidLambauer, could someone share their opinion on this one?

damienwebdev commented 3 months ago

I don't have enough familiarity with this to comment.

furan917 commented 3 months ago

If you wouldn't mind a random flyby, I would strongly recommend that option 2 not be taken lightly.

It would be better to be as explicit as possible. Id suggest either by allowing for a new item within your objects or a user to repo map that is considered during branch rule/branch protection setup. This way you dont have issues aligning global changes, and your user doesnt get additional privelages in much more repos than necessary.

DavidLambauer commented 3 months ago

We can loosen the permissions, as it would also benefit me in the DevDocs repository. Permissions should be managed at the user level.

Vinai commented 3 months ago

I think keeping the setup more secure by default is the way to go.