Denormalize project permissions to merge request in Elasticsearch
What does this MR do?
As part of our efforts to avoid using joins in Elasticsearch queries &2054 we need to "denormalize" (copy them into child docs) the permission related fields needed for searching.
In order to allow us to search for merge requests without joining to the
project we need to store the merge_requests_access_level
as well as
the visibility_level
of the project on the merge request record. This MR starts adding these new fields when an MR is indexed but it does not backfill for the existing projects and as such we can't yet use these fields in searches. The sooner we start setting these fields, however, the less we'll have to backfill later. The overall steps needed before we can finish this work is described in &5468 (closed) and this is the first step.
This MR also introduces an extra field project_id
for the merge
request in Elasticsearch which is redundant since we already have
target_project_id
and project_id
is just aliased to this value but
adding it to Elasticsearch will make the query logic simpler to share
across all document types. Previously they were all able to consistently
join to a parent "project" so it helps when changing the code they all
have a field called project_id
.
As well as saving these new fields with the merge requests we need to also update these fields when they change which we also do in this MR. We need to track updates in a few places:
- When a
ProjectFeature
record is changed (this is wheremerge_requests_access_level
lives) - When a
Project
is updated (this is wherevisibility_level
lives) - When a project is moved to another group. This logic was already
implemented generically to delegate to
Project
but we update the spec for this just to be safe.
The change to index these new fields also introduced an N+1 query which
required an update to MergeRequestClassProxy
to preload these new fields
we're now setting in Elasticsearch.
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _____.
-
- [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Related to #322786 (closed)