Dex: Determine criteria for repo write access

Created on 1 Sep 2018  路  19Comments  路  Source: dexidp/dex

We had quite a few people volunteer in https://github.com/dexidp/dex/issues/1273, but we ideally want to have some pre-conditions. Helping merge PRs, fixing issues, etc.

Open to ideas and extra points for something written down.

Most helpful comment

If a person or their company uses dex, has demonstrate an understanding of this project (either by submitting PRs to dex or related projects such as Helm charts), and the ability to work productively with the community, that person can have write access to this repo. We want to be liberal with this privilege and enable companies using dex to have a voice in its development.

The first 10 PRs by new maintainers must be approved by a maintainer from a different company.

Access to quay.io/dexidp will be restricted to @srenatus, @rithujohn191 and I to prevent new maintainers from being able to unilaterally push images.

If you would like access, please email me at [email protected] stating your case or open a public issue. Come join the fun 馃槂

All 19 comments

Maybe we could follow some of the kubernetes flow and allow certain people to be reviewers (happy to volunteer) and people who have more history with the project + 1 or 2 Dex / coreos sponsers to agree for approvers. Allow reviewers to become approvers after 3 months of solid work.

@jimangel This sounds like a solid choice -- do you know if that approach documented in some place?

Thinking more about this, and trying not to turn a molehill into a mountain, there are 2 other things I would add:

  1. Since the k8s flow is more for community org membership and less about repo governance, we could simplify the process by having the user open a PR (vs. emailing a group) against the OWNERS file with the exact permission they're requesting (to be reviewed and approved by 2 people). I'm not against dexidp becoming an org / having members, but I don't consider it a day 1 need.

  2. Someone (cough @srenatus cough) should have the keys to the castle (ability to revert merges, remove reviewers / approvers if malicious or inactive, and cut releases). This person should have a top priority of getting 1 or 2 more people to their status as fast as possible so the community isn't relying on one admin for everything.

This approach can be clearly documented in a CONTRIBUTING file which can be referenced in the README

Side note: what is the plan for cutting releases? Should the person in the second item create a quay account and maintain that with their elevated repo access?

Thoughts?

Yeah, I suppose keeping this simple is preferable.

Please correct me if I'm wrong, but I think the obvious next steps here are

  1. finish the move

    • [ ] replace github.com/coreos/ by github.com/dexidp/ in source files and docs

    • [ ] ensure travis picks up tests in PRs (see #1280)

  2. cut a release
  3. cleanup issues and PRs
  4. (maybe?) find a channel somewhere to have do real-time communication (IRC? Matrix? a channel in the gophers' slack?)

With this in place, we can move this forward, discuss new features, release cycles, etc.

While I'm confident merging the smaller PRs, I'd appreciate someone outside of my employer's org for the bigger ones, to not drive the project into a direction that is unwanted by the community -- So, I don't know about the planned involvement (and time available) of @rithujohn191 and @ericchiang, but, to _+1_ @jimangel, we should get a few more people (from different orgs) on board quickly.

馃挱 If we want to formalize this, there's also OPA's GOVERNANCE.md that might be applicable? I'm not aware of where this stems from. There surely are over models to follow. (I don't suppose we need to settle this question to finish the move, though.)

I am happy to maintain the chart for Dex if that helps.
I am using it locally so it will get some love.
https://github.com/helm/charts/tree/master/stable/dex

(off topic, but I'm starting to use "help wanted" to indicate "lgtm, send a PR" for anyone that's looking for ways to contribute https://github.com/dexidp/dex/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22)

I am happy to maintain the chart for Dex if that helps.
I am using it locally so it will get some love.

@rendhalver Sorry I hadn't seen your message before -- Thank you for doing that, it's much appreciated! 馃槂 馃帀

I am happy to maintain the chart for Dex if that helps.
I am using it locally so it will get some love.

@rendhalver Sorry I hadn't seen your message before -- Thank you for doing that, it's much appreciated! 馃槂 馃帀

You are welcome. :)
We are using it internally and have pushed up a few patches.

@rendhalver It would perhaps be nice if this was referenced in the docs, or the top-level README? Couldn't make up my mind where the right place would be, but if you're so inclined, I'd be happy to merge that 馃槃

Sounds like a good idea.
I will run up a PR. :)

@rendhalver let me know if you need an extra body to document / test / review. I leverage dex with my own manifests but we're currently moving all deployments to helm templates :)

@jimangel if we can get you into the maintainers list and the OWNERS file that would be awesome.
I will see what we need to do to get that setup.

馃挱 I think we still haven't linked the helm chart from anywhere... I guess another way to do that could be to create a small document outlining how to use it -- this could then also link to the definition. Alternatively, just a link in the top-level README might already help discoverability... 馃

@srenatus are you opposed to the k8s example docs leveraging helm template (using the helm binary as the template generator but no need for a fully installed helm [dependent w/ tiller]) as the "dex / k8s" preferred install method? I would be happy to rewrite them.

I say helm template mainly due to the fact the community is removing tiller in helm v3 and using helm this way would align us for an easy update later this year.

@jimangel I'm not really anywhere near that community. If you say that's the better way (and since your argument regarding helm v3 is making a lot of sense), I suppose that's the right move 馃憤

@srenatus I'm a little swamped atm with k8s release team stuff, but I have added a long term TODO for me to overhaul the dex / k8s docs (and validate the helm side of the house). Looking forward to it! Feel free to ping me for status at any time!

Also, I feel like this issue was semi-hijacked... Do we have the criteria defined for repo write access? Can this issue be closed?

@jimangel thanks, I appreciate the heads-up and the offer. 馃槂

I'm afraid we've not made much progress on the issue at hand, though. 馃槄

If a person or their company uses dex, has demonstrate an understanding of this project (either by submitting PRs to dex or related projects such as Helm charts), and the ability to work productively with the community, that person can have write access to this repo. We want to be liberal with this privilege and enable companies using dex to have a voice in its development.

The first 10 PRs by new maintainers must be approved by a maintainer from a different company.

Access to quay.io/dexidp will be restricted to @srenatus, @rithujohn191 and I to prevent new maintainers from being able to unilaterally push images.

If you would like access, please email me at [email protected] stating your case or open a public issue. Come join the fun 馃槂

Was this page helpful?
0 / 5 - 0 ratings

Related issues

treilly1969 picture treilly1969  路  7Comments

srenatus picture srenatus  路  4Comments

tanmaykm picture tanmaykm  路  4Comments

simonswine picture simonswine  路  4Comments

se0wtf picture se0wtf  路  3Comments