Page MenuHomePhorge

Decide who has admin/commit/security access
Closed, ResolvedPublic

Description

We have a few advanced user roles:

Who gets to be in them?

Event Timeline

taavi created this object in space S1 Public.

As of right now, both Blessed Committers and Security Viewers have the admins as members. I have no problem with that changing.

Blessed Committers was given on the old upstream by having a track record of positive contributions. So maybe something like "Has a track record of landed diffential revisions and an application to the core team."

I would think at least Security Viewers would require an application of some kind, to be approved by the core team.

So some feedback from my experience doing this in the open.

There is great benefit from having people submitting the code to push it themselves. This just keeps people involved because they have one last step to be able to consider their thing done. It gives them ownership, and this changes people's behavior.

This is fairly easy to do in a company, but how does one do this in the open? People end up being able to push anything in the repo, and you don't want that. Here is how we ended up solving this:

  • There is a set of trusted reviewers that need to accept the code before it is possible to push. This can be enforced with a combination of the owner feature and and herald rule.
  • arc land is provided a custom workflow, where it actually doesn't push the code, but hit a endpoint where a bot will arc patch, rebase, run smoke tests and if it all goes well, push the changes - or repport in the diff if it didn't. We call that landbot (I know, not very original, but at least it's clear what it is). gitlab call this merge train, which is kind of a fun name.

This process turns out to be beneficial in general, so that when code becomes fubared after a rebase is not merged for instance. Avoid human errors as well.

It is important to keep the option to trigger the old land workflow, using something like --bypass-landbot, in which case you must be one of the owner. The need for this is obvious: if landbot is fubared, people need to be able to continue to work.

All in all, this ends up being a workflow that has been immensely beneficial in various place I've worked for the past decade, and the only thing I'd have to complain about is that it is not well integrated into phabricator as it is and would benefit from some love.

There is great benefit from having people submitting the code to push it themselves.

Totally agreed! I 100% hope that's the model we move towards. Used Herald to enforce a "Commits must have a Revision approved by #blessed_reviewers" rule in prior companies as well, worked like a charm and kep things fairly open

Gets a little murkier when we get into instance admin, folks who receive security vuln reports, and folks with admin to the underlying infra. Probably will be something to sort out after we come up with a proper 'core team' and a technical charter for the project. All just super ad hoc right now!

On the topic of increasing community involvement we will also want to produce documentation for setting up development environments as well as the steps to submit changes upstream (like a quality checklist). To make development environments even easier we might want to consider supporting something like a vagrantfile so people can get started with very few steps.

Matthew changed the visibility from "All Users" to "Public (No Login Required)".Jun 11 2021, 23:52

Since we're planning to eventually host more-open/accessible community repositories I created these as separate object rules instead of as a global rule

Currently rP and rARC only allow Blessed Committers to push - with those herald rules in place should we open that up? Maybe we should have a separate group "Contributors" for anyone who wants to submit revisions for approval? Or should it be opened to any user?

In T15004#425, @speck wrote:

Currently rP and rARC only allow Blessed Committers to push - with those herald rules in place should we open that up?

Small thing but we should switch from a Herald rule (H7) to using the owners application.

Maybe we should have a separate group "Contributors" for anyone who wants to submit revisions for approval? Or should it be opened to any user?

If possible we should open to all users but this may rely on having good anti-spam protections

hi, i'm the owner of a little sw Company based in Italy. I based it on phab and i would like to contribute to Phorge also with my employees.
If someone of admins is interested please contact me via email or in conpherence
Thank you everybody for your work, i hope the Phorge has a great future ahead

In T15004#100, @speck wrote:

On the topic of increasing community involvement we will also want to produce documentation for setting up development environments as well as the steps to submit changes upstream (like a quality checklist). To make development environments even easier we might want to consider supporting something like a vagrantfile so people can get started with very few steps.

We have a document that results in a working dev environment for Wikimedia's phabricator fork: https://www.mediawiki.org/wiki/Phabricator/Local_Dev_Environment I've been intending to simplify it and perhaps build a version based on docker / docker-compose.

T15011 discusses some of this...

@willson556 built a container that works awesome - Just a few small things to work out - https://github.com/willson556/phorge-devcontainer

This container was easy enough for me to get up and running in like 4 steps (see the README)

I think once some of those issues are resolved, we should host the source for this container here at we.phorge.it

For the development environment this would probably be best in a diviner book/document instead of a wiki.

T15011 discusses some of this...

@willson556 built a container that works awesome - Just a few small things to work out - https://github.com/willson556/phorge-devcontainer

This container was easy enough for me to get up and running in like 4 steps (see the README)

I think once some of those issues are resolved, we should host the source for this container here at we.phorge.it

I hope to address all of the feedback by the end of the week! And yeah, I just put it up on my GitHub temporarily, agreed it belongs on this instance.

avivey triaged this task as Unbreak Now! priority.Jun 23 2021, 18:27

This is my understanding of the items in the description currently, please indicate if this is not correct

Security - I think this may have originally been intended for tagging items which are related to security issues that need addressed, such as vulnerabilities in the project. I think this is a tag that anyone could use when submitting issues.
Security Viewers - This is used to wall off items that should be restricted from public viewing, namely security reports, putting things into {S2}
Blessed Committers - The group of people who can push changes to the upstream

Currently {S2} is also being used to wall off the infrastructure details of this server. Would it make sense to have another project/group, like "Blessed Infrastructure" for the team members who are entrusted to have access to the server and infrastructure details?

From an organizational perspective I think it would make sense to have another group of the "Core Phorge Team" members who are helping to organize this project. If anything I think it will help to field questions/assistance, and I think in the future the list of people in Blessed Committers will likely grow beyond the core team (I would hope so at least).

As for membership into the core team and blessed committers, in addition to signing some form of CLA for code changes, I think we should have something along the lines of

  1. To bootstrap, initial core team member candidates should apply (like, a comment here?) and have some initial standing credentials with Phabricator. Whether this is from being a community member of Phabricator or having ~2 other community members who are familiar with the community vouch for them.
  2. Future core team members could be vouched for by others, and/or promoted from having standing credentials with Phorge.
  3. All core team members should agree to uphold the vision statement for the project, maybe this could also be done via Legalpad application. As the vision statement changes should probably require re-agreeing.
In T15004#650, @speck wrote:

This is my understanding of the items in the description currently, please indicate if this is not correct

Security - I think this may have originally been intended for tagging items which are related to security issues that need addressed, such as vulnerabilities in the project. I think this is a tag that anyone could use when submitting issues.
Security Viewers - This is used to wall off items that should be restricted from public viewing, namely security reports, putting things into {S2}
Blessed Committers - The group of people who can push changes to the upstream

That was the intention when I created the groups.

Currently {S2} is also being used to wall off the infrastructure details of this server. Would it make sense to have another project/group, like "Blessed Infrastructure" for the team members who are entrusted to have access to the server and infrastructure details?

That would be very easy to do, would there be any objections to me making that change?

From an organizational perspective I think it would make sense to have another group of the "Core Phorge Team" members who are helping to organize this project. If anything I think it will help to field questions/assistance, and I think in the future the list of people in Blessed Committers will likely grow beyond the core team (I would hope so at least).

That makes sense. I personally see Blessed Committers only requiring a track record of good diffs, as it will just give the ability to land revisions. Right now, anyone can make a diff if they'd like, which I believe aligns well with the open-source philosophy.

As for membership into the core team and blessed committers, in addition to signing some form of CLA for code changes, I think we should have something along the lines of

  1. To bootstrap, initial core team member candidates should apply (like, a comment here?) and have some initial standing credentials with Phabricator. Whether this is from being a community member of Phabricator or having ~2 other community members who are familiar with the community vouch for them.
  2. Future core team members could be vouched for by others, and/or promoted from having standing credentials with Phorge.
  3. All core team members should agree to uphold the vision statement for the project, maybe this could also be done via Legalpad application. As the vision statement changes should probably require re-agreeing.

I like this proposal.

I would say let’s go ahead and make those changes. I’ll be on later tonight from a workstation and can make those changes then (~8hrs) if needed.

In T15004#672, @speck wrote:

I would say let’s go ahead and make those changes. I’ll be on later tonight from a workstation and can make those changes then (~8hrs) if needed.

Done! Project is Blessed Roots, space is S3. I added all admins.

This comment was removed by eax.

I like the proposal above. Especially with having the core team "sign" a vision statement. The goal is less legalistic and more of ensuring we have a consistent view of the end product.

Wouldn't this be a subtask of Legal? If admin has access to shell and/or PII, then this would be something for legal stuff.

Nighters383 renamed this task from Decide who has admin/commit/security access to Nightster383Decide who has admin/commit/security access.Oct 30 2021, 15:57
Nighters383 claimed this task.
Nighters383 lowered the priority of this task from Unbreak Now! to High.
Nighters383 changed the edit policy from "All Users" to "Nighters383 (Charles Minnich)".
Nighters383 edited projects, added Auth (archived); removed Phorge, Governance.
chris renamed this task from Nightster383Decide who has admin/commit/security access to Decide who has admin/commit/security access.Oct 30 2021, 16:20
chris removed Nighters383 as the assignee of this task.
chris changed the edit policy from "chris (Christopher Wetherill)" to "All Users".
chris edited projects, added Phorge, Governance; removed Auth (archived).
chris added a subscriber: Nighters383.
chris changed the edit policy from "All Users" to "Trusted Contributors (Project)".Oct 30 2021, 16:26
chris removed a subscriber: Nighters383.
avivey claimed this task.

We've sort of reached a status-quo:

  • Trusted Contributors is basically "community members" - with a low bar for entry
  • Blessed Committers is the next level up, and can basically push whatever code they want
  • admin/root/security is basically "core team".