Jump to content

GitLab/GitLab SRE Requirements

From Wikitech

This page collects the requirements and best practices for the Wikimedia SRE team for the usage of GitLab. Most requirements result from the current CI Gerrit and the established workflows.

tldr:

Implemented:

  • 2FA for /repos/sre group
  • sync all SREs from ops LDAP group to /repos/sre
  • Trusted Runners for production CI jobs
  • A shared understanding of responsibilities of assignee, reviewer and maintainer
  • gerritlab can be used for stacked changes within the same repository
  • Merge requests can be linked to Phabricator tasks
  • Standardization for service image builds (blubber): kokkuri
  • Standardization for Debian package builds: wmf-debci

Open/missing (blockers):

  • SSH keys are not cross-checked
  • Depending code changes can not be uploaded properly (partly a premium feature)
  • Depending code changes can't be execute in CI properly (needs 3rd party tooling like zuul)
  • Standardization for production image builds (Docker)

Open/missing (annoyances):

  • Multiple reviewers (is a premium feature)
    • Can be worked around with bot assignments but will extend the review time for changes with multiple independent reviewers
  • Merge request approvals have no meaning (approval rules is a premium feature)
  • Proper links in GitLab to linked Phabricator tasks are missing

Security

2FA

All SREs and members with access to SRE repos must use two-factor authentication. This can be enforced by setting the flag "All users in this group must set up two-factor authentication" in https://gitlab.wikimedia.org/admin/groups/repos/sre/edit. See Add_two-factor_authentication.

Currently this options is enabled in SRE group by it's parent group /repos.

See also https://phabricator.wikimedia.org/T347295 and https://phabricator.wikimedia.org/T347295

LDAP group management

All SREs must be managed with the LDAP group ops. The GitLab group /repos/sre is used to manage and authorize all SRE user accounts in GitLab. This group is synced with the LDAP group ops.

See also https://phabricator.wikimedia.org/T343035

SSH Key policy

If SSH is used for git access, all SREs must use a non-production SSH Key.

Currently there is NO cross-validation of duplicate SSH keys.

See also instructions Add_an_SSH_key.

Code contribution workflows

Basic contribution workflows are described in Workflows#Making_a_GitLab_Merge_Request. This section describes more advanced use cases relevant for SREs.

Depending code changes

Some code changes might require multiple changes which depend on each other. So for example multiple subsequent changes to the Puppet repository or changes to Mediawiki and extensions. In Gerrit this changes can either be submitted as stacked changes or, if changes are required in multiple repositories as changes on the same topic. Furthermore for CI Zuuls Depends-On is a feature that enables cross-repo dependencies.

In GitLab there is no direct translation to this concept. To create depending changes within the same repository multiple branches can be used. So to add feature a and a depending feature b to the main branch the following set of merge requests could be used: MAIN < feature_branch_a (MR) < feature_branch_b (MR). However this requires some attention when opening the MR to make sure the correct destination branch is used.

gerritlab

There is a tool gerritlab which automates the workflow of stacked changes in GitLab. The usage is quite similar to Gerrit when (then git lab is used instead of git review). Although gerritlab neeeds manual configuration and a dedicated API token it's the recommended way to create stacked MRs currently.

See also https://phabricator.wikimedia.org/T300819.

Depends-On

Zuuls Depends-On is a feature that enables cross-repo dependencies for CI jobs. This allows to execute CI jobs for a set of depending code changes. This is NOT supported in the default GitLab CI and gitlab-runner model. There are some GitLab features like Merge request dependencies which cover some uses cases. However this is NOT supported in GitLab community edition (premium only). See https://phabricator.wikimedia.org/T349872.

So to allow CI to work with multiple depending code changes additional tooling is needed (which is most probably a newer version of Zuul https://phabricator.wikimedia.org/T350288). This GitLab Zuul integration is still work in progress.

Code review workflows

Basic code review workflows are described in Code_review and https://docs.gitlab.com/ee/development/code_review.html.

Assignee

"A assignee should be the the author or directly responsible individual (DRI)", see the-responsibility-of-the-merge-request-author. Most of the time this is the author of the merge request.

To assign a merge request use the /assign @user quick action in a text area in a merge request, or https://docs.gitlab.com/ee/user/project/merge_requests/#assign-a-user-to-a-merge-request.

Multiple assignees are NOT supported in GitLab community edition (premium only).

Reviewer

"A reviewer is responsible for reviewing the specifics of the chosen solution", see the-responsibility-of-the-reviewer.

To request a review for a merge request use the /assign_reviewer @user quick action in a text area in a merge request, or https://docs.gitlab.com/ee/user/project/merge_requests/reviews/index.html#request-a-review.

Workflows in WMF require getting reviews from multiple subject matter experts. In Gerrit one simply adds multiple users to the "Reviewer" field. However multiple reviewers are NOT supported in GitLab community edition (premium only).

The documentation in Mediawiki suggests to get a review from multiple people "you can leave a comment in the merge request and @mention each user you'd like to review. It's also possible to comment inline, to ask a reviewer to comment on a specific part of the patch". See Code_review#Requesting_reviews.

Similar to Gerrit is it possible to add a comment/review either as a comment on the merge request or inline in the code. Depending on the use-case it can make sense to link to certain lines in the code. Open the "Changes" tab in the merge request and add either a inline comment or getting the link to the code line for referencing the file in a comment.

Maintainer

"A maintainer is responsible for the overall health, quality, and consistency of the GitLab codebase, across domains and product areas", see the-responsibility-of-the-maintainer. The maintainer role is a special, elevated permission users can get on a per-project or pre-group level. See also https://docs.gitlab.com/ee/user/permissions.html. All SREs have the highest permission ("owner") which includes the maintainer permission.

Being maintainer also allows merging the actual merge request. So beside getting a reviewer a merge request also needs attention from a maintainer to get merged.

Approving or disapproving a merge request

After review a merge requests can be approved by using the /approve quickaction or clicking the Approve button in the UI. GitLab offers configuration of mandatory merge request approvals as a premium feature: https://docs.gitlab.com/ee/user/project/merge_requests/approvals/#required-approvals. GitLab community edition also supports merge request approvals, but they don't have any impact on the MR or the pipeline and are optional. Setting approval rules is NOT supported in GitLab community edition (premium only).

It should be discussed if merge requests approvals can still be used to signal general approval (like Gerrits "+1") for the merge request even though they don't have any technical binding. More discussion also should happen how Gerrits scoring (-2 to +2) can be translated to Gitlab. This topic was opened already in https://www.mediawiki.org/wiki/GitLab/Gerrit_to_GitLab#Review_scores.

tbd

CI workflows

Trusted Runners

To build artifacts for production CI workers with additional security must be used. This CI workers ("gitlab-runners") are available as Trusted Runners. Trusted Runners reduce the risk of leaking production secrets or compromised builds by previous or parallel, malicious CI jobs.

The runners live in the production data center and must be used for production builds. Access to this Runners has to be requested: Request access to Trusted Runners.

Typical use-cases are container image builds for services (with blubber and kokkuri) or Debian package builds (with wmf-debci).

Standardization for Docker image builds

Currently WMF gitlab-runners can NOT build container images using Dockerfile syntax. See https://phabricator.wikimedia.org/T351792. Some common abstraction should be available to build SREs base-images and production-images.

Standardization for service image builds (blubber)

To build services images with blubber kokkuri can be used. This CI abstraction uses existing ./.pipeline/blubber.yaml manifests to build container images.

Open: still open is how automatic rebuild of all images can be implemented.

Standardization for Debian package Image builds

To build Debian packages wmf-debci can be used. This CI abstraction uses a standard Debian build environment to build packages.

Open: still open is how packages can be synced to the apt repository, see https://phabricator.wikimedia.org/T347004.

Best practices

In Gerrit code changes are linked to Phabricator when the Bug: T123456 stanza is used in the commit message. In GitLab the same stanza can be used to indicate a relation between merge requests and Phabricator tasks. The CodeReviewBot in Phabricator also links opened merge requests automatically in Phabricator.

However proper linking from GitLab to Phabricator does NOT work currently, see open tasks https://phabricator.wikimedia.org/T265617 and https://phabricator.wikimedia.org/T337570.

Using notifications and the ToDo-List

To make sure code reviews are not delayed or stalled, it can help to allow notifications from WMF GitLab instance and use the ToDo-List (which can be compared a bit to the Gerrit dashboard). The ToDo-List under https://gitlab.wikimedia.org/dashboard/todos shows all merge requests which needs attention (for examples form direct pings, cc or reviewer assignments).

tbd


5 tips to get your code reviewed faster