commit f93bfad20fd423bbd1cf1bd4b15cf16b0f9056cb
author Edwin Kempin <ekempin@google.com> Tue Apr 06 10:03:52 2021 +0200
committer Edwin Kempin <ekempin@google.com> Mon Apr 12 11:04:38 2021 +0200
tree 7acf533692b43e7fc415ab4cae8c0d4e113dd7ee
parent a59e1edfc441d9736287c7b5a653fbfe6a653cd4

Disallow projects to override inherited list configuration parameters

Some configuration parameters have a list of values and can be specified
multiple times. If such a value was set on project level the complete
inherited list was overridden. It was not possible to just add a value
to the inherited list, but if this was wanted the complete list with the
additional value had to be set on project level.

Example:
If in the parent project 'codeOwners.globalCodeOwner = [A, B]' was set
and a child project set 'codeOwners.globalCodeOwner = [C]', the
inherited global code owners A and B were overridden, and the effective
global code owner was only C.

For most users this behavior was non-obvious, mainly due to the way how
list values are represented in config files. E.g.
'codeOwners.globalCodeOwner = [A, B]' is written as:

  [codeOwners]
    globalCodeOwner = A
    globalCodeOwner = B

and the wrong expectation was that adding in a child project

  [codeOweners]
    globalCodeOwner = C

would result in the global code owners being [A,B,C].

This behavior was not only non-intuitive but also made it hard to
maintain global code owners globally. E.g. if A and B should be global
code owners for all projects, with the above example we would have
needed to repeat them in the child project configuration:

  [codeOweners]
    globalCodeOwner = A
    globalCodeOwner = B
    globalCodeOwner = C

This meant that configuring a new global code owner required adding
it in all config files that set any global code owner, which was
impractical.

It also meant that global code owners couldn't be enforced for all
projects, which can be dangerous if globally operating bots depend on
this configuration. E.g. a project owner removing inherited global code
owners (intended or not) breaks all bots that rely on being global code
owner.

Due to these issues we are changing the way how list configuration
parameters are inherited, so that child projects can only extend the
inherited list, but not override or unset it.

Example:
If a parent project defines

  [codeOwners]
    globalCodeOwner = A
    globalCodeOwner = B

and a child project adds

  [codeOweners]
    globalCodeOwner = C

the effective global code owners are [A,B,C] now.

This behavior matches JGits default behavior for inheriting config
values from a base config (see JGits Config class):
For multi-value / list parameters the the returned list is the list of
values set in the base config (aka parent config) + the list of values
in the current config (aka child config). Note that the values from the
base config (aka parent config) config are always returned first. This
way values from the base config (aka parent config) are overridden when
reading single-value parameters, since for the single-value parameters
always only the last value counts.

To implement this kind of inheritance between projects we simply use the
base config support in JGit (see CodeOwnersPluginConfig). In addition we
must apply the same logic when taking values from gerrit.config into
account.

This change affects all multi-value / list configuration parameters,
which are:
* globalCodeOwner
* exemptedUser
* overrideApproval
* disabledBranch

This means these parameters can no longer be overridden/unset by child
projects.

The behavior for single-value parameters (single string, boolean, enum,
int, long) is not changed. Setting them for a project still overrides
any inherited value for the same parameter.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I1d2576767155c43f49f7b5fdfe582331e743e090