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

RFC: new release process #20

Open
daenney opened this issue Jan 2, 2016 · 11 comments
Open

RFC: new release process #20

daenney opened this issue Jan 2, 2016 · 11 comments

Comments

@daenney
Copy link
Member

daenney commented Jan 2, 2016

RFC: New release process

While discussing what the default permissions people should get for repositories in our organisation the problem of release integrity showed up.

Current situation

Most members of our organisation are members of the collaborators team. This grants them write access to well over 30 repositories and is one of the main reasons we can move so fast. Many eyes on the same code with many people being able to push/merge code. However, having write permissions on Github equates to being able to create and push tags. These tags are then picked up by Travis which rolls them into Forge release. Herein lies a problem.

Any collaborator can now, for whatever reason, push and release a version of a module. Since we hand out these permissions freely and not based on a "you have to prove your worth and gain our trust first" principle we risk that someone with malicious intent, or majorly inebriated after a tech conference, pushes a release of a module that does severe damage to the end user's system.

When people instruct their environments to fetch modules directly from Github this is not something we can prevent. Simply put, in order to not be able to push tags we need to revoke 'write' permission for most people and only hand that out to a select few we trust. This has the undesirable side-effect that is severely slows down progress on any module. However, when people fetch modules from the Forge a certain amount of trust is placed on the fact that if it is fetched from the Forge it should not harm the system. A degree of curation and quality control is expected there which we currently cannot guarantee.

Future situation

I would like to get to a place where people still have write access to our repositories and can even push tags but that this does not automatically result in a release published on the forge. This has the following benefits:

  • People can continue working at their own speed on modules
  • We no longer need to encrypt and sync the Forge credentials into every .travis.yml
  • We can, with a high degree of confidence, know that release on the Forge are "good"

I would also like us to gain the ability to:

  • Have a full audit trail between who committed, tagged and released to Github and Forge

Proposed solution

As such we need a solution that allows us to "gate" releases to the Forge. I propose a workflow where:

  • People work and commit to their hearts intent on a module
  • People tag releases as they see fit
  • Travis uploads the release tarbal to an upload queue of our "release service"
  • Someone from the releases team then approves the upload for delivery to the Forge

This is very much like how larger FOSS projects such as Debian handle the release process of packages. Though it does create a small blocker on a team of people that have the release power, as long as this team is big enough and geographically diverse it should not cause any issues. It also automatically denotes a number of people that can be contacted should issues with a release be uncovered once it's made it out to the Forge.

This is not the place to go into implementation details of such release service. It is merely a place to discuss possible workflows.

@danzilio
Copy link
Member

danzilio commented Jan 2, 2016

we risk that someone with malicious intent, or majorly inebriated after a tech conference, pushes a release of a module that does severe damage to the end user's system

The most likely scenario is probably a user's GitHub account being compromised. Is there a way for us to require MFA for members? We should probably require MFA for releasers.

I think this proposal is sound. Is the intent to require all releases to use this workflow or just releases tagged by unauthorized releasers? That's my non-implementation detail-y way of asking: should we make the service smart enough so when a release is tagged by an approved releaser it bypasses this workflow and just pushes the release directly to the Forge?

@daenney
Copy link
Member Author

daenney commented Jan 2, 2016

The most likely scenario is probably a user's GitHub account being compromised. Is there a way for us to require MFA for members? We should probably require MFA for releasers.

I think that's possible. From what I know of the Github API it is possible to request this information so we could make that a precursor to being able to use/login to the releases service. Famous Last Words.

That's my non-implementation detail-y way of asking: should we make the service smart enough so when a release is tagged by an approved releaser it bypasses this workflow and just pushes the release directly to the Forge?

I don't think we should provide any ways to bypass it. If anything I think it would be good to require that the tagger != releaser to ensure people can't just push their own changes through if they have enough privileges to do so. This means you'll always need at least 2 co-consiprators if you want to do something malicious.

@igalic
Copy link
Contributor

igalic commented Jan 4, 2016

the way i've designed my release (proposed) process was that one person creates a pr, another merges&tags it. that's already two people. i'm not sure we can, in general, keep people from merging their own prs, other than through honesty…

@daenney
Copy link
Member Author

daenney commented Jan 4, 2016

I don't think that we can either. Github does provide status checks but they're executed prior to merge, I don't think they can stop a merge if it turns out that someone is trying to merge their own code.

The idea of this is to keep the regular process of working on code the same. Like you said, one person commits, another reviews and merges but to have an additional gate when it comes to uploading to the Forge. My hope is that that would also provide us for a centralised way to manage all our activities around the Forge and thus only one location that would need to have/store credentials to do so.

@igalic
Copy link
Contributor

igalic commented Jan 4, 2016

can you elaborate on the credentials issue, please?

@daenney
Copy link
Member Author

daenney commented Jan 4, 2016

Of course. Right now, whenever we wish to hookup a module to Travis for release we need to get the Forge password credential, decode it from GPG, travis encrypt it, add that to .travis.yml of the module and then recode the file with all the secrets in GPG and push it back up to Github.

In my proposal Travis wouldn't be the one doing the upload to the Forge, the Release Gate service would. As such, Travis nor the .travis.yml would need to know about the password and we wouldn't have to do that whole dance every time we add/transfer a module. The only entity that would need to hold on to it would the the Release Gate.

@danzilio
Copy link
Member

danzilio commented Jan 4, 2016

I believe you can set the status checks to block a merge by protecting the branch. That will prevent any PR from being merged unless its status checks are all green and it will prevent pushes directly to the branch.

@daenney
Copy link
Member Author

daenney commented Jan 4, 2016

Correct, but that's not what we're discussing here. Protecting a branch means that every commit will have to go through the PR flow except for those people that are exempt (administrators aren't exempt by default). This means that in case of an emergency or a super tiny edit like fixing a typo in a README we'd need to go through the full flow every time, which is costly in the case of a serious bug and unnecessary in the case of a typo change.

This proposal is not about restricting who can make direct edits to anything or the collaboration workflow on Github. This proposal is about what happens once the code is ready to "be distributed".

@danzilio
Copy link
Member

danzilio commented Jan 4, 2016

Hmm...is it though? We've heard this same complaint from our developers at work, but I'm not convinced the PR workflow should ever be bypassed. This seems like a topic for another thread though ;)

@daenney
Copy link
Member Author

daenney commented Jan 4, 2016

This seems like a topic for another thread though ;)

Correct. Do write up a proposal though. It would be interesting to collect everyone's thoughts on the matter.

@igalic
Copy link
Contributor

igalic commented Jan 4, 2016

i agree.
we are also breaking the pr workflow with blacksmith, though, as we commit the new version on top of master and then push that.
but: we do that with an automated tool, so i'm less flinchy about that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants