Page MenuHomePhabricator

Sync ldap/ops users to group on GitLab and define permissions for repos/sre
Closed, ResolvedPublic

Description

Task to ensure members of ldap/ops have accounts on GitLab and when they log in they have the right permissions there.

What's needed:

Event Timeline

dancy renamed this task from Sync ldap/ops users to group on GitLab define permissions for repos/sre to Sync ldap/ops users to group on GitLab and define permissions for repos/sre.Jul 28 2023, 7:43 PM

Initial proposal for permission migration.

Background

There are 5 permission levels in GitLab: guest, reporter, developer, maintainer, owner. The explanation for each is in GitLab's permission documentation.

Each repo can have protected branches that can only be directly pushed to by maintainers by default (but it is configurable).

Recommendation

  • Most things will migrate to gitlab under the repos/sre namespace (for example, https://gerrit.wikimedia.org/repos/sre/puppet)
  • Members:
    • Everyone in the ldap/ops group added to gitlab's repos/sre group with "Developer" role
    • This inherits to subprojects BUT CANNOT BE DOWNGRADED in subprojects/subgroups (for example, someone in repos/sre/software/thingy could not be made a reporter or guest BUT could be made an owner)
    • No one can be removed in subprojects/subgroups
    • TODO: does there need to be a special subset of members of repos/sre who could have the "owner" role to manage subproject permissions? Could those people be made GitLab administrators instead?
    • Rationale: This allows everyone to create merge requests and push to non-protected branches, and will DENY push to protected branches—this is the same as the operations/puppet repo in Gerrit
  • Branches
    • Set mainline branches (e.g., production or main) as protected branches
      • "Developer" can MERGE to protected branches
      • "Maintainer" can PUSH to protected branches (nobody by default)
      • Rationale: This maintains the need for code review in operations/*
      • No protected tags
        • "Developer" (i.e., everyone) can push tags by default (although this is configurable within subprojects)
    • Rationale: This maintains the status quo of operations/* in gerrit where no one can push directly, but ldap/ops and +2 and submit and push tags
  • Caveats
    • This explicitly does not use a people/sre gitlab group as a member of the repos/sre group. This quickly becomes complicated and does not cascade beyond 2-levels of subgroups, best avoided
    • Current gerrit repositories in the operations/* namespace where membership is NOT a superset of ldap/ops will need to be moved away from repos/sre, likely. For example, operations/software/gerrit is maintained by releng but lives under the operations/* tree in gerrit. This does not translate well to GitLab where sub-project membership cannot be downgraded or winnowed-down. @dancy is generating the list of repositories where ownership is not a superset of the ldap/ops membership.

Thanks for creating a path for syncing the ops group to GitLab. Some thoughts from my side:

Everyone in the ldap/ops group added to gitlab's repos/sre group with "Developer" role
Rationale: This allows everyone to create merge requests and push to non-protected branches, and will DENY push to protected branches—this is the same as the operations/puppet repo in Gerrit

The current configuration in cookbooks is maintainer permissions is needed to push and/or merge to the protected branch.

2023-08-09-gitlab-protected-branch.png (152×1 px, 18 KB)

As far as I know all SREs have +2 permissions (so are allowed to merge). This would translate to maintainer permissions in GitLab instead of developer.

Caveat: there are no CI-runners available to this instance

The are two runners available on the test instance, one Shared and one Trusted Runner.

As far as I know all SREs have +2 permissions (so are allowed to merge). This would translate to maintainer permissions in GitLab instead of developer.

That's right! My mistake. I updated the settings to allow merge on repos/sre/cookbooks from Maintainers + Developers (which is not the default).

Why not make everyone a Maintainer or an Owner? Why Developer vs Maintainer?

Two main reasons:

  1. By default, Maintainers can push directly to protected branches. We could update protected branches to say "no one" can directly push. Instead we made everyone a Developer because...
  2. You can up-level roles in subprojects and subgroups, but not down-level. You could make a Developer at the repos/sre level an Owner of repos/sre/cookbooks, but not vice-versa. Maintainers can do a lot by default. Developer is the minimum useful role.

Caveat: there are no CI-runners available to this instance

The are two runners available on the test instance, one Shared and one Trusted Runner.

oh! Nice :)

I would ask why the "Developer" role was chosen instead of the "Owner" one. Specifically, looking at https://gitlab.wikimedia.org/help/user/permissions.md looks like a few important functions would be prevented:

  • Deleting issues and merge requests
  • Modifying merge request policies
  • Deleting packages from the package registry
  • manage push rules for repositories
  • pushing to protected branches
  • Importing/exporting projects

I suppose that probably the reason for going with the developer role here has to do with preventing modifications of team memberships, but all of the above seems to be important actions we would want to self-manage in our set of repositories.
Add to this that any one of us can anyways circumvent these protections and make themselves owners of the projects as we have global root, meaning the enforcement is just at face value, that we restrict the roles we have or not.

I would ask why the "Developer" role was chosen instead of the "Owner" one. Specifically, looking at https://gitlab.wikimedia.org/help/user/permissions.md looks like a few important functions would be prevented:

  • Deleting issues and merge requests
  • Modifying merge request policies
  • Deleting packages from the package registry
  • manage push rules for repositories
  • pushing to protected branches
  • Importing/exporting projects

I suppose that probably the reason for going with the developer role here has to do with preventing modifications of team memberships, but all of the above seems to be important actions we would want to self-manage in our set of repositories.
Add to this that any one of us can anyways circumvent these protections and make themselves owners of the projects as we have global root, meaning the enforcement is just at face value, that we restrict the roles we have or not.

You're all maintainers now (a bug in GitLab has made the "developer" role very useless). The delta between maintainer and owner is pretty small, so if it's easiest to just make everyone on owner: I'm fine with that, too.

The caveat is: you can't down-level people in subgroups/subrepos.

So if you had a repos/sre/very-sensitive-project the lowest role you could give anyone else in SRE is, currently, maintainer.

But maybe that's fine and we could make other top-level trees for projects where only subteams can access?


Initially, I was trying to make folks developers because developers can't push to protected branches.

This is configured at the "group" level via the Fully Protected option:

2023-08-14_gitlab-group-level-branch-permissions.png (449×915 px, 61 KB)

Push access to protected branches seems undesirable to me (a bad thing to whoopsy—if this were the default in gerrit I'd have directly pushed a few times), and is different than the default behavior in Gerrit where we rely on "Submit".

The only way to stop Maintainers+ from pushing is via protected branch permissions on each repo (not configurable at the group level):

Protected branch permissions

Change 957329 had a related patch set uploaded (by Ahmon Dancy; author: Ahmon Dancy):

[operations/puppet@production] Sync ldap/ops into GitLab repos/sre group

https://gerrit.wikimedia.org/r/957329

The initial sync of ldap/ops into https://gitlab.wikimedia.org/groups/repos/sre has been completed. Periodic syncs will occur once https://gerrit.wikimedia.org/r/c/operations/puppet/+/957329 is merged.

dancy triaged this task as Low priority.Sep 13 2023, 6:12 PM

Change 957329 merged by RLazarus:

[operations/puppet@production] Sync ldap/ops into GitLab repos/sre group

https://gerrit.wikimedia.org/r/957329

Change 957775 had a related patch set uploaded (by Ahmon Dancy; author: Ahmon Dancy):

[operations/puppet@production] Sync ldap/ops into GitLab repos/sre group (v2)

https://gerrit.wikimedia.org/r/957775

Change 957775 merged by RLazarus:

[operations/puppet@production] Sync ldap/ops into GitLab repos/sre group (v2)

https://gerrit.wikimedia.org/r/957775

dancy updated the task description. (Show Details)