Fix Security feature InstanceSecurityDashboard#all_pipelines cross-joining CI
Discovered in !62092 (closed) we found InstanceSecurityDashboard#all_pipelines
joins between CI and non-CI tables.
But this will not be possible once we move ci__builds_*
to a separate database.
Query
SELECT
"ci_pipelines".*
FROM
"ci_pipelines"
WHERE
"ci_pipelines"."project_id" IN (
SELECT
"users_security_dashboard_projects"."project_id"
FROM
"users_security_dashboard_projects"
WHERE
"users_security_dashboard_projects"."user_id" = 4
AND (
EXISTS(
SELECT
1
FROM
"project_authorizations"
WHERE
"users_security_dashboard_projects"."user_id" = 4
AND "project_authorizations"."user_id" = 4
AND (
users_security_dashboard_projects.project_id = project_authorizations.project_id
)
AND "project_authorizations"."access_level" IN (20, 30, 40, 50)
)
)
AND "users_security_dashboard_projects"."project_id" = 1
)
Spec failure details
Failures:
1) InstanceSecurityDashboard#all_pipelines returns pipelines for the projects with security reports
Failure/Error: expect(subject.all_pipelines).to contain_exactly(pipeline1)
ActiveRecord::StatementInvalid:
PG::UndefinedTable: ERROR: relation "users_security_dashboard_projects" does not exist
LINE 1: ...rs_security_dashboard_projects"."project_id" FROM "users_sec...
^
db_name: gitlabhq_test_ci
# ./ee/spec/models/instance_security_dashboard_spec.rb:26:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:394:in `block (3 levels) in <top (required)>'
# ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
# ./spec/spec_helper.rb:385:in `block (2 levels) in <top (required)>'
# ./spec/spec_helper.rb:381:in `block (3 levels) in <top (required)>'
# ./lib/gitlab/application_context.rb:31:in `with_raw_context'
# ./spec/spec_helper.rb:381:in `block (2 levels) in <top (required)>'
# ------------------
# --- Caused by: ---
# PG::UndefinedTable:
# ERROR: relation "users_security_dashboard_projects" does not exist
# LINE 1: ...rs_security_dashboard_projects"."project_id" FROM "users_sec...
# ^
# ./ee/spec/models/instance_security_dashboard_spec.rb:26:in `block (3 levels) in <top (required)>'
Finished in 22 minutes 16 seconds (files took 1 minute 11.48 seconds to load)
1770 examples, 1 failure, 2 pending
Failed examples:
rspec ./ee/spec/models/instance_security_dashboard_spec.rb:25 # InstanceSecurityDashboard#all_pipelines returns pipelines for the projects with security reports
Options
- Remove it if it is unused
- Replace the logic to use
pluck(:project_id)
instead of a sub-select. This seems reasonably safe this context since there will be more pipelines than projects. Then again it depends on the context this is used in (which is still yet to be tracked down).
Sharding team recommended approach
This method appears to be unused so we'll just remove it. There is an MR already !66503 (merged) .
Edited by Dylan Griffith