diff options
Diffstat (limited to 'spec/support/database')
| -rw-r--r-- | spec/support/database/ci_tables.rb | 22 | ||||
| -rw-r--r-- | spec/support/database/cross-join-allowlist.yml | 196 | ||||
| -rw-r--r-- | spec/support/database/gitlab_schema.rb | 25 | ||||
| -rw-r--r-- | spec/support/database/multiple_databases.rb | 9 | ||||
| -rw-r--r-- | spec/support/database/prevent_cross_database_modification.rb | 10 | ||||
| -rw-r--r-- | spec/support/database/prevent_cross_joins.rb | 57 |
6 files changed, 275 insertions, 44 deletions
diff --git a/spec/support/database/ci_tables.rb b/spec/support/database/ci_tables.rb deleted file mode 100644 index 99fc7ac2501..00000000000 --- a/spec/support/database/ci_tables.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -# This module stores the CI-related database tables which are -# going to be moved to a separate database. -module Database - module CiTables - def self.include?(name) - ci_tables.include?(name) - end - - def self.ci_tables - @@ci_tables ||= Set.new.tap do |tables| # rubocop:disable Style/ClassVars - tables.merge(Ci::ApplicationRecord.descendants.map(&:table_name).compact) - - # It was decided that taggings/tags are best placed with CI - # https://gitlab.com/gitlab-org/gitlab/-/issues/333413 - tables.add('taggings') - tables.add('tags') - end - end - end -end diff --git a/spec/support/database/cross-join-allowlist.yml b/spec/support/database/cross-join-allowlist.yml new file mode 100644 index 00000000000..2b4cfc6773a --- /dev/null +++ b/spec/support/database/cross-join-allowlist.yml @@ -0,0 +1,196 @@ +- "./ee/spec/controllers/operations_controller_spec.rb" +- "./ee/spec/controllers/projects/issues_controller_spec.rb" +- "./ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb" +- "./ee/spec/features/ci/ci_minutes_spec.rb" +- "./ee/spec/features/merge_request/user_merges_immediately_spec.rb" +- "./ee/spec/features/merge_request/user_sees_merge_widget_spec.rb" +- "./ee/spec/features/merge_trains/two_merge_requests_on_train_spec.rb" +- "./ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb" +- "./ee/spec/features/merge_trains/user_adds_to_merge_train_when_pipeline_succeeds_spec.rb" +- "./ee/spec/features/projects/pipelines/pipeline_spec.rb" +- "./ee/spec/features/projects/settings/auto_rollback_spec.rb" +- "./ee/spec/features/projects/settings/pipeline_subscriptions_spec.rb" +- "./ee/spec/features/projects/settings/protected_environments_spec.rb" +- "./ee/spec/finders/ee/namespaces/projects_finder_spec.rb" +- "./ee/spec/finders/group_projects_finder_spec.rb" +- "./ee/spec/finders/security/findings_finder_spec.rb" +- "./ee/spec/graphql/ee/resolvers/namespace_projects_resolver_spec.rb" +- "./ee/spec/lib/analytics/devops_adoption/snapshot_calculator_spec.rb" +- "./ee/spec/lib/ee/gitlab/background_migration/migrate_approver_to_approval_rules_spec.rb" +- "./ee/spec/lib/ee/gitlab/background_migration/migrate_security_scans_spec.rb" +- "./ee/spec/lib/ee/gitlab/background_migration/populate_latest_pipeline_ids_spec.rb" +- "./ee/spec/lib/ee/gitlab/background_migration/populate_resolved_on_default_branch_column_spec.rb" +- "./ee/spec/lib/ee/gitlab/background_migration/populate_uuids_for_security_findings_spec.rb" +- "./ee/spec/lib/ee/gitlab/background_migration/populate_vulnerability_feedback_pipeline_id_spec.rb" +- "./ee/spec/lib/ee/gitlab/usage_data_spec.rb" +- "./ee/spec/migrations/schedule_populate_resolved_on_default_branch_column_spec.rb" +- "./ee/spec/models/ci/build_spec.rb" +- "./ee/spec/models/ci/minutes/project_monthly_usage_spec.rb" +- "./ee/spec/models/ci/pipeline_spec.rb" +- "./ee/spec/models/ee/vulnerability_spec.rb" +- "./ee/spec/models/merge_request_spec.rb" +- "./ee/spec/models/project_spec.rb" +- "./ee/spec/models/security/finding_spec.rb" +- "./ee/spec/models/security/scan_spec.rb" +- "./ee/spec/presenters/ci/pipeline_presenter_spec.rb" +- "./ee/spec/requests/api/ci/minutes_spec.rb" +- "./ee/spec/requests/api/graphql/ci/minutes/usage_spec.rb" +- "./ee/spec/requests/api/graphql/mutations/environments/canary_ingress/update_spec.rb" +- "./ee/spec/requests/api/graphql/mutations/vulnerabilities/create_external_issue_link_spec.rb" +- "./ee/spec/requests/api/graphql/project/pipeline/security_report_summary_spec.rb" +- "./ee/spec/requests/api/graphql/vulnerabilities/location_spec.rb" +- "./ee/spec/requests/api/groups_spec.rb" +- "./ee/spec/requests/api/namespaces_spec.rb" +- "./ee/spec/requests/api/vulnerability_findings_spec.rb" +- "./ee/spec/serializers/dashboard_environment_entity_spec.rb" +- "./ee/spec/serializers/dashboard_environments_serializer_spec.rb" +- "./ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb" +- "./ee/spec/services/ci/create_pipeline_service/runnable_builds_spec.rb" +- "./ee/spec/services/ci/minutes/additional_packs/change_namespace_service_spec.rb" +- "./ee/spec/services/ci/minutes/additional_packs/create_service_spec.rb" +- "./ee/spec/services/ci/minutes/refresh_cached_data_service_spec.rb" +- "./ee/spec/services/ci/process_pipeline_service_spec.rb" +- "./ee/spec/services/ci/trigger_downstream_subscription_service_spec.rb" +- "./ee/spec/services/clear_namespace_shared_runners_minutes_service_spec.rb" +- "./ee/spec/services/deployments/auto_rollback_service_spec.rb" +- "./ee/spec/services/ee/ci/job_artifacts/destroy_all_expired_service_spec.rb" +- "./ee/spec/services/ee/ci/job_artifacts/destroy_batch_service_spec.rb" +- "./ee/spec/services/ee/issues/build_from_vulnerability_service_spec.rb" +- "./ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb" +- "./ee/spec/services/ee/merge_requests/refresh_service_spec.rb" +- "./ee/spec/services/security/report_summary_service_spec.rb" +- "./ee/spec/services/security/vulnerability_counting_service_spec.rb" +- "./ee/spec/workers/scan_security_report_secrets_worker_spec.rb" +- "./ee/spec/workers/security/store_scans_worker_spec.rb" +- "./spec/controllers/admin/runners_controller_spec.rb" +- "./spec/controllers/groups/runners_controller_spec.rb" +- "./spec/controllers/groups/settings/ci_cd_controller_spec.rb" +- "./spec/controllers/projects/logs_controller_spec.rb" +- "./spec/controllers/projects/merge_requests_controller_spec.rb" +- "./spec/controllers/projects/runners_controller_spec.rb" +- "./spec/controllers/projects/serverless/functions_controller_spec.rb" +- "./spec/controllers/projects/settings/ci_cd_controller_spec.rb" +- "./spec/features/admin/admin_runners_spec.rb" +- "./spec/features/groups/settings/ci_cd_spec.rb" +- "./spec/features/ide/user_opens_merge_request_spec.rb" +- "./spec/features/merge_request/user_merges_immediately_spec.rb" +- "./spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb" +- "./spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb" +- "./spec/features/merge_request/user_resolves_wip_mr_spec.rb" +- "./spec/features/merge_request/user_sees_deployment_widget_spec.rb" +- "./spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb" +- "./spec/features/merge_request/user_sees_merge_widget_spec.rb" +- "./spec/features/merge_request/user_sees_mini_pipeline_graph_spec.rb" +- "./spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb" +- "./spec/features/merge_request/user_sees_pipelines_spec.rb" +- "./spec/features/project_group_variables_spec.rb" +- "./spec/features/project_variables_spec.rb" +- "./spec/features/projects/badges/list_spec.rb" +- "./spec/features/projects/environments_pod_logs_spec.rb" +- "./spec/features/projects/infrastructure_registry_spec.rb" +- "./spec/features/projects/jobs_spec.rb" +- "./spec/features/projects/package_files_spec.rb" +- "./spec/features/projects/pipelines/pipeline_spec.rb" +- "./spec/features/projects/pipelines/pipelines_spec.rb" +- "./spec/features/projects/serverless/functions_spec.rb" +- "./spec/features/projects/settings/pipelines_settings_spec.rb" +- "./spec/features/runners_spec.rb" +- "./spec/features/security/project/internal_access_spec.rb" +- "./spec/features/security/project/private_access_spec.rb" +- "./spec/features/security/project/public_access_spec.rb" +- "./spec/features/triggers_spec.rb" +- "./spec/finders/ci/pipelines_finder_spec.rb" +- "./spec/finders/ci/pipelines_for_merge_request_finder_spec.rb" +- "./spec/finders/ci/runners_finder_spec.rb" +- "./spec/finders/clusters/knative_services_finder_spec.rb" +- "./spec/finders/projects/serverless/functions_finder_spec.rb" +- "./spec/frontend/fixtures/runner.rb" +- "./spec/graphql/mutations/ci/runner/delete_spec.rb" +- "./spec/graphql/resolvers/ci/group_runners_resolver_spec.rb" +- "./spec/graphql/resolvers/ci/job_token_scope_resolver_spec.rb" +- "./spec/graphql/resolvers/merge_request_pipelines_resolver_spec.rb" +- "./spec/graphql/types/ci/job_token_scope_type_spec.rb" +- "./spec/helpers/packages_helper_spec.rb" +- "./spec/lib/api/entities/package_spec.rb" +- "./spec/lib/gitlab/background_migration/migrate_legacy_artifacts_spec.rb" +- "./spec/lib/gitlab/prometheus/query_variables_spec.rb" +- "./spec/mailers/emails/pipelines_spec.rb" +- "./spec/migrations/cleanup_legacy_artifact_migration_spec.rb" +- "./spec/migrations/migrate_protected_attribute_to_pending_builds_spec.rb" +- "./spec/migrations/re_schedule_latest_pipeline_id_population_with_all_security_related_artifact_types_spec.rb" +- "./spec/migrations/schedule_migrate_security_scans_spec.rb" +- "./spec/models/ci/build_spec.rb" +- "./spec/models/ci/job_artifact_spec.rb" +- "./spec/models/ci/job_token/scope_spec.rb" +- "./spec/models/ci/pipeline_spec.rb" +- "./spec/models/ci/runner_spec.rb" +- "./spec/models/clusters/applications/runner_spec.rb" +- "./spec/models/deployment_spec.rb" +- "./spec/models/environment_spec.rb" +- "./spec/models/merge_request_spec.rb" +- "./spec/models/project_spec.rb" +- "./spec/models/user_spec.rb" +- "./spec/presenters/ci/build_runner_presenter_spec.rb" +- "./spec/presenters/ci/pipeline_presenter_spec.rb" +- "./spec/presenters/packages/detail/package_presenter_spec.rb" +- "./spec/requests/api/ci/pipelines_spec.rb" +- "./spec/requests/api/ci/runner/jobs_request_post_spec.rb" +- "./spec/requests/api/ci/runner/runners_post_spec.rb" +- "./spec/requests/api/ci/runners_spec.rb" +- "./spec/requests/api/commit_statuses_spec.rb" +- "./spec/requests/api/graphql/group_query_spec.rb" +- "./spec/requests/api/graphql/merge_request/merge_request_spec.rb" +- "./spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb" +- "./spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb" +- "./spec/requests/api/graphql/mutations/environments/canary_ingress/update_spec.rb" +- "./spec/requests/api/graphql/mutations/merge_requests/create_spec.rb" +- "./spec/requests/api/graphql/packages/composer_spec.rb" +- "./spec/requests/api/graphql/packages/conan_spec.rb" +- "./spec/requests/api/graphql/packages/maven_spec.rb" +- "./spec/requests/api/graphql/packages/nuget_spec.rb" +- "./spec/requests/api/graphql/packages/package_spec.rb" +- "./spec/requests/api/graphql/packages/pypi_spec.rb" +- "./spec/requests/api/graphql/project/merge_request/pipelines_spec.rb" +- "./spec/requests/api/graphql/project/merge_request_spec.rb" +- "./spec/requests/api/graphql/project/merge_requests_spec.rb" +- "./spec/requests/api/graphql/project/pipeline_spec.rb" +- "./spec/requests/api/merge_requests_spec.rb" +- "./spec/requests/api/package_files_spec.rb" +- "./spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb" +- "./spec/services/ci/create_pipeline_service/cross_project_pipeline_spec.rb" +- "./spec/services/ci/create_pipeline_service/needs_spec.rb" +- "./spec/services/ci/create_pipeline_service_spec.rb" +- "./spec/services/ci/destroy_pipeline_service_spec.rb" +- "./spec/services/ci/expire_pipeline_cache_service_spec.rb" +- "./spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb" +- "./spec/services/ci/job_artifacts/destroy_associations_service_spec.rb" +- "./spec/services/ci/job_artifacts/destroy_batch_service_spec.rb" +- "./spec/services/ci/pipeline_processing/shared_processing_service.rb" +- "./spec/services/ci/pipeline_processing/shared_processing_service_tests_with_yaml.rb" +- "./spec/services/ci/register_job_service_spec.rb" +- "./spec/services/clusters/applications/prometheus_config_service_spec.rb" +- "./spec/services/deployments/older_deployments_drop_service_spec.rb" +- "./spec/services/environments/auto_stop_service_spec.rb" +- "./spec/services/environments/stop_service_spec.rb" +- "./spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb" +- "./spec/services/merge_requests/create_service_spec.rb" +- "./spec/services/merge_requests/post_merge_service_spec.rb" +- "./spec/services/merge_requests/refresh_service_spec.rb" +- "./spec/support/prometheus/additional_metrics_shared_examples.rb" +- "./spec/support/shared_examples/ci/pipeline_email_shared_examples.rb" +- "./spec/support/shared_examples/features/packages_shared_examples.rb" +- "./spec/support/shared_examples/features/search_settings_shared_examples.rb" +- "./spec/support/shared_examples/features/variable_list_shared_examples.rb" +- "./spec/support/shared_examples/models/concerns/limitable_shared_examples.rb" +- "./spec/support/shared_examples/quick_actions/merge_request/merge_quick_action_shared_examples.rb" +- "./spec/support/shared_examples/requests/api/graphql/packages/group_and_project_packages_list_shared_examples.rb" +- "./spec/support/shared_examples/requests/api/graphql/packages/package_details_shared_examples.rb" +- "./spec/support/shared_examples/requests/api/logging_application_context_shared_examples.rb" +- "./spec/support/shared_examples/requests/api/status_shared_examples.rb" +- "./spec/support/shared_examples/requests/graphql_shared_examples.rb" +- "./spec/support/shared_examples/services/onboarding_progress_shared_examples.rb" +- "./spec/support/shared_examples/services/packages_shared_examples.rb" +- "./spec/support/shared_examples/workers/idempotency_shared_examples.rb" +- "./spec/tasks/gitlab/generate_sample_prometheus_data_spec.rb" +- "./spec/workers/pipeline_process_worker_spec.rb" +- "./spec/workers/pipeline_schedule_worker_spec.rb" diff --git a/spec/support/database/gitlab_schema.rb b/spec/support/database/gitlab_schema.rb new file mode 100644 index 00000000000..fe05fb998e6 --- /dev/null +++ b/spec/support/database/gitlab_schema.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# This module gathes information about table to schema mapping +# to understand table affinity +module Database + module GitlabSchema + def self.table_schemas(tables) + tables.map { |table| table_schema(table) }.to_set + end + + def self.table_schema(name) + tables_to_schema[name] || :undefined + end + + def self.tables_to_schema + @tables_to_schema ||= all_classes_with_schema.to_h do |klass| + [klass.table_name, klass.gitlab_schema] + end + end + + def self.all_classes_with_schema + ActiveRecord::Base.descendants.reject(&:abstract_class?).select(&:gitlab_schema?) # rubocop:disable Database/MultipleDatabases + end + end +end diff --git a/spec/support/database/multiple_databases.rb b/spec/support/database/multiple_databases.rb new file mode 100644 index 00000000000..8ce642a682c --- /dev/null +++ b/spec/support/database/multiple_databases.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Database + module MultipleDatabases + def skip_if_multiple_databases_not_setup + skip 'Skipping because multiple databases not set up' unless Gitlab::Database.has_config?(:ci) + end + end +end diff --git a/spec/support/database/prevent_cross_database_modification.rb b/spec/support/database/prevent_cross_database_modification.rb index 460ee99391b..b4c968e3c41 100644 --- a/spec/support/database/prevent_cross_database_modification.rb +++ b/spec/support/database/prevent_cross_database_modification.rb @@ -74,18 +74,20 @@ module Database return if cross_database_context[:transaction_depth_by_db].values.all?(&:zero?) - tables = PgQuery.parse(sql).dml_tables + parsed_query = PgQuery.parse(sql) + tables = sql.downcase.include?(' for update') ? parsed_query.tables : parsed_query.dml_tables return if tables.empty? cross_database_context[:modified_tables_by_db][database].merge(tables) all_tables = cross_database_context[:modified_tables_by_db].values.map(&:to_a).flatten + schemas = Database::GitlabSchema.table_schemas(all_tables) - unless PreventCrossJoins.only_ci_or_only_main?(all_tables) + if schemas.many? raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError, - "Cross-database data modification queries (CI and Main) were detected within " \ - "a transaction '#{all_tables.join(", ")}' discovered" + "Cross-database data modification of '#{schemas.to_a.join(", ")}' were detected within " \ + "a transaction modifying the '#{all_tables.to_a.join(", ")}'" end end end diff --git a/spec/support/database/prevent_cross_joins.rb b/spec/support/database/prevent_cross_joins.rb index 789721ccd38..4b78aa9014c 100644 --- a/spec/support/database/prevent_cross_joins.rb +++ b/spec/support/database/prevent_cross_joins.rb @@ -11,7 +11,7 @@ # # class User # def ci_owned_runners -# ::Gitlab::Database.allow_cross_joins_across_databases!(url: link-to-issue-url) +# ::Gitlab::Database.allow_cross_joins_across_databases(url: link-to-issue-url) # # ... # end @@ -21,33 +21,43 @@ module Database module PreventCrossJoins CrossJoinAcrossUnsupportedTablesError = Class.new(StandardError) + ALLOW_THREAD_KEY = :allow_cross_joins_across_databases + def self.validate_cross_joins!(sql) - return if Thread.current[:allow_cross_joins_across_databases] + return if Thread.current[ALLOW_THREAD_KEY] + + # Allow spec/support/database_cleaner.rb queries to disable/enable triggers for many tables + # See https://gitlab.com/gitlab-org/gitlab/-/issues/339396 + return if sql.include?("DISABLE TRIGGER") || sql.include?("ENABLE TRIGGER") # PgQuery might fail in some cases due to limited nesting: # https://github.com/pganalyze/pg_query/issues/209 - tables = PgQuery.parse(sql).tables + # + # Also, we disable GC while parsing because of https://github.com/pganalyze/pg_query/issues/226 + begin + GC.disable + tables = PgQuery.parse(sql).tables + ensure + GC.enable + end - unless only_ci_or_only_main?(tables) + schemas = Database::GitlabSchema.table_schemas(tables) + + if schemas.include?(:gitlab_ci) && schemas.include?(:gitlab_main) + Thread.current[:has_cross_join_exception] = true raise CrossJoinAcrossUnsupportedTablesError, - "Unsupported cross-join across '#{tables.join(", ")}' discovered " \ - "when executing query '#{sql}'" + "Unsupported cross-join across '#{tables.join(", ")}' modifying '#{schemas.to_a.join(", ")}' discovered " \ + "when executing query '#{sql}'. Please refer to https://docs.gitlab.com/ee/development/database/multiple_databases.html#removing-joins-between-ci_-and-non-ci_-tables for details on how to resolve this exception." end end - # Returns true if a set includes only CI tables, or includes only non-CI tables - def self.only_ci_or_only_main?(tables) - tables.all? { |table| CiTables.include?(table) } || - tables.none? { |table| CiTables.include?(table) } - end - module SpecHelpers def with_cross_joins_prevented subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') do |event| ::Database::PreventCrossJoins.validate_cross_joins!(event.payload[:sql]) end - Thread.current[:allow_cross_joins_across_databases] = false + Thread.current[ALLOW_THREAD_KEY] = false yield ensure @@ -57,8 +67,12 @@ module Database module GitlabDatabaseMixin def allow_cross_joins_across_databases(url:) - Thread.current[:allow_cross_joins_across_databases] = true - super + old_value = Thread.current[ALLOW_THREAD_KEY] + Thread.current[ALLOW_THREAD_KEY] = true + + yield + ensure + Thread.current[ALLOW_THREAD_KEY] = old_value end end end @@ -67,11 +81,18 @@ end Gitlab::Database.singleton_class.prepend( Database::PreventCrossJoins::GitlabDatabaseMixin) +ALLOW_LIST = Set.new(YAML.load_file(File.join(__dir__, 'cross-join-allowlist.yml'))).freeze + RSpec.configure do |config| config.include(::Database::PreventCrossJoins::SpecHelpers) - # TODO: remove `:prevent_cross_joins` to enable the check by default - config.around(:each, :prevent_cross_joins) do |example| - with_cross_joins_prevented { example.run } + config.around do |example| + Thread.current[:has_cross_join_exception] = false + + if ALLOW_LIST.include?(example.file_path) + example.run + else + with_cross_joins_prevented { example.run } + end end end |