commit c113a315c6e4ca60b98b714eb68017828c94725e
author Edwin Kempin <ekempin@google.com> Mon Aug 12 15:19:17 2024 +0000
committer Edwin Kempin <ekempin@google.com> Wed Aug 14 13:43:51 2024 +0000
tree 2e52794414ed423af99f6d6e1dbd395c23b9991d
parent fe7aaaf7c97f640834c94d170ab120a400d907df

CheckCodeOwner: Replace 'code_owner_config_file_paths' field

The 'code_owner_config_file_paths' field contained the paths of the code
owner config files that assign code ownership to the given email for the
specified path. We are dropping this field now and instead return this
information via a new 'assigns_code_ownership_to_user' field that is
returned for the inspected code owner config files. For this we change
the 'code_owner_configs' field that has been recently added by change
Ib3a57feac to a 'checked_code_owner_configs' field which contains the
code owner config files as CheckedCodeOwnerConfigFileInfos instead of
CodeOwnerConfigFileInfos. CheckedCodeOwnerConfigFileInfo is a wrapper
around CodeOwnerConfigFileInfo that also contains results of checking
the code owner config file, e.g. the new
'assigns_code_ownership_to_user' field.

Before this change:
...
"code_owner_configs": [
{
  "project": "Code-Owners-Test",
  "branch": "refs/heads/master",
  "path": "/foo/bar/OWNERS"
},
{
  "project": "Code-Owners-Test",
  "branch": "refs/heads/master",
  "path": "/foo/OWNERS"
},
{
  "project": "Code-Owners-Test",
  "branch": "refs/heads/master",
  "path": "/OWNERS"
},
"code_owner_config_file_paths": [
  "foo/OWNERS"
]
...

Now:
...
"checked_code_owner_configs": [
{
  "code_owner_config": {
    "project": "Code-Owners-Test",
    "branch": "refs/heads/master",
    "path": "/foo/bar/OWNERS"
  },
  "assigns_code_ownership_to_user": false
},
{
  "code_owner_config": {
    "project": "Code-Owners-Test",
    "branch": "refs/heads/master",
    "path": "/foo/OWNERS"
  },
  "assigns_code_ownership_to_user": true
},
{
  "code_owner_config": {
    "project": "Code-Owners-Test",
    "branch": "refs/heads/master",
    "path": "/OWNERS"
  },
  "assigns_code_ownership_to_user": false
},
...

The new JSON is better as from the name of the old
'code_owner_config_file_paths' field it was not clear that in only
contained the files that assign code ownership to the user, but not as
one could think, all inspected code owner config files.

Also with the new JSON we have the possibility to return further
information per code owner config file in the future (e.g. a
'are_folder_code_owners_ignored' field that informs about whether folder
code owners are being ignored).

Code owner config files are returned in multiple places (when listing
code owners and when checking code ownership), but the new
'assigns_code_ownership_to_user' field should only be set when checking
code ownership. To avoid confusion when this field is being set we do
not want to add it as an optional field to CodeOwnerConfigFileInfo but
we rather add CheckedCodeOwnerConfigFileInfo as a wrapper for
CodeOwnerConfigFileInfo that can also contain results of checking the
code owner config file. This way CodeOwnerConfigFileInfo that is
returned when listing code owners doesn't have the field and
CheckCodeOwnerInfo can use CheckedCodeOwnerConfigFileInfo where this
field is always set.

Having this clearly seperated gets more important if we want to add
further fields for check results for code owner config files in the
future.

One drawback of the old 'code_owner_config_file_paths' field was that it
only considered the inspected code owner config files from the parent
directories, but not the special code owner config file in
refs/meta/config that may declare default code owners for a repository
E.g. for a file '/foo/bar/baz.txt' we inspect the following code owner
config files: '/foo/bar/OWNERS', '/foo/OWNERS', '/OWNERS' and '/OWNERS'
in 'refs/meta/config'. The latter one was not included in the
'code_owner_config_file_paths' field if it assigned code ownership to
the user (because the field only contained a path, but not a branch
name, hence '/OWNERS' in the code branch couldn't be distinguished from
'/OWNERS' in 'refs/meta/config'). With the new 'code_owner_configs'
field, now 'checked_code_owner_configs', we no longer have this problem
as the entries in this field have the full code owner config key
(project, branch, path) and hence the '/OWNERS' file in
'refs/meta/config' is returned there too (if it exists).

Strinctly speaking dropping the 'code_owner_config_file_paths' field and
changing the 'code_owner_configs' field is a backwards incompatible
change, but since the CheckCodeOwner REST endpoint is intended to be
used for debugging only it should be fine to break the API. I checked
our request logs internally and in the last month there were only 9
calls of this REST endpoint, so it's unlikely we'll break any importany
automation if we drop this field.

Bug: Google b/345161989
Change-Id: Ib654acccdf5fcfbbce2f6e4cf893bbf13f4d21bc
Signed-off-by: Edwin Kempin <ekempin@google.com>