-
Notifications
You must be signed in to change notification settings - Fork 836
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
groups/groups.yaml can become difficult to maintain #460
Comments
Strong +1 for this! |
Maybe longterm, but right now the token to manage groups is extremely highly privileged.. I think only dims and I have access to it (the process is a manual run). So either way, we have to funnel approvals through a couple folks anyways. Sharding the config will only get us partially the way there. |
Can we go around this by setting up the sync as a postsubmit like we do for peribolos? |
Peribolos has a bunch of safeguards in place to prevent locking itself out from GitHub. We don't have those kinds of safeguards in place on the groups sync script.. right now, it requires a dry run, manual verification of the changes, and then a real live run. If we had better testing and some of those safeguards in place, I'd be way more comfortable/confident with us doing this. |
maybe we can break the monolith file into smaller files which are merged at run-time. At least it would be easier to read? Maybe even directory structure to separate RBAC groups from role groups from administrative groups |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
#925 is a start at enforcing some conventions, though I'm not sure what "don't lock us out" tests we should have |
Oh, one other note to address #460 (comment) There are four humans who directly have the org admin role, in the event that the @kubernetes.io groups disappear or stop working |
Created #996 /assign |
I recently updated slack-infra configs to shard out sig-testing resources. I really liked the approach of references:
|
So I'm not sure whether to repurpose this issue or close it in favor of a new one. But to refresh on the definition of done.... I'm not going to allow approvers in groups/sig-foo/OWNERS until we have sufficient checks or enforcement in place to ensure that:
The former can probably handled by properly sharding groups, and enumerating which use cases don't need all the extra paranoia. The latter could even be as simple as groups/whatever_test.go with a hardcoded list of groups that you've got to update every time you add a new group. If someone wants to join the pool of approvers in groups/OWNERS, I'll allow it if they're going to work on implementing the above. |
cc: @kubernetes/release-engineering, if this piques your interest. @spiffxp -- Happy to jump in on groups/OWNERS to share the load. It'd also be motivation to jump back into this refactor I was noodling on: uwu-tools/ggreconcile#1 |
Jumping in on OWNERS doesn't happen unless I see demonstrated commitment to implement the enforcement I described. A good demonstration would be PR's to improve the state of things, or authoring and driving consensus on a proposal and the necessary work items such that they meet the criteria of a help-wanted issue. |
A year too late for #996 (comment) .... created #2401 |
#2414 follows on from @nikhita's work I'm realizing we have very, very little test coverage of how this thing actually works. I would feel more comfortable with more test coverage, and think it'd be a good help-wanted issue. But I don't think it's a complete blocker to proceeding with approver delegation. I think what remains is:
|
/milestone v1.23 |
/remove-priority important-longterm I could see this being useful if, for example, we end up migrating mailing lists |
/area groups |
Looking to push this forward to done now that we have tooling able to support delegated approval. I've been going through and sharding out groups to sig-owned subdirectories, creating them as needed. What remains are the trickier corner cases, updating docs, and updating owners files. Proposed path forward
|
/assign |
/close Announcement: https://groups.google.com/g/kubernetes-dev/c/DhPel8J8QiA |
@spiffxp: Closing this issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
i foresee that groups/groups.yaml will face the fate of k/test-infra/testgrid/config.yaml :) - i.e. one big yaml with a few owners that is difficult to edit and causes conflicts in PRs.
ideally groups should be sharded on the owner side.
can group settings be delegated on the side of group owners and perhaps only the group names can be enumerated in groups/groups.yaml ?
/kind feature
/priority important-longterm
The text was updated successfully, but these errors were encountered: