Resolve incorrect connection usage raised in ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb

Summary

ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb specs fail when the sec database is separate.

Further details

Failure:

     Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection::CrossSchemaAccessError:
       The query tried to access ["vulnerabilities", "projects"] (of gitlab_main_cell,gitlab_sec) which is outside of allowed schemas ([:gitlab_internal, :gitlab_sec, :gitlab_shared]) for the current connection 'sec'

It fails because Vulnerabilities::BulkDismissService#dismiss triggers a cross-join.

See https://gitlab.com/gitlab-org/gitlab/-/blob/f223f200cd82ea52ced3269d260694e30481e472/ee/app/services/vulnerabilities/bulk_dismiss_service.rb#L51

    def dismiss(vulnerabilities)
      vulnerability_attrs = vulnerabilities.with_projects.pluck(:id, :state, :project_id, 'projects.project_namespace_id') # rubocop:disable CodeReuse/ActiveRecord

with_projects calls includes. Also, it currently has an exception to allow a cross-join.

See https://gitlab.com/gitlab-org/gitlab/-/blob/02b025571341b90fb51757da6f38c545e9dfb521/app/models/vulnerability.rb#L9

  scope :with_projects, -> { includes(:project).allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/486246') }

This is related to Resolve cross DB issues in app/models/vulnerabi... (#486246 - closed), and should be solved by addressing that other issue.

Failing specs

failing spec
Failures:

  1) User with admin_vulnerability custom role Mutations::Vulnerabilities::BulkDismiss has access via a custom role
     Failure/Error: raise CrossSchemaAccessError, message
     
     Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection::CrossSchemaAccessError:
       The query tried to access ["vulnerabilities", "projects"] (of gitlab_main_cell,gitlab_sec) which is outside of allowed schemas ([:gitlab_internal, :gitlab_sec, :gitlab_shared]) for the current connection 'sec'
     # ./lib/gitlab/database/query_analyzers/gitlab_schemas_validate_connection.rb:43:in `analyze'
     # ./lib/gitlab/database/query_analyzer.rb:134:in `block in process_sql'
     # ./lib/gitlab/database/query_analyzer.rb:131:in `each'
     # ./lib/gitlab/database/query_analyzer.rb:131:in `process_sql'
     # ./lib/gitlab/database/query_analyzer.rb:74:in `block (2 levels) in hook!'
     # ./lib/gitlab/database/query_analyzer.rb:146:in `with_ignored_recursive_calls'
     # ./lib/gitlab/database/query_analyzer.rb:73:in `block in hook!'
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:111:in `public_send'
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:111:in `block in read_using_load_balancer'
     # ./lib/gitlab/database/load_balancing/load_balancer.rb:65:in `read'
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:110:in `read_using_load_balancer'
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:48:in `select_all'
     # ./ee/app/services/vulnerabilities/bulk_dismiss_service.rb:52:in `dismiss'
     # ./ee/app/services/vulnerabilities/bulk_dismiss_service.rb:20:in `block in execute'
     # ./ee/app/services/vulnerabilities/bulk_dismiss_service.rb:19:in `each'
     # ./ee/app/services/vulnerabilities/bulk_dismiss_service.rb:19:in `each_slice'
     # ./ee/app/services/vulnerabilities/bulk_dismiss_service.rb:19:in `each'
     # ./ee/app/services/vulnerabilities/bulk_dismiss_service.rb:19:in `execute'
     # ./ee/app/graphql/mutations/vulnerabilities/bulk_dismiss.rb:40:in `resolve'
     # ./lib/gitlab/graphql/present/field_extension.rb:18:in `resolve'
     # ./lib/gitlab/graphql/calls_gitaly/field_extension.rb:15:in `resolve'
     # ./lib/gitlab/graphql/tracers/instrumentation_tracer.rb:19:in `execute_multiplex'
     # ./app/graphql/gitlab_schema.rb:44:in `multiplex'
     # ./app/controllers/graphql_controller.rb:223:in `execute_query'
     # ./app/controllers/graphql_controller.rb:67:in `execute'
     # ./lib/gitlab/auth/current_user_mode.rb:44:in `bypass_session!'
     # ./app/controllers/concerns/sessionless_authentication.rb:43:in `sessionless_bypass_admin_mode!'
     # ./lib/gitlab/ip_address_state.rb:11:in `with'
     # ./ee/app/controllers/ee/application_controller.rb:45:in `set_current_ip_address'
     # ./app/controllers/application_controller.rb:506:in `set_current_admin'
     # ./lib/gitlab/i18n.rb:114:in `with_locale'
     # ./app/controllers/application_controller.rb:489:in `set_locale'
     # ./app/controllers/application_controller.rb:480:in `set_current_context'
     # ./lib/gitlab/middleware/action_controller_static_context.rb:23:in `call'
     # ./lib/gitlab/middleware/sidekiq_shard_awareness_validation.rb:20:in `block in call'
     # ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
     # ./lib/gitlab/middleware/sidekiq_shard_awareness_validation.rb:20:in `call'
     # ./lib/gitlab/middleware/memory_report.rb:13:in `call'
     # ./lib/gitlab/middleware/speedscope.rb:13:in `call'
     # ./lib/gitlab/query_limiting/middleware.rb:17:in `block in call'
     # ./lib/gitlab/query_limiting/transaction.rb:48:in `run'
     # ./lib/gitlab/query_limiting/middleware.rb:16:in `call'
     # ./lib/gitlab/database/load_balancing/rack_middleware.rb:23:in `call'
     # ./lib/gitlab/middleware/go.rb:21:in `call'
     # ./lib/gitlab/etag_caching/middleware.rb:21:in `call'
     # ./lib/gitlab/middleware/query_analyzer.rb:11:in `block in call'
     # ./lib/gitlab/database/query_analyzer.rb:83:in `within'
     # ./lib/gitlab/middleware/query_analyzer.rb:11:in `call'
     # ./lib/ci/job_token/middleware.rb:11:in `call'
     # ./lib/gitlab/middleware/multipart.rb:173:in `call'
     # ./lib/gitlab/middleware/read_only/controller.rb:50:in `call'
     # ./lib/gitlab/middleware/read_only.rb:18:in `call'
     # ./lib/gitlab/middleware/unauthenticated_session_expiry.rb:18:in `call'
     # ./lib/gitlab/middleware/strip_cookies.rb:29:in `call'
     # ./lib/gitlab/middleware/same_site_cookies.rb:27:in `call'
     # ./lib/gitlab/middleware/path_traversal_check.rb:40:in `call'
     # ./lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call'
     # ./lib/gitlab/middleware/basic_health_check.rb:25:in `call'
     # ./lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call'
     # ./lib/gitlab/middleware/request_context.rb:15:in `call'
     # ./lib/gitlab/middleware/webhook_recursion_detection.rb:15:in `call'
     # ./config/initializers/fix_local_cache_middleware.rb:11:in `call'
     # ./lib/gitlab/middleware/compressed_json.rb:44:in `call'
     # ./lib/gitlab/middleware/static.rb:11:in `call'
     # ./lib/gitlab/webpack/dev_server_middleware.rb:34:in `perform_request'
     # ./lib/gitlab/testing/clear_process_memory_cache_middleware.rb:13:in `call'
     # ./lib/gitlab/testing/request_inspector_middleware.rb:35:in `call'
     # ./lib/gitlab/testing/robots_blocker_middleware.rb:30:in `call'
     # ./lib/gitlab/testing/request_blocker_middleware.rb:47:in `call'
     # ./lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:19:in `call'
     # ./lib/gitlab/middleware/sidekiq_web_static.rb:20:in `call'
     # ./lib/gitlab/metrics/requests_rack_middleware.rb:79:in `call'
     # ./spec/support/helpers/graphql_helpers.rb:512:in `post_graphql'
     # ./spec/support/helpers/graphql_helpers.rb:533:in `post_graphql_mutation'
     # ./ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb:166:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:474:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
     # ./spec/spec_helper.rb:473:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:468:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:459:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:455:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:94:in `with_raw_context'
     # ./spec/spec_helper.rb:455:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:426:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/ci/config/feature_flags.rb:38:in `ensure_correct_usage'
     # ./spec/spec_helper.rb:425:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:275:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'

Finished in 53.19 seconds (files took 11.96 seconds to load)
14 examples, 1 failure

Failed examples:

rspec ./ee/spec/requests/custom_roles/admin_vulnerability/request_spec.rb:165 # User with admin_vulnerability custom role Mutations::Vulnerabilities::BulkDismiss has access via a custom role

Proposal

Change Vulnerability#with_projects scope, and use preload instead of includes.

That doesn't seem sufficient to solve the problem though. See #501582 (comment 2219514379)

Edited Nov 20, 2024 by Fabien Catteau
Assignee Loading
Time tracking Loading