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

Update repo permissions on k/enhancements #590

Closed
justaugustus opened this issue Jul 17, 2018 · 32 comments
Closed

Update repo permissions on k/enhancements #590

justaugustus opened this issue Jul 17, 2018 · 32 comments
Assignees
Labels
area/enhancements Issues or PRs related to the Enhancements subproject kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@justaugustus
Copy link
Member

justaugustus commented Jul 17, 2018

Feature Enhancement maintainers often use GitHub settings like Milestone to determine which Features Enhancements require triage.

It's important that these settings are only touched by Feature Enhancements Maintainers to ensure better signal during the release process.

As such, I'd like to do the following:

I suggest kubernetes-milestone-maintainers for write access because that team:

  • is exactly the list of contributors that would need milestone privileges
  • likely also the list of people that would need to edit enhancement issue content
  • is actively pruned across release cycles

Previous idea:

- remove the `kubernetes-maintainers` and `kubernetes-release-managers` from having direct write access to k/features~
- update the membership of `features-maintainers` to only include the following:
  - SIG PM leadership
  - Product Management subproject leadership
  - Program Management subproject leadership
  - current Release Team Feature Lead & Shadows
- ensure the following are Maintainers of the `features-maintainers` GitHub group:
  - SIG PM leadership
  - Product Management subproject leadership
  - Program Management subproject leadership

Here's the current list to simplify the updates:
- Maintainers: @apsinha @idvoretskyi @calebamiles @justaugustus @dustinkirkland @jdumars
- Members: @kacole2 @robertsandoval @rajendar38

We'll go with lazy consensus for this on Wednesday, 7/25.

Google Group thread: https://groups.google.com/forum/#!topic/kubernetes-pm/JFthLm0q-uA

cc: @kubernetes/kubernetes-maintainers @kubernetes/kubernetes-release-managers @kubernetes/features-maintainers

/assign
/assign @cblecker
/sig pm

@liggitt
Copy link
Member

liggitt commented Jul 18, 2018

seems like that should be harmonized with membership of https://github.com/orgs/kubernetes/teams/kubernetes-milestone-maintainers and feature-approvers and milestone-maintainers aliases in https://github.com/kubernetes/kubernetes/blob/master/OWNERS_ALIASES

@liggitt
Copy link
Member

liggitt commented Jul 18, 2018

cc @kubernetes/kubernetes-milestone-maintainers

@liggitt
Copy link
Member

liggitt commented Jul 18, 2018

As long as this repo is where kubernetes/kubernetes features are planned/tracked, I think milestone modification permissions should be consistent between the two repos.

@smarterclayton
Copy link
Contributor

I'm also surprised at the proposal that maintainers aren't able to decide the status of their features. That seems... unusual, especially since those are the people most likely to have an accurate assessment of the feature.

I'm also unsure as to how the "protect the milestone label" translates to "maintainers and release managers don't have write access to this repo".

@justaugustus
Copy link
Member Author

@liggitt @smarterclayton -- hmmm, this is more of a "too many cooks" concern than anything else.
If someone who doesn't actually maintain the Features Tracking spreadsheet moves a milestone, we (currently) have no clever way of tracking that without sweeping this repo again.

If that could be solved though...

@liggitt
Copy link
Member

liggitt commented Jul 18, 2018

Yeah, it seems like fixing the current manual issue tracker <-> spreadsheet process would be better than putting obstacles in the way of the actual milestone maintainers

Can sig-release/sig-pm look into pulling snapshot reports of milestone items from the features repo automatically? A bot that scrapes the milestone items daily/weekly/whenever and snapshots that as a committed md file would stay up to date and show change over time.

@justaugustus
Copy link
Member Author

Can sig-release/sig-pm look into pulling snapshot reports of milestone items from the features repo automatically? A bot that scrapes the milestone items daily/weekly/whenever and snapshots that as a committed md file would stay up to date and show change over time.

Absolutely, @liggitt. I've previously opened some issues around the topic and plan on taking up this effort:

Would love any feedback around the process on those issues as well! :)

@cblecker
Copy link
Member

Two thoughts:

  • Limiting direct write access to the repo is in line with our ongoing efforts to move this over to bots. I think that's still worth moving forward with.
  • We could enable the milestone prow plugin to allow the /milestone command to work in this repo, similar to k/k.

@liggitt
Copy link
Member

liggitt commented Jul 18, 2018

Limiting direct write access to the repo is in line with our ongoing efforts to move this over to bots. I think that's still worth moving forward with.

Probably so, though description editing of issues created by someone else is probably done more in this repo than any other I work in (all sig-auth leads work to maintain/curate our sig's feature issues, for example). The bots don't let us do that.

We could enable the milestone prow plugin to allow the /milestone command to work in this repo, similar to k/k.

we could, but the premise of this issue was to prevent modification of feature milestones by some of the people responsible for the content of the milestone

@cblecker
Copy link
Member

description editing of issues created by someone else is probably done more in this repo than any other I work in

That I didn't know. I know that's still a pending issue for removing it from k/k, but wasn't aware that same use case applies here.

My thought on the milestone command was to offer a way to still allow milestone modification, but not need direct write access. The milestone command could also be modified to do things like ping a team when a milestone is modified so that changes can be tracked.

@justaugustus
Copy link
Member Author

@cblecker @liggitt --

To summarize:

Feature Owners (SIG Leads + people implementing / sheparding feature) need to:

  • edit issues
  • edit milestones

Feature Maintainers (SIG PM - Product Management, Release Team Features Lead & Shadows) need to:

  • edit issues
  • edit labels
  • edit milestones
  • have trusted signal on the current status of a feature in the milestone

In addition, passerbys (users or contributors) should be able to have a clear idea of the likelihood of an issue being delivered within a milestone, without having to read the entire discussion or get into the technical weeds.

So as a solution, how about we do the following:

  • allow feature-maintainers and milestone-maintainers access to edit issues
  • clean up feature-maintainers and milestone-maintainers
  • enable /milestone command
  • introduce a set of Feature triage labels and accompanying /feature command, that would be reserved for Feature Maintainers

The latter lends itself to @liggitt's suggestion for a milestone scraping bot and echoes some of the intent of kubernetes/test-infra#8316.

@m1093782566
Copy link

@justaugustus

clean up feature-maintainers and milestone-maintainers

What does it mean? And what's the clean-up rule?

@justaugustus
Copy link
Member Author

What does it mean? And what's the clean-up rule?

@m1093782566 --
I'd like to audit the GitHub teams and ensure only the required people are a part of it.

For features-maintainers:

update the membership of features-maintainers to only include the following:
SIG PM leadership
Product Management subproject leadership
Program Management subproject leadership
current Release Team Feature Lead & Shadows

For milestone-maintainers, we'll likely need to include members of past and present Release Teams, as well as ping SIGs for appropriate reps and then document it for posterity.

@justaugustus justaugustus added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jul 22, 2018
@justaugustus
Copy link
Member Author

justaugustus commented Jul 26, 2018

@liggitt @smarterclayton -- please provide feedback on this PR about maintaining milestone-maintainers: kubernetes/sig-release#239

@justaugustus justaugustus added the tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team label Jul 29, 2018
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 27, 2018
@nikhita
Copy link
Member

nikhita commented Nov 22, 2018

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 22, 2018
@spiffxp
Copy link
Member

spiffxp commented Feb 7, 2019

As of today:

I can't speak to the policy of who is in what team at present, though FWIW:

  • the membership of kubernetes-release-managers is disjoint with enhancements-maintainers
  • the membership of kubernetes and enhancements-maintainers is not cleanly separated, I would rather move over whomever needs write access to this repo to enhancements-maintainers

@justaugustus
Copy link
Member Author

justaugustus commented Feb 17, 2019

Thanks for resurrecting this one, @spiffxp!

How about we do the following?

I suggest kubernetes-milestone-maintainers for write access because that team:

  • is exactly the list of contributors that would need milestone privileges
  • likely also the list of people that would need to edit enhancement issue content
  • is actively pruned across release cycles

@justaugustus justaugustus changed the title Tighten Permissions on k/features Update repo permissions on k/enhancements Feb 17, 2019
@justaugustus
Copy link
Member Author

@cblecker -- thoughts on the plan above?

@cblecker
Copy link
Member

I personally don't have any concerns about the above. That said, I'm not an active feature contributor, so I am not up on the current features/enhancements process. I'd suggest re-seeking feedback from @liggitt.

@justaugustus
Copy link
Member Author

justaugustus commented Feb 20, 2019

Slack convo with @liggitt yesterday:

justaugustus [8:08 PM]
Hey Jordan, could I get your thoughts on this one when you have a chance? #590 (comment)

liggitt [8:37 PM]
Seems sensible overall. Have you figured out who would be losing permissions? Sometimes that's helpful to compute, along with how recently some of those users did actions that would have been affected, to see if there's a dimension that has been forgotten.

justaugustus [8:41 PM]
Hmmm, I did not. Operating off of the premise that those who should be doing the actions should really be part of kubernetes-milestone-maintainers anyway, so funneling them through that process instead. If there's an easy to calculate the diff then perhaps, but I think we'd want to steer them into getting milestone privs regardless of the result.

@justaugustus
Copy link
Member Author

Sent notification to mailing list regarding this change: https://groups.google.com/d/msg/kubernetes-pm/JFthLm0q-uA/SzMBBCzIBQAJ

Lazy consensus: Thursday, 2/21 EOD PT
Change to happen: Friday, 2/22

@cblecker
Copy link
Member

All the team memberships are in k/org.. so to pull the diff:

  • Grab membership lists of kubernetes-maintainers, kubernetes-release-managers
  • Combine and de-dupe
  • Grab membership of kubernetes-milestone-maintainers
  • Remove intersection between the two lists

Then you have a list of those who would have their access removed.

Then you should be able to use something like https://k8s.devstats.cncf.io/d/46/pr-reviews-by-contributor?orgId=1&var-period=d7&var-repo_name=kubernetes%2Fenhancements&var-reviewers=All&from=now-90d&to=now to find out if they have touched anything in k/enhancements.

Data is power!

@justaugustus
Copy link
Member Author

Okay, I did a little massaging here:

Overall, that devstats board won't be useful here.
From the thread, what people actually care about is whether or not they can edit descriptions for existing issues, which I don't think can be easily gleaned from DevStats.

So for people:

  • who have created an enhancement tracking issue - no problem; you can edit your own issue
  • who want to edit an enhancement tracking issue - I'd qualify as milestone-maintainers and they should PR to k/org with their SIG's approval for access

For the sake of overcommunication...


@kubernetes/kubernetes-maintainers and @kubernetes/kubernetes-release-managers --

We are updating the permissions on k/enhancements and will be removing groups that don't require write access to the repo, namely kubernetes-maintainers and kubernetes-release-managers. Additionally, we will be granting @kubernetes/kubernetes-milestone-maintainers write access.

The most requested use case for write access to this repo is being able to edit the descriptions for Enhancement tracking issues.

If you feel you should have this capability, you should be a part of the kubernetes-milestone-maintainers GitHub team. To gain access to this team, please file a PR in k/org updating this team and have the SIG that you'll be maintaining milestones for approve the PR.

The following list of contributors will be losing write access to k/enhancements:
@aleksandra-malinowska
@alex-mohr
@apelisse
@aronchick
@bgrant0607-nocc
@bprashanth
@brendandburns
@caesarxuchao
@cjcullen
@david-mcmahon
@davidopp
@ecordell
@eparis
@erictune
@errordeveloper
@euank
@fabioy
@fgrzadkowski
@freehan
@gmarek
@grodrigues3
@hongchaodeng
@ingvagabund
@ixdy
@janetkuo
@jbeda
@jessfraz
@jingxu97
@jlowdermilk
@jsafrane
@jszczepkowski
@k8s-release-robot
@Kargakis
@kashomon
@kevin-wangzefeng
@krousey
@lukemarsden
@MaciekPytel
@madhusudancs
@maisem
@matchstick
@mbohlool
@mdelio
@mike-saparov
@mml
@mtaufen
@ncdc
@nikhiljindal
@philips
@pmorie
@pweil-
@q-lee
@quinton-hoole
@random-liu
@rmmh
@sarahnovotny
@smarterclayton
@sttts
@thelinuxfoundation
@timstclair
@vishh
@xiang90
@yifan-gu
@yujuhong
@zmerlynn

@BenTheElder
Copy link
Member

@justaugustus
Copy link
Member Author

justaugustus commented Feb 20, 2019

the last bit of that list doesn't seem to be registering...

@BenTheElder -- probably maxed out on @ mentions on the comment, but everyone should've been notified as a result of being part of those teams.

@justaugustus
Copy link
Member Author

@spiffxp
Copy link
Member

spiffxp commented Feb 22, 2019

/unassign @cblecker
/assign @spiffxp
Doing the needful as described in #590 (comment)

@k8s-ci-robot k8s-ci-robot assigned spiffxp and unassigned cblecker Feb 22, 2019
@spiffxp
Copy link
Member

spiffxp commented Feb 22, 2019

Dropped the following teams (their ACL):

  • kubernetes-maintainers (write)
  • kubernetes-reviewers (read) (this is unnecessary since it's a public repo)
  • kubernetes-release-managers (write)

Added the following teams (their ACL):

  • kubernetes-milestone-maintainers (write)

@spiffxp
Copy link
Member

spiffxp commented Feb 22, 2019

Removed the hold on kubernetes/org#498, it will take some time for those changes to propagate. Suggest leaving this open until they're verified.

@justaugustus
Copy link
Member Author

justaugustus commented Feb 23, 2019

Thanks for making the changes, @spiffxp! I've verified that the updates to enhancements-maintainers have taken effect.

I've opened two PRs to close the loop on this:

(the latter will close this issue once it merges)

@spiffxp spiffxp removed the tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team label Feb 23, 2019
@justaugustus justaugustus added the area/enhancements Issues or PRs related to the Enhancements subproject label Apr 19, 2020
@justaugustus
Copy link
Member Author

justaugustus commented Jan 15, 2021

x-post from #enhancements:

The Triage role

Recommended for contributors who need to proactively manage issues and pull requests without write access

ref: https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization

did not exist on GitHub when we initially configured privileges for k/enhancements, so we needed to grant milestone-maintainers write access to the repo to allow them to edit the descriptions on Enhancement issues.

Given this role does exist now, I've downgraded the access of milestone-maintainers from Write to Triage to prevent accidental pushes in the future.

@annajung -- FYI, in case enhancement owners encounter issues during enhancement collection.

cc: @kubernetes/enhancements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/enhancements Issues or PRs related to the Enhancements subproject kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

No branches or pull requests

10 participants