-
Notifications
You must be signed in to change notification settings - Fork 213
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
go-maintainers missing permissions #982
Comments
I am not sure about the "who removed" part, but I think the policy currently says that maintainers do not normally have admin permissions: https://github.com/open-telemetry/community/blob/main/docs/how-to-configure-new-repository.md#permissions I think this was also discussed once in the past (granting admin permissions to maintainers) but I can't remember the outcome of that decision. |
Here, found the discussion: #616 |
Based on the audit log this was done in Dec 2021. The reason is very simple, since as TC members we are responsible to respect the rules and maintain the organization, and the rules are here:
There is no exception/request documented that go maintainers need special set of permissions. |
@Aneurysm9 what admin permission is required? edit: looks like its not possible |
@dyladan we discussed this in the GC, and I told you folks (2w ago if I remember correctly the time) that github allows us to define new "roles" with custom permissions, so we can define a "super-maintainer" role with more permissions if we identify that is needed. Also we can then update the community rules to give "super-maintainer" role to our maintainers. @dyladan this issue is not a real concern issue, since they did not need these permissions. It is just @Aneurysm9 trying to denigrate my reputation one more time, see here, they did not actually realize that they miss any permission. |
Among the permissions we require is the ability to manage the CI checks required on protected branches. This is a permissions that was explicitly called out in the original issue discussing changing maintainer permissions. As it is a permission that we do not frequently exercise I had not noted the permissions change in the last two months. That does not change the fact that the permissions are required and appropriate. @bogdandrutu I did not mention you at all and I do not appreciate you continuing to impute to me motivations that I do not have. Continued "insulting or derogatory comments, and personal ... attacks" will not be tolerated. |
In the issue linked:
CI hooks are different than branch protection rules which you are referring to. I am not sure whether the right to change branch protection is or not required for the maintainers, but I would be happy to discuss that in the GC/TC meeting. |
I definitely supportive to give as much permissions to repo maintainers as needed to control things independently. I listed a few reasons in the similar ticket #616 on why we went with this model https://github.com/open-telemetry/community/blob/main/docs/how-to-configure-new-repository.md#permissions. Also a note that unfortunately even "Maintain" permissions is not sufficient to allow managing CI checks. So far we were solving the shortcomings of our tooling and GitHub permission model limitations by granting individuals permission (mostly temporary) and helping with configuration via the tickets in this repo. This is burdensome for both - maintainers and TC members. We also discussed the need for dedicated GitHub admins group here: #923 (comment) that may be focused on fixing this and other tooling problems. However I am not aware of anybody volunteered to create and drive this group so far. Even for the large project as k8s with a lot of tooling, the group managing GH admin things is quite small. All that said, this particular issue may be just a misunderstanding and I hope it can be resolved. @Aneurysm9 is there any immediate issue that needs resolution wrt CI and branch protection set up? @bogdandrutu it is a good idea to re-iterate on this again. Maybe we can request some CNCF support/budgeting to solve the mentioned issues and it will allow us to make maintainers Admins again. If this will be discussed at GC meeting, let me know if any input from me is needed. |
Without the ability to edit protected branches we do not have the ability to manage the CI checks that are required on protected branches. This is not immediately required, AFAIK, but will in the near future with the upcoming release of Go 1.18. |
ok, let's keep the issue open. If this switch it planned soon, let me know if we need to make somebody as an admin for the repo for now. I also have added it to the GC meeting agenda: https://docs.google.com/document/d/1-23Sf7-xZK3OL5Ogv2pK0NP9YotlSa0PKU9bvvtQwp8/edit# @bogdandrutu should I put your name as a topic owner? This is a long standing issue and I hope we can resolve it one way or another. Thank you for bringing it up. |
I would suggest to close this issue since the title, description, and early discussion were about go maintainers missing admin permission which is resolved by the fact that the documentation clearly states they don't have those permissions. I would create a new issue requesting an additional permission for maintainers of a SIG. |
Not sure if this is the right issue for it but given there will be a GC session about it, just some notes. Just confirming that this org is actually an enterprise cloud account, so has support for custom roles as it doesn't seem to be available to normal organizations. Looking through the documented permissions (I don't have an enterprise account to verify the actual permissions UI) While I may be misinterpreting some of the permission descriptions, what seem to be notable omissions are
On the bright side, "Delete an issue", which IIRC was a gap between Maintain and Admin, does seem to be there. From my reading, if the above two gaps could also be solved then a custom role could be a viable alternative to Admin but without it, it seems to be needed for reasonable maintenance of a repo. |
Looked at custom roles. These are only permissions that are allowed beyond the "Maintain" role:
I created a custom role in the organization with all these permissions and tested it with @anuraaga. Unfortunately this role doesn't allow to do CI modification: So custom roles would not work as a solution, at least for now. |
We discussed this in the GC call today. Our general preference is to restrict permissions to the smallest necessary set, however given @SergeyKanzhelev's testing, this doesn't appear to be possible. As such, if this permission is truly needed by maintainers (the GC doesn't have the context to weigh in on this) then it seems to follow that they should have their permissions changed to admin. @dyladan will follow up, and some TC folks should probably weigh in as well (they may have more stronger opinions than the GC on this topic). |
@anuraaga your examples are great, but would like to challenge a bit here:
Changing the versions "targeted by CI" may be a significant change (sometimes even a breaking change) that may be better to require more thinking before doing it. I do agree that this is something that we should be able to do, but also it does not happen that often and may be something we can automate.
This is definitely something that we need to fix. I believe secrets should be rotated/managed at org level and have org level accounts (shared with every TC/GC members). This is a single point of failure, the fact that if @anuraaga wants today can move away from the project and we cannot release any otel-java lib. |
If there is some automation that isn't too complicated for updating the required checks that should be fine.
I agree if org secrets provided all the ones we need, there wouldn't be a need at the repo level. Not sure how practical it is to set that up - for reference, we have Maven, Gradle, AWS, and GPG secrets. But want to clarify that in terms of java publishing it's not really a single point of failure since multiple maintainers have Maven access, so just a matter of swapping the secret. An org secret might have more risk of being a single point. |
I think this issue can be closed now that #1311 is merged.
@open-telemetry/go-maintainers if you would still like Admin permissions, please open a new "Repo Maintenance" community issue requesting it. |
The @open-telemetry/go-maintainers team is missing
admin
permissions on the opentelemetry-go repo. We have had these permissions in the past and require them to appropriately maintain our repository.Can we have the
admin
permissions restored and an explanation of who removed them, when, and for what reason?The text was updated successfully, but these errors were encountered: