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

New teams #62

Merged
merged 2 commits into from
Aug 17, 2024
Merged

New teams #62

merged 2 commits into from
Aug 17, 2024

Conversation

ryjones
Copy link
Contributor

@ryjones ryjones commented Aug 6, 2024

Creating teams for repos

@ryjones
Copy link
Contributor Author

ryjones commented Aug 6, 2024

@baentsch @bhess this is to move people from collaborator roles to teams with roles. The level of access should remain the same - most of the collaborators were already members of a team with identical access.

Signed-off-by: Ry Jones <ry@linux.com>
Signed-off-by: Ry Jones <ry@linux.com>
Copy link

clowarden bot commented Aug 10, 2024

Validation succeeded

✅ The proposed configuration changes are valid!

Configuration changes

Directory

  • team liboqs-cupqc-maintainers has been removed
  • team liboqs-cpp-admins has been added
    • Maintainers
      • vsoftco
  • team liboqs-go-admins has been added
    • Maintainers
      • vsoftco
  • team www has been added
    • Maintainers
      • crockeea
    • Members
      • SWilson4
      • christianpaquin
      • praveksharma
      • vsoftco
  • team liboqs-java-committers has been added
    • Maintainers
      • ryjones
    • Members
      • jimouris
  • thomwiggers is now a member of team liboqs-committers
  • ashman-p is now a member of team liboqs-committers
  • cothan is now a member of team liboqs-committers

Github

  • user Martyrshot is no longer a collaborator of repository liboqs
  • user cothan is no longer a collaborator of repository liboqs
  • user thomwiggers is no longer a collaborator of repository liboqs
  • user ashman-p is no longer a collaborator of repository liboqs
  • team liboqs-cpp-admins has been added to repository liboqs-cpp (role: admin)
  • user vsoftco is no longer a collaborator of repository liboqs-cpp
  • team liboqs-go-admins has been added to repository liboqs-go (role: admin)
  • user vsoftco is no longer a collaborator of repository liboqs-go
  • team liboqs-java-committers has been added to repository liboqs-java (role: write)
  • team oqs-provider-codeowners role in repository oqs-provider has been updated to admin
  • team www has been added to repository www (role: write)
  • user praveksharma is no longer a collaborator of repository www
  • user christianpaquin is no longer a collaborator of repository www
  • user vsoftco is no longer a collaborator of repository www
  • user SWilson4 is no longer a collaborator of repository www
  • user crockeea is no longer a collaborator of repository www

🔸 Please review the changes detected as they will be applied immediately once this PR is merged 🔸

@ryjones
Copy link
Contributor Author

ryjones commented Aug 10, 2024

@baentsch please review

Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

As far as oqsprovider is concerned, this seems to go from too few permissions for too few people to too many permissions for too many people. Is there no way to model exactly what's documented in GOVERNANCE.md (unless answers to #45 contradict that)?

config.yaml Show resolved Hide resolved
@ryjones ryjones merged commit 6db8147 into main Aug 17, 2024
2 checks passed
@ryjones ryjones deleted the teams branch August 17, 2024 19:02
Copy link

clowarden bot commented Aug 17, 2024

Reconciliation completed

✅ The reconciliation completed successfully and all changes have been applied across the services!

Changes applied

Github

  • team liboqs-cpp-admins has been added
    • Maintainers
      • vsoftco
  • team liboqs-go-admins has been added
    • Maintainers
      • vsoftco
  • team liboqs-java-committers has been added
    • Maintainers
      • ryjones
    • Members
      • jimouris
  • team www has been added
    • Maintainers
      • crockeea
    • Members
      • SWilson4
      • christianpaquin
      • praveksharma
      • vsoftco
  • cothan is now a member of team liboqs-committers
  • thomwiggers is now a member of team liboqs-committers
  • ashman-p is now a member of team liboqs-committers
  • user thomwiggers is no longer a collaborator of repository liboqs
  • user ashman-p is no longer a collaborator of repository liboqs
  • user cothan is no longer a collaborator of repository liboqs
  • user Martyrshot is no longer a collaborator of repository liboqs
  • team liboqs-cpp-admins has been added to repository liboqs-cpp (role: admin)
  • user vsoftco is no longer a collaborator of repository liboqs-cpp
  • team liboqs-go-admins has been added to repository liboqs-go (role: admin)
  • user vsoftco is no longer a collaborator of repository liboqs-go
  • team liboqs-java-committers has been added to repository liboqs-java (role: write)
  • team oqs-provider-codeowners role in repository oqs-provider has been updated to admin
  • team www has been added to repository www (role: write)
  • user praveksharma is no longer a collaborator of repository www
  • user SWilson4 is no longer a collaborator of repository www
  • user crockeea is no longer a collaborator of repository www
  • user vsoftco is no longer a collaborator of repository www
  • user christianpaquin is no longer a collaborator of repository www

@baentsch
Copy link
Member

@dstebila Can you please explain why you think it's a good idea to give admin rights to oqs-provider to everyone with a "code-owner" designation? If I'm not wrong, you now approved all contributors to have even more privileges than normal GH maintainer(s): This is IMO the exact opposite of the least privilege concept and also a direct contradiction to GOVERNANCE.md. The latter irks me even more than the security downgrade: Why did I write this in the first place if not to provide some "guard rails" protecting the code?

@dstebila
Copy link
Member

@dstebila Can you please explain why you think it's a good idea to give admin rights to oqs-provider to everyone with a "code-owner" designation? If I'm not wrong, you now approved all contributors to have even more privileges than normal GH maintainer(s): This is IMO the exact opposite of the least privilege concept and also a direct contradiction to GOVERNANCE.md. The latter irks me even more than the security downgrade: Why did I write this in the first place if not to provide some "guard rails" protecting the code?

You're right. What level of access should the codeowners team have?

@baentsch
Copy link
Member

You're right. What level of access should the codeowners team have?

Codeowners in my eyes are Contributors and should have the same rights. But I already don't understand the separation of "codeowners" and "contributors" to begin with. This only exists in oqsprovider, no-where else: Why?

In general, I think the whole file is a big mess, completely un-structured, not orthogonal across projects and not in line with GOVERNANCE.md files where present. Yes, it did become better recently with the introduction of teams (though not consistently, see e.g., boringssl), but still, I don't understand why there's not a simple, orthogonal/per-project "maintainers", "committers", and "triage" category with admin (or maintain, if no sufficient trust exists), write, and triage rights, respectively as per our governance rules? Related question: Why is there a need to sometime have "admin" and sometime "maintainer" and sometime both teams per sub-project? Then: Why is there absolutely no documentation (e.g., as to exceptions to the "orthogonality" rule)?
This "orthogonality" can be simplified by having a single triage team triaging all OQS projects (as is the case if I get it right) and a single language-wrapper sub-project "designator" for all language-wrapper projects but rust (not the case right now, if I get it right). These changes alone should cut the size of the file substantially and make it understandable to the unwary (or at least me :). In any case, it should never violate GOVERNANCE.md files as it did from the very start until now. Lastly, I don't understand why an admin often (but not always) is maintainer (e.g., of teams): If this should be managed by the community, only community members (arguably maintainers) should be listed. I understand it was started by an admin to manage the multitude of projects, but at no time did I see agreement (and adherence) to basic principles like these:

  • Never violate GOVERNANCE.md files' contents where present
  • Keep things orthogonal across projects -- if not, document by comment why it's not possible so everyone can understand exceptions
  • Do not have LF admins overrule maintainer's rights

OK, the latter is probably just my problem since the don't-trust-@baentsch principle started in April when all my permissions were withdrawn, so probably unrealistic/unacceptable to LF. Nevertheless I think it would be a logical principle if the community and not LF admins should control the project('s access rights). Conceptually, what about an OQS-maintainers team listed as maintainer for all sub-project's maintainer team lists? In my "ideal" world, every person would be listed in as few teams as possible, ideally just one -- every additional use of a specific person's ID should be accompanied by a comment. This way, it becomes obvious when specific persons become "risky" (in terms of "key person risk") and the community aware of the need to do something about it.

As usual, sorry for the long text & disregard as you see fit.

@SWilson4
Copy link
Member

Codeowners in my eyes are Contributors and should have the same rights. But I already don't understand the separation of "codeowners" and "contributors" to begin with. This only exists in oqsprovider, no-where else: Why?

I actually created the "oqs-provider-codeowners" team here so as not to override the oqs-provider GOVERNANCE file. Codeowners need "write" access in order to approve pull requests. Requesting codeowner review on PRs was the stated purpose of open-quantum-safe/oqs-provider#458, so the codeowners added there needed to have "write" access at minimum. However, the codeowner list included people who were not listed as committers in the GOVERNANCE file, so I couldn't simply add them to the "oqs-provider-committers" team (which already had sufficient perms) without overriding that file.

@SWilson4
Copy link
Member

Perhaps our Committers ought in fact to have "maintain" privileges, Maintainers ought to have "admin" privileges, and Contributors (incl. people in the codeowners file) ought to have "write" privileges? (See here for documentation on perms.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants