Conversation
93b9c1b to
ea1e19b
Compare
ildikov
left a comment
There was a problem hiding this comment.
Hi Steve,
Thank you for typing up the updates to the governance rules following the PTG discussion this week.
I added some suggestions to the wording inline, I think the content looks good and covers what we talked about at the PTG.
README.md
Outdated
| A Maintainer has the ability to merge code into the Kata Containers project. Maintainers are active Contributors and participants in the projects. In order to become a Maintainer, you must be nominated and approved by the established Maintainers. Maintainers have write access to the Kata Containers repos on GitHub. | ||
| Kata Containers Committers (as defined by the [kata-containers-committer team](https://github.com/orgs/kata-containers/teams/kata-containers-committer)) | ||
| have the ability to merge code into the Kata Containers project. | ||
| Maintainers are active Contributors and participants in the projects. In order to become a Committer, you must be nominated by established Committer and approved by quorum of the active Architecture committee via an issue against the community repo |
There was a problem hiding this comment.
| Maintainers are active Contributors and participants in the projects. In order to become a Committer, you must be nominated by established Committer and approved by quorum of the active Architecture committee via an issue against the community repo | |
| Committers are active Contributors and participants in the projects. In order to become a Committer, you must be nominated by established Committer and approved by quorum of the active Architecture committee via an issue against the community repo |
README.md
Outdated
| e.g. https://github.com/kata-containers/community/issues/403. Committers have write access to the Kata Containers repos on GitHub, which | ||
| gives the ability to approve PRs, trigger the CI and merge PRs. | ||
|
|
||
| One of the requirements to be a committer is that you are an active contributor to the project as adjudged by the above criteria. The committer list will be required for people who no longer meet the criteria |
There was a problem hiding this comment.
| One of the requirements to be a committer is that you are an active contributor to the project as adjudged by the above criteria. The committer list will be required for people who no longer meet the criteria | |
| One of the requirements to be a committer is that you are an active contributor to the project as adjudged by the above criteria. Assessing the list of active contributors happens twice a year, |
README.md
Outdated
| gives the ability to approve PRs, trigger the CI and merge PRs. | ||
|
|
||
| One of the requirements to be a committer is that you are an active contributor to the project as adjudged by the above criteria. The committer list will be required for people who no longer meet the criteria | ||
| twice a year, lining up with the Architecture Committee election cycle. At/after the election, people who are in the kata-containers-committer team, but who haven't been an active contributor in the last 12 months will be created |
There was a problem hiding this comment.
| twice a year, lining up with the Architecture Committee election cycle. At/after the election, people who are in the kata-containers-committer team, but who haven't been an active contributor in the last 12 months will be created | |
| lining up with the preparation for the Architecture Committee election cycle. At that time, the kata-containers-committer team will also be reviewed, and a list of people, who are on the team but haven't been an active contributor in the last 12 months, will be created |
README.md
Outdated
|
|
||
| One of the requirements to be a committer is that you are an active contributor to the project as adjudged by the above criteria. The committer list will be required for people who no longer meet the criteria | ||
| twice a year, lining up with the Architecture Committee election cycle. At/after the election, people who are in the kata-containers-committer team, but who haven't been an active contributor in the last 12 months will be created | ||
| and shared with the Architecture Committee and community and after a short review period to check of errors in the tooling, will be removed from the team. |
There was a problem hiding this comment.
| and shared with the Architecture Committee and community and after a short review period to check of errors in the tooling, will be removed from the team. | |
| and shared with the Architecture Committee and community. After a short review period, to allow time to check for any errors, inactive team members will be removed. |
README.md
Outdated
|
|
||
| Kata Containers Admins (as defined by the [kata-containers-admin team](https://github.com/orgs/kata-containers/teams/kata-containers-admin) have admin access to | ||
| the kata-containers repo, allowing them to do actions like, change the branch protection rules for repositories, delete a repository and manage the access of others. | ||
| There are sometimes good reasons to temporarily give others admin access, such as to create a secret that is used in a particular CI infrastructure, but generally |
There was a problem hiding this comment.
| There are sometimes good reasons to temporarily give others admin access, such as to create a secret that is used in a particular CI infrastructure, but generally | |
| The Admin group is intentionally kept small, however, individuals can be granted temporary admin access to carry out tasks, like creating a secret that is used in a particular CI infrastructure. |
README.md
Outdated
| Kata Containers Admins (as defined by the [kata-containers-admin team](https://github.com/orgs/kata-containers/teams/kata-containers-admin) have admin access to | ||
| the kata-containers repo, allowing them to do actions like, change the branch protection rules for repositories, delete a repository and manage the access of others. | ||
| There are sometimes good reasons to temporarily give others admin access, such as to create a secret that is used in a particular CI infrastructure, but generally | ||
| we should try and minimise this access. The admin list should be reviewed and updated after each Architecture Committee election and typically contain: |
There was a problem hiding this comment.
| we should try and minimise this access. The admin list should be reviewed and updated after each Architecture Committee election and typically contain: | |
| The Admin list is reviewed and updated after each Architecture Committee election and typically contains: |
README.md
Outdated
|
|
||
| ### Owner | ||
|
|
||
| GitHub organization owners have complete admin access to the organization, so should be limited for security reasons. The owners list should be reviewed and updated after each Architecture Committee election and contain: |
There was a problem hiding this comment.
| GitHub organization owners have complete admin access to the organization, so should be limited for security reasons. The owners list should be reviewed and updated after each Architecture Committee election and contain: | |
| GitHub organization owners have complete admin access to the organization, and therefore this group is limited to a small number of individuals, for security reasons. The owners list is reviewed and updated after each Architecture Committee election and contains: |
README.md
Outdated
| * [Developers](#developers) | ||
| * [Contributor](#contributor) | ||
| * [Maintainer](#maintainer) | ||
| * [Maintainer](#committer) |
There was a problem hiding this comment.
| * [Maintainer](#committer) | |
| * [Committer](#committer) |
ea1e19b to
69ae07d
Compare
Thanks so much for the care review. I've incorporated your suggestions. |
fidencio
left a comment
There was a problem hiding this comment.
There's one part that affects me the most, which is "admins".
I left my comment there, but I sincerely feel like we're taking a step on the wrong direction on this one.
README.md
Outdated
|
|
||
| Kata Containers Admins (as defined by the [kata-containers-admin team](https://github.com/orgs/kata-containers/teams/kata-containers-admin) have admin access to | ||
| the kata-containers repo, allowing them to do actions like, change the branch protection rules for repositories, delete a repository and manage the access of others. | ||
| There are sometimes good reasons to temporarily give others admin access, such as to create a secret that is used in a particular CI infrastructure, but generally |
There was a problem hiding this comment.
This part affects me the most, for instance.
I'm not part of the AC anymore, but I've been dealing -- maybe even more than AC members? -- with a lot of the admin stuff, ranging from setting up CI systems to simply unblocking PRs.
With this change, theoretically, I should have no access to do so, which will require a better velocity coming from the architecture committee.
More than that, adding someone to temporarily do something feels like a bureaucratic thing that would take more time than just having an AC member doing the thing. :-)
There was a problem hiding this comment.
I'm not part of the AC anymore, but I've been dealing -- maybe even more than AC members? -- with a lot of the admin stuff, ranging from setting up CI systems to simply unblocking PRs.
With this change, theoretically, I should have no access to do so, which will require a better velocity coming from the architecture committee.
So I think that in this circumstances you would probably fall into the
Optionally, some specific people that the Architecture Committee agree on adding for a specific purpose (e.g. to manage the CI)
category, but we also don't want to cover over the problem of the AC not being responsive, so should try and address that as well.
More than that, adding someone to temporarily do something feels like a bureaucratic thing that would take more time than just having an AC member doing the thing. :-)
The idea of temporarily adding is based on things that I've had to do in the CAA repo. People setting up CI wanted to add secrets e.g. AZURE_SUBSCRIPTION_ID that are required in our infrastructure and whilst they could have given an admin the secret and asked them to add it, that doesn't feel like a secure process. An example of this closer to home is when you/Gaby recently added ITA_KEY secret (assuming this is sensitive, if not then sorry for the bad example). If you didn't have access would you (or rather Intel) have preferred to be given admin access temporarily to add that, or to send the secret to the AC for them to add (and therefore have access). I don't believe that anyone on the AC would be interested in exploiting secrets, but speaking personally, I'd rather not be involved in handling any that I don't own.
If you don't think this is a concern and others agree, then it would be more secure to block this, so I can remove it from the proposal.
There was a problem hiding this comment.
People setting up CI wanted to add secrets e.g.
AZURE_SUBSCRIPTION_IDthat are required in our infrastructure and whilst they could have given an admin the secret and asked them to add it, that doesn't feel like a secure process.
This makes sense, indeed.
| ### Admin | ||
|
|
||
| Kata Containers Admins (as defined by the [kata-containers-admin team](https://github.com/orgs/kata-containers/teams/kata-containers-admin) have admin access to | ||
| the kata-containers repo, allowing them to do actions like, change the branch protection rules for repositories, delete a repository and manage the access of others. | ||
| The Admin group is intentionally kept small, however, individuals can be granted temporary admin access to carry out tasks, like creating a secret that is used in a particular CI infrastructure. | ||
| The Admin list is reviewed and updated after each Architecture Committee election and typically contains: | ||
| - The Architecture Committee | ||
| - Optionally, some specific people that the Architecture Committee agree on adding for a specific purpose (e.g. to manage the CI) |
There was a problem hiding this comment.
I understand we're taking a security measure to prevent something to happen. However, the community has always been based on trust and work being done.
Having this feels like we're shooting ourselves in the foot, as either you're an AC member, member of OIF, or you depend on their own criteria (regardless of them being actually active or not) to decide who'll be actually helping the project.
Now, if we want to go through a formal process, I'd strongly recommend that any change in the admin side would have to go through a vote and the majority of the admins would have to agree before it happens (including secrets being added, machines being added, PRs being merged, etc). The malicious actor may come from anywhere, including AC / OIF.
There was a problem hiding this comment.
IMO this language is fine since it mostly codifies what is already roughly the current state of things.
Now, if we want to go through a formal process, I'd strongly recommend that any change in the admin side would have to go through a vote and the majority of the admins would have to agree before it happens (including secrets being added, machines being added, PRs being merged, etc). The malicious actor may come from anywhere, including AC / OIF.
I suggest discussing adding this clause in another PR/issue since this PR has already been up for so long.
There was a problem hiding this comment.
I understand we're taking a security measure to prevent something to happen. However, the community has always been based on trust and work being done.
I don't think the proposed text hampers trust in any way. People will still manage to gain admin powers if this is beneficial for the project. The concern is more around the housekeeping when people leave the project IMHO.
sprt
left a comment
There was a problem hiding this comment.
Thanks @stevenhorsman! No major objection from me.
README.md
Outdated
| ## Developers | ||
|
|
||
| For code contributors, there are currently two roles relevant to project governance: | ||
| For contributors, there are several roles relevant to project governance: |
There was a problem hiding this comment.
To solidify "developer" as the umbrella term for all 4 roles:
| For contributors, there are several roles relevant to project governance: | |
| For Kata developers, there are several roles relevant to project governance: |
README.md
Outdated
| ## Architecture Committee | ||
|
|
||
| The Architecture Committee is responsible for architectural decisions, including standardization, and making final decisions if Maintainers disagree. It is comprised of 7 members, who are elected by contributors. | ||
| The Architecture Committee is responsible for architectural decisions, including standardization, and making final decisions if Committers disagree. It is comprised of 7 members, who are elected by contributors. |
There was a problem hiding this comment.
In general in this doc I would use:
- "Contributor" (capitalized) to refer to the specific Contributor developer role.
- "developer" to refer broadly to the 4 developer roles.
| The Architecture Committee is responsible for architectural decisions, including standardization, and making final decisions if Committers disagree. It is comprised of 7 members, who are elected by contributors. | |
| The Architecture Committee is responsible for architectural decisions, including standardization, and making final decisions if Committers disagree. It is comprised of 7 members, who are elected by Contributors. |
There was a problem hiding this comment.
I think I've updated based on your suggestions...
| ### Committer | ||
|
|
||
| A Maintainer has the ability to merge code into the Kata Containers project. Maintainers are active Contributors and participants in the projects. In order to become a Maintainer, you must be nominated and approved by the established Maintainers. Maintainers have write access to the Kata Containers repos on GitHub. | ||
| Kata Containers Committers (as defined by the [kata-containers-committer team](https://github.com/orgs/kata-containers/teams/kata-containers-committer)) |
There was a problem hiding this comment.
Note that this link isn't public - you have to be an org member to access it. We may or may not want to keep it here.
There was a problem hiding this comment.
So I think we'd also publish the proposed committers twice a year under these proposals. I think for github permissions we have to have that team as the primary source of committers though.
README.md
Outdated
| A Maintainer has the ability to merge code into the Kata Containers project. Maintainers are active Contributors and participants in the projects. In order to become a Maintainer, you must be nominated and approved by the established Maintainers. Maintainers have write access to the Kata Containers repos on GitHub. | ||
| Kata Containers Committers (as defined by the [kata-containers-committer team](https://github.com/orgs/kata-containers/teams/kata-containers-committer)) | ||
| have the ability to merge code into the Kata Containers project. | ||
| Committers are active Contributors and participants in the projects. In order to become a Committer, you must be nominated by established Committer and approved by quorum of the active Architecture committee via an issue against the community repo |
There was a problem hiding this comment.
Typos
| Committers are active Contributors and participants in the projects. In order to become a Committer, you must be nominated by established Committer and approved by quorum of the active Architecture committee via an issue against the community repo | |
| Committers are active Contributors and participants in the project. In order to become a Committer, you must be nominated by an established Committer and approved by quorum of the active Architecture Committee via an issue against the community repo |
f57962b to
4723bbd
Compare
README.md
Outdated
| A Maintainer has the ability to merge code into the Kata Containers project. Maintainers are active Contributors and participants in the projects. In order to become a Maintainer, you must be nominated and approved by the established Maintainers. Maintainers have write access to the Kata Containers repos on GitHub. | ||
| Kata Containers Committers (as defined by the [kata-containers-committer team](https://github.com/orgs/kata-containers/teams/kata-containers-committer)) | ||
| have the ability to merge code into the Kata Containers project. | ||
| Committers are active Contributors and participants in the project. In order to become a Committer, you must be nominated by established Committer and approved by quorum of the active Architecture Committee via an issue against the community repo |
There was a problem hiding this comment.
| Committers are active Contributors and participants in the project. In order to become a Committer, you must be nominated by established Committer and approved by quorum of the active Architecture Committee via an issue against the community repo | |
| Committers are active Contributors and participants in the project. In order to become a Committer, you must be nominated by an established Committer and approved by quorum of the active Architecture Committee via an issue against the community repo |
| ### Admin | ||
|
|
||
| Kata Containers Admins (as defined by the [kata-containers-admin team](https://github.com/orgs/kata-containers/teams/kata-containers-admin) have admin access to | ||
| the kata-containers repo, allowing them to do actions like, change the branch protection rules for repositories, delete a repository and manage the access of others. | ||
| The Admin group is intentionally kept small, however, individuals can be granted temporary admin access to carry out tasks, like creating a secret that is used in a particular CI infrastructure. | ||
| The Admin list is reviewed and updated after each Architecture Committee election and typically contains: | ||
| - The Architecture Committee | ||
| - Optionally, some specific people that the Architecture Committee agree on adding for a specific purpose (e.g. to manage the CI) |
There was a problem hiding this comment.
IMO this language is fine since it mostly codifies what is already roughly the current state of things.
Now, if we want to go through a formal process, I'd strongly recommend that any change in the admin side would have to go through a vote and the majority of the admins would have to agree before it happens (including secrets being added, machines being added, PRs being merged, etc). The malicious actor may come from anywhere, including AC / OIF.
I suggest discussing adding this clause in another PR/issue since this PR has already been up for so long.
Update the contributor role descriptions and guidelines for review, based on the vPTG discussion Signed-off-by: stevenhorsman <steven@uk.ibm.com>
4723bbd to
6c29767
Compare
gkurz
left a comment
There was a problem hiding this comment.
/lgtm
Thanks @stevenhorsman and sorry for the insanely long delay before reviewing this.
Update the contributor role descriptions and guidelines for review, based on the vPTG discussion