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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documented admin permissions for adding maintainers and creating repos. #174

Merged
merged 4 commits into from Sep 20, 2023

Conversation

dblock
Copy link
Member

@dblock dblock commented Aug 8, 2023

Description

A lightweight doc that explains how to effect maintainer permissions or create/adopt new repos in the org.

Note that I attempted to reference an internal Amazon wiki with a little 馃敀 emoji, following the Artsy README style.

Screenshot 2023-08-08 at 2 48 50 PM

Issues Resolved

Closes #82.
Closes #79.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock
Copy link
Member Author

dblock commented Aug 8, 2023

@nknize @anastead FYI

ADMINS.md Outdated
The AWS Open Source Program Office (OSPO) currently owns and manages the opensearch-project organization, and has permissions to create a new repo. While the admins above have admin-level permissions, they do not have permissions to create new repositories or move repositories into the organization.

All new repositories inside opensearch-project follow the [security response process](SECURITY.md), and therefore require an Amazon team to be engaged when necessary. If you don't work for Amazon, and wish to create a repository in this organization, or move a repository into opensearch-project, please first discuss it in the [#maintainers channel on the public Slack](https://opensearch.slack.com/archives/C05L60S4UBT), and one of the above-mentioned admins will seek a team to support you. If you work for Amazon, and need a repo, you may need to take some additional steps. Please follow the instructions on [馃敀 this internal Wiki](https://w.amazon.com/bin/view/OpenSearch_Project/GitHub_Repos/).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not comfortable sharing internal wiki's.
But anyway, an alternative I believe would be better is to have an issue template in this repo which describes "the request for creating/moving a repo". This would be even field for all requestor's (Community or Amazonian's) and obviously admins can follow up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your reasons for not being comfortable with sharing an internal link? This one is pretty vanilla IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just exposing internal tools and their URLs. May be its just me :).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can make a generic note saying that there may be additional steps depending on organization affiliation. I am not concerned about exposing internal tools personally, but I would like to avoid as many Amazon specific references as we can. Does that sound like a reasonable alternative?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not concerned about this internal wiki link, as that URL does not disclose any interesting information and its documented that its 'restricted' access.

Copy link
Member Author

@dblock dblock Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we all agree on a couple of things:

  1. The link itself is not anything sensitive, and we must continue being careful about not disclosing any information that shouldn't be.
  2. It's totally ok for a company to ask its employees to follow an additional process because if I leave my employer someone will have to inherit some of my commitments for me.

I like my idea of putting a link to a private wiki in open-source because it's "open-source first", and it has worked for me well in past life. I want employees at Amazon to be reading these open-source docs, then if they need to do something extra because the company asks them to, read something internal.

I would also, personally, be totally fine for another company with N+1 developers heavily involved in OpenSearch open-source (e.g. Aiven, cc: @reta) to put an internal link to their own wiki here.

I'm curious what @hyandell thinks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS managed open source projects should not include internal URLs. This includes wiki links.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it. But thought I'd try, I still think it's a harmless idea.

peternied
peternied previously approved these changes Aug 9, 2023
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, thanks for documenting this parts to the 'what and why' of some of the Admin practices.

馃憤 adding the #maintainers slack channel - great place to start those discussions.

nknize
nknize previously requested changes Aug 9, 2023
Copy link
Contributor

@nknize nknize left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the transparency intent and motivation. Putting down my initial thoughts in a quick first round review.

ADMINS.md Outdated

## Overview

This document explains who the admins are (see below), what they do in opensearch-project, and how they should be doing it. They are added with Admin-level permissions to every repositories in opensearch-project organization. If you're interested in becoming a maintainer, see [MAINTAINERS](MAINTAINERS.md). If you're interested in contributing, see [CONTRIBUTING](CONTRIBUTING.md).
This document explains who the admins are (see below), what they do in opensearch-project, and how they should be doing it. They are added with Admin-level permissions to every repositories in opensearch-project organization. If you're interested in becoming a maintainer, see [MAINTAINERS](MAINTAINERS.md). If you're interested in contributing, see [CONTRIBUTING](CONTRIBUTING.md).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we ever document why or how the individuals below were granted admin karma?

Copy link
Member Author

@dblock dblock Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did not. Originally this was (a very large) group that worked on setting up the GitHub org and systems during the fork. The list was reduced to people who maintain the support infrastructure (https://github.com/opensearch-project/opensearch-build, CI/CD). Is it worth adding? Is there another organizational README that states such history?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this:

These are individuals that worked on creating the OpenSearch fork, and those that currently support the organization-wide infrastructure, such as the public CI/CD.

Perform administrative tasks, such as [adding](MAINTAINERS.md#adding-a-new-maintainer) and [removing maintainers](MAINTAINERS.md#removing-a-maintainer).
Perform administrative tasks, such as [adding](RESPONSIBILITIES.md#adding-a-new-maintainer) and [removing maintainers](RESPONSIBILITIES.md#removing-a-maintainer).

Please note that maintainers typically do not have admin-level permissions in their repos in this organization. Admin-level permissions allow for sensitive and destructive actions, such as managing security, or deleting a repository. Therefore, admin access in opensearch-project was designed to be deliberately centralized in ways that requires that two people to make any one sensitive change. If you need to perform an admin function, such as adding or removing a maintainer, please [follow the maintainer nomination process](RESPONSIBILITIES.md#becoming-a-maintainer), then ask to effect permissions by tagging `@admin` in your pull request, or the [#maintainers channel on the public Slack](https://opensearch.slack.com/archives/C05L60S4UBT). One of the above-mentioned admins will make this change for you.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to flesh this out a lot more. A move to more open governance with shared ownership should allow for non AWS members to be granted admin karma but we don't have anything formalized. Perhaps we can have @hyandell weigh in here on what is possible w/ outside collaborators vs AWS members of the opensearch-project github org.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all good future suggestions, but my intent here is to document current state.

ADMINS.md Outdated
Adopt organizational best practices, work in the open, and collaborate with other admins by opening issues before making process changes. Prefer consistency, and avoid diverging from practices in the opensearch-project organization.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "organizational best practices" mean? Do we have anything concrete here to offer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is existing text that just moved, I don't want to remove it or change it, but it's a good point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed "Organizational Best Practices" to "Organizational Practices" and said "Adopt organizational practices documented in this repo," to make it clear.

There are currently two ways new repositories may appear in the opensearch-project organization: creating a new repo and adopting, or moving a repo from outside of the organization into it. The process is the same.

The AWS Open Source Program Office (OSPO) currently owns and manages the opensearch-project organization, and has permissions to create a new repo. While the admins above have admin-level permissions, they do not have permissions to create new repositories or move repositories into the organization.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There currently isn't a public way for someone to "open a request" with the AWS Open Source Program Office. Is this even worth calling out? Or should we be more direct with something like the below rough draft:

The AWS Open Source Program Office (OSPO) currently controls and manages the opensearch-project github organization, and is the only group with permissions to create new repositories. Therefore there is no mechanism for community maintainers or collaborators that are not employed by Amazon to directly request the OSPO create a new repository on their behalf. If you are interested in donating a repository to the project, reach out to an AWS admin in the list above (or post on the OpenSearch Forum) your intent to donate a repository to the opensearch-project organization.

Then maybe we have a separate guide Interested in donating a project or repository to OpenSearch?

Copy link
Member Author

@dblock dblock Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say basically the same thing below and explain that beyond permissions there needs to be an Amazon team on-call for security.

All new repositories inside opensearch-project follow the security response process, and therefore require an Amazon team to be engaged when necessary. If you don't work for Amazon, and wish to create a repository in this organization, or move a repository into opensearch-project, please first discuss it in the #maintainers channel on the public Slack, and one of the above-mentioned admins will seek a team to support you.

Do you have specific suggestions to edit this text?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better for the opensearch maintainers to bring new repo requests to the AWS OSPO, rather than encouraging others to go directly, if that makes sense. 馃榿

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for as long as OSPO creates repos, most definitely. My updated text version says this:

"All new repositories inside opensearch-project follow the security response process, and therefore require an Amazon team to be engaged when necessary. If you wish to create a repository in this organization, or move a repository into opensearch-project, please contact one of the above-mentioned admins via the #maintainers channel on the public Slack."

WDYT?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not able to access the security response link. Is it because it is an internal link? It may be good to elaborate more and provide transparency on what it takes to be compliant with security

Copy link
Member Author

@dblock dblock Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a relative link in the content so it renders incorrectly in my copy-pasted comment. The complete URL is https://github.com/opensearch-project/.github/blob/main/SECURITY.md

@nknize nknize requested a review from hyandell August 9, 2023 16:13
@nknize
Copy link
Contributor

nknize commented Aug 9, 2023

/cc @ke4qqq @mikemccand @rbowen @spotaws for fresh eyes

@dblock dblock added the bake-time Marks pull requests that are pending a minimum waiting period before they are merged label Aug 11, 2023
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the updates, this is a good step forward. To me the feedback in open conversations have been addressed or could be followed up in a separate PR.

@dblock dblock requested a review from nknize August 14, 2023 15:46

### Adopt Organizational Best Practices
All new repositories inside opensearch-project follow the [security response process](SECURITY.md), and therefore require an Amazon team to be engaged when necessary. If you wish to create a repository in this organization, or move a repository into opensearch-project, please contact one of the above-mentioned admins via the [#maintainers channel on the public Slack](https://opensearch.slack.com/archives/C05L60S4UBT).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the place we generally get into sticky wickets with admins and projects is around the project settings page. We haven't defined what those best practices are, and so one frequent reason cited for needing admin rights is to change them.

I think it might be worth calling out something that says "if you're a maintainer and you want to change the settings there, open an issue in .github and tag the admin team, and they'll review the change and make it".

It's been on my personal backlog to figure out a way to script setting those settings so they get set up in a certain way at creation, so we're being intentional when we change them, but I haven't gotten around to it.

The other reason we get requests for admin rights is to be able to use zen hub. I think it's worth it to state explicitly that this is not a reason to grant admin rights. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good point, but for now I'd rather not introduce a new process of opening issues in .github. So far I've seen issues opened in their own repos for changes in that repo, rather than in .github.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally just get backchanneled. I've never seen a issue in a particular repo. Is that how we want folks to handle it? If so, let's put that there.

@dblock
Copy link
Member Author

dblock commented Aug 15, 2023

Let's wait for a few days for @nknize to review/dismiss his asks.

@peternied
Copy link
Member

Doh, it would be great if we could reset the bake-time timer... maybe someone could make a PR while waiting ;)

@nknize
Copy link
Contributor

nknize commented Aug 16, 2023

I'll take a look again tomorrow. The folks I tagged have been on travel and I think it's worth getting their thoughts as well.

@dblock
Copy link
Member Author

dblock commented Aug 21, 2023

I'll take a look again tomorrow. The folks I tagged have been on travel and I think it's worth getting their thoughts as well.

Last change was 11 days ago, I'll merge this mid week if nobody objects. This PR just documents current state, there's no new process here.

@peternied peternied dismissed nknize鈥檚 stale review September 19, 2023 13:32

Outdated feedback, no response.

@peternied
Copy link
Member

@nknize I dismissed your review since there hasn't been any updates and the comments raised have responses from @dblock.

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Copy link
Member Author

dblock commented Sep 20, 2023

I rebased, which reset bake time. Merging.

@dblock dblock merged commit 17519a8 into opensearch-project:main Sep 20, 2023
1 of 2 checks passed
@dblock dblock deleted the admins-repos branch September 20, 2023 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bake-time Marks pull requests that are pending a minimum waiting period before they are merged
Projects
None yet
8 participants