Skip to content

Support for multiple CODEOWNERS sections

Problem to solve

The CODEOWNERS file is powerful because it allows rules to be dynamically applied by path, but it is limited by the fact that:

  • only one rule can match each path
  • all rules are enforced or no rules are enforced

If multiple teams (e.g. security, UX, frontend, backend) want to use code owners then they have to carefully negotiate which paths ping which teams. It is almost impossible to get desired results because each team will want to manage their approval interests independently without worrying about which rules take priority.

More problematically, all code owner rules must be enforced or none. This means that if there are important compliance rules that must be enforced, every rule must be enforced. This means another team that simply wants to be notified and added as optional approvers of relevant merge requests must forgo using code owners or become a required approver.

Further details

For example:

  • the compliance team needs to make sure all changes to the app/payments code is reviewed by Jane
  • there are multiple development teams that work on the payment code, and want to automatically assign code reviews to the correct teams, app/payments/checkout, app/payments/renewal and app/payments/cancellation

With CODEOWNERS this isn't possible since only one rule can match - the most specific rule (last matching rule in the file)

Proposal

Allow multiple section in the CODEOWNERS file, and the most specific rule from each section wins. That way each team can have their own file and not get in the way of each other.

  • there is an implicit group with the name CODEOWNERS for rules not in a named section. Since it's implicit, it will not be shown on the MR widget as a named group. Members of this group will continue to show up next to the path as they do today
  • if there are multiple sections with the same name, including the implied section, these are treated as the same section in file order (since the file is parsed to to bottom)
  • section names are case insensitive (e.g two sections Database and database will be parsed as the same section)
  • section name shown in the interface is taken from the first section name (e.g. if Database is before database, then Database will be shown instead of database)
  • the blob page (example) will continue to show specific users, not section. This may change in the future.
# rules before a defined section are implicitly treated as part of the group CODEOWNERS
README.md  @example-user

[Documentation]
README.md  @gl-docs
ee/docs    @gl-docs
docs       @gl-docs

[Database]
README.md  @gl-database
model/db   @gl-database

If someone changes the README today on the database group would be added as a reviewer. What we want is:

  • example user
  • docs
  • database

to all be added as approvers in distinct sections so that all three can be required.

Mockup merge request widget Mockup protected branches (follow-up #214609 (closed))
mr-widget Screen_Shot_2019-07-25_at_10.16.31_AM

If multiple sections are specified for the same path, they will be shown with the section name in the MR widget

image

Availability & Testing

  • We should ensure that performance is acceptable when there is a large number (up to reasonable limits) of either rules, groups, files, or directories, or a large number of all of those.
  • There are existing end-to-end tests that should be updated to include multiple CODEOWNERS sections to provide additional coverage: gitlab-org/quality/testcases#765
  • run package-and-qa before changes are merged to confirm that existing end-to-end tests still pass.

Customers

Links / references

  • A use-case where we would like to use this for gitlab/gitlab
Edited by Mark Lapierre