Skip to content

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:

  1. When a ProjectFeature record is changed (this is where merge_requests_access_level lives)
  2. When a Project is updated (this is where visibility_level lives)
  3. 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

Availability and Testing

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)

Edited by Dylan Griffith

Merge request reports