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

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

Closed
gowrizrh opened this issue May 14, 2024 · 6 comments · Fixed by #87
Closed

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

gowrizrh opened this issue May 14, 2024 · 6 comments · Fixed by #87

Comments

@gowrizrh
Copy link
Member

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
Copy link
Contributor

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 #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
Copy link
Contributor

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

@damienwebdev
Copy link
Member

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

@furan917
Copy link

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
Copy link
Contributor

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

@Vinai
Copy link
Contributor

Vinai commented Jun 4, 2024

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

sprankhub added a commit to sprankhub/terraform that referenced this issue Jun 9, 2024
@sprankhub sprankhub linked a pull request Jun 9, 2024 that will close this issue
sprankhub added a commit to sprankhub/terraform that referenced this issue Jun 13, 2024
sprankhub added a commit that referenced this issue Jun 13, 2024
Add mono repo push allowances, fixes #85
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 a pull request may close this issue.

6 participants