Resolve cross-join in ProjectsGrade.grades_for with an instance

Fabien Catteaurequested to merge
503387-remove-cross-join-from-vulnerabilities-projects_grade-2 into master

What does this MR do and why?

Resolve cross-join in ProjectsGrades.grades_for(vulnerable) when the vulnerable is an InstanceSecurityDashboard.

  • Add a new method that plucks the IDs of all user dashboard projects that are visible and not archived.
  • Change .grades_for to leverage that method, and pass the project IDs to Vulnerabilities::Statistics.for_project.

Note: There are up to 1,000 projects in a dashboard. In production the vast majority of dashboards only have a few projects. The queries has been tested on postgres.ai with hundreds of projects.

References

Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

On an environment where sec is separate, spec for .grades_for with an instance are now successful.

--- rspec_failures_master	2024-12-13 00:40:07.461651303 -0800
+++ rspec_failures_mr	2024-12-13 00:40:37.121367875 -0800
@@ -1,14 +1,11 @@
-29 examples, 8 failures
+27 examples, 5 failures
 
 Failed examples:
 
 rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:60 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is a Group when subgroups are not included when the filter is not given when remove_cross_join_from_vulnerabilities_projects_grade is disabled returns the letter grades for given vulnerable
 rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:85 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is a Group when subgroups are not included when the filter is given when remove_cross_join_from_vulnerabilities_projects_grade is disabled returns the filtered letter grade for given vulnerable
 rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:117 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is a Group when subgroups are included when the filter is not given when remove_cross_join_from_vulnerabilities_projects_grade is disabled returns the letter grades for given vulnerable
-rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:136 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is a Group when subgroups are included when the filter is not given when multiple batches are required when remove_cross_join_from_vulnerabilities_projects_grade is disabled returns the letter grades for given vulnerable
-rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:162 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is a Group when subgroups are included when the filter is given when remove_cross_join_from_vulnerabilities_projects_grade is disabled returns the filtered letter grade for given vulnerable
-rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:196 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerables are groups when remove_cross_join_from_vulnerabilities_projects_grade is disabled in one of the groups returns all letter grades for each vulnerable
-rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:227 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is an InstanceSecurityDashboard when the filter is not given returns the letter grades for given vulnerable
-rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:242 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is an InstanceSecurityDashboard when the filter is given returns the filtered letter grade for given vulnerable
+rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:142 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerable is a Group when subgroups are included when the filter is given when remove_cross_join_from_vulnerabilities_projects_grade is disabled returns the filtered letter grade for given vulnerable
+rspec ./ee/spec/models/vulnerabilities/projects_grade_spec.rb:176 # Vulnerabilities::ProjectsGrade.grades_for when the given vulnerables are groups when remove_cross_join_from_vulnerabilities_projects_grade is disabled in one of the groups returns all letter grades for each vulnerable
 
-[TEST PROF INFO] Time spent in factories: 00:02.268 (21.37% of total time)
+[TEST PROF INFO] Time spent in factories: 00:01.823 (17.27% of total time)

NOTE: Specs for .grades_for with groups and when remove_cross_join_from_vulnerabilities_projects_grade is disabled still fail. That's expected. See Resolve cross-join in ProjectsGrade.grades_for ... (!172720 - merged)

NOTE: The exact lines of the specs no longer match because an unnecessary context has been removed.

console log 
$ git checkout master
$ bundle exec bin/rspec /home/debian/gitlab-development-kit/gitlab/ee/spec/models/vulnerabilities/projects_grade_spec.rb|grep -A 100 failures>rspec_failures_master
$ git checkout 503387-remove-cross-join-from-vulnerabilities-projects_grade-2
$ bundle exec bin/rspec /home/debian/gitlab-development-kit/gitlab/ee/spec/models/vulnerabilities/projects_grade_spec.rb|grep -A 100 failures>rspec_failures_mr
$ git diff --stat master.. ee/spec/
 ee/spec/models/vulnerabilities/projects_grade_spec.rb | 20 --------------------
 1 file changed, 20 deletions(-)

How to set up and validate locally

  • Run the following spec in an environment where sec is separate.

    bundle exec bin/rspec -E InstanceSecurityDashboard ee/spec/models/vulnerabilities/projects_grade_spec.rb

  • Create a personal security dashboard, add projects, and check the Project security status on the right.

SQL queries

What previously was a single JOIN query is now divided in two consecutive queries.

See !175602 (comment 2269010409)

non_archived_project_ids

Query triggered by InstanceSecurityDashboard#non_archived_project_ids:

SELECT 
  "projects"."id" 
FROM 
  "projects" 
  LEFT JOIN project_features ON projects.id = project_features.project_id 
WHERE 
  "projects"."id" IN (
    SELECT 
      "users_security_dashboard_projects"."project_id" 
    FROM 
      "users_security_dashboard_projects" 
    WHERE 
      "users_security_dashboard_projects"."user_id" = 188 
      AND (
        EXISTS(
          SELECT 
            1 
          FROM 
            "project_authorizations" 
          WHERE 
            "users_security_dashboard_projects"."user_id" = 188 
            AND "project_authorizations"."user_id" = 188 
            AND (
              users_security_dashboard_projects.project_id = project_authorizations.project_id
            ) 
            AND "project_authorizations"."access_level" IN (30, 40, 50)
        )
      )
  ) 
  AND (
    "project_features"."security_and_compliance_access_level" IS NULL 
    OR "project_features"."security_and_compliance_access_level" IN (20, 30) 
    OR (
      "project_features"."security_and_compliance_access_level" = 10 
      AND EXISTS (
        SELECT 
          1 
        FROM 
          "project_authorizations" 
        WHERE 
          "project_authorizations"."user_id" = 188 
          AND (
            project_authorizations.project_id = project_features.project_id
          ) 
          AND (
            project_authorizations.access_level >= 10
          )
      )
    )
  ) 
  AND "projects"."archived" = FALSE

See https://console.postgres.ai/gitlab/gitlab-production-main/sessions/34658/commands/107220

Second query

Query triggered by .grades_for with an InstanceSecurityDashboard but no filter:

SELECT 
  "vulnerability_statistics"."letter_grade", 
  array_agg(project_id) project_ids 
FROM 
  "vulnerability_statistics" 
WHERE 
  "vulnerability_statistics"."project_id" IN (86, 87) 
GROUP BY 
  "vulnerability_statistics"."letter_grade"

See https://console.postgres.ai/gitlab/gitlab-production-main/sessions/34607/commands/106970

Query triggered by .grades_for with an InstanceSecurityDashboard but and filter on the letter grade:

SELECT 
  "vulnerability_statistics"."letter_grade", 
  array_agg(project_id) project_ids 
FROM 
  "vulnerability_statistics" 
WHERE 
  "vulnerability_statistics"."project_id" IN (86, 87) 
  AND "vulnerability_statistics"."letter_grade" = 1 
GROUP BY 
  "vulnerability_statistics"."letter_grade"

Related to #513472 (closed)

Edited by Fabien Catteau

Merge request reports