diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index a3d1716f1d0..c3abc716a78 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -528,8 +528,8 @@ .frontend:rules:default-frontend-jobs: rules: - - <<: *if-default-refs - changes: *code-backstage-patterns + - <<: *if-merge-request-labels-run-all-rspec + - changes: *code-backstage-patterns .frontend:rules:default-frontend-jobs-as-if-foss: rules: diff --git a/app/models/concerns/cascading_namespace_setting_attribute.rb b/app/models/concerns/cascading_namespace_setting_attribute.rb index e58e5ddc966..c70c6dca105 100644 --- a/app/models/concerns/cascading_namespace_setting_attribute.rb +++ b/app/models/concerns/cascading_namespace_setting_attribute.rb @@ -176,10 +176,10 @@ module CascadingNamespaceSettingAttribute private def locked_value(attribute) + return application_setting_value(attribute) if locked_by_application_setting?(attribute) + ancestor = locked_ancestor(attribute) return ancestor.read_attribute(attribute) if ancestor - - Gitlab::CurrentSettings.public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend end def locked_ancestor(attribute) diff --git a/db/post_migrate/20211022113000_drop_index_security_ci_builds_on_name_and_id_parser_features_broken.rb b/db/post_migrate/20211022113000_drop_index_security_ci_builds_on_name_and_id_parser_features_broken.rb new file mode 100644 index 00000000000..9ee1b10a072 --- /dev/null +++ b/db/post_migrate/20211022113000_drop_index_security_ci_builds_on_name_and_id_parser_features_broken.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class DropIndexSecurityCiBuildsOnNameAndIdParserFeaturesBroken < Gitlab::Database::Migration[1.0] + TABLE = "ci_builds" + COLUMNS = %i[name id] + INDEX_NAME = "index_security_ci_builds_on_name_and_id_parser_features_broken" + CONSTRAINTS = "(name::text = ANY (ARRAY['container_scanning'::character varying::text, + 'dast'::character varying::text, + 'dependency_scanning'::character varying::text, + 'license_management'::character varying::text, + 'sast'::character varying::text, + 'secret_detection'::character varying::text, + 'coverage_fuzzing'::character varying::text, + 'license_scanning'::character varying::text]) + ) AND type::text = 'Ci::Build'::text" + + disable_ddl_transaction! + + def up + remove_concurrent_index(TABLE, COLUMNS, name: INDEX_NAME, where: CONSTRAINTS) + end + + def down + add_concurrent_index(TABLE, COLUMNS, name: INDEX_NAME, where: CONSTRAINTS) + end +end diff --git a/db/schema_migrations/20211022113000 b/db/schema_migrations/20211022113000 new file mode 100644 index 00000000000..85044f34206 --- /dev/null +++ b/db/schema_migrations/20211022113000 @@ -0,0 +1 @@ +ded528d0485951403f1c5af804f40b1c0a7c71a0dc67f24fadbc357a45fb1a19 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 202e79a8a57..54aff7afd13 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -26564,8 +26564,6 @@ CREATE INDEX index_secure_ci_builds_on_user_id_name_created_at ON ci_builds USIN CREATE INDEX index_security_ci_builds_on_name_and_id_parser_features ON ci_builds USING btree (name, id) WHERE (((name)::text = ANY (ARRAY[('container_scanning'::character varying)::text, ('dast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('license_management'::character varying)::text, ('sast'::character varying)::text, ('secret_detection'::character varying)::text, ('coverage_fuzzing'::character varying)::text, ('license_scanning'::character varying)::text])) AND ((type)::text = 'Ci::Build'::text)); -CREATE INDEX index_security_ci_builds_on_name_and_id_parser_features_broken ON ci_builds USING btree (name, id) WHERE (((name)::text = ANY (ARRAY[('container_scanning'::character varying)::text, ('dast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('license_management'::character varying)::text, ('sast'::character varying)::text, ('secret_detection'::character varying)::text, ('coverage_fuzzing'::character varying)::text, ('license_scanning'::character varying)::text])) AND ((type)::text = 'Ci::Build'::text)); - CREATE INDEX index_security_findings_on_confidence ON security_findings USING btree (confidence); CREATE INDEX index_security_findings_on_project_fingerprint ON security_findings USING btree (project_fingerprint); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fb1ee883be6..d58c185e3c6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10301,6 +10301,9 @@ msgstr "" msgid "DAST Scans" msgstr "" +msgid "DAST profile not found: %{name}" +msgstr "" + msgid "DNS" msgstr "" @@ -18305,6 +18308,9 @@ msgstr "" msgid "Insufficient permissions" msgstr "" +msgid "Insufficient permissions for dast_configuration keyword" +msgstr "" + msgid "Integration" msgstr "" @@ -26065,12 +26071,18 @@ msgstr "" msgid "Profile Settings" msgstr "" +msgid "Profile failed to delete" +msgstr "" + msgid "Profile image guideline" msgstr "" msgid "Profile page:" msgstr "" +msgid "Profile parameter missing" +msgstr "" + msgid "ProfileSession|on" msgstr "" @@ -38916,6 +38928,9 @@ msgstr "" msgid "You are not authorized to perform this action" msgstr "" +msgid "You are not authorized to update this profile" +msgstr "" + msgid "You are not authorized to update this scanner profile" msgstr "" diff --git a/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb index e8f2b18e662..6be6e3f048f 100644 --- a/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb +++ b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb @@ -136,6 +136,21 @@ RSpec.describe NamespaceSetting, 'CascadingNamespaceSettingAttribute' do .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/) end end + + context 'when parent locked the attribute then the application settings locks it' do + before do + subgroup_settings.update!(delayed_project_removal: true) + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + stub_application_setting(lock_delayed_project_removal: true, delayed_project_removal: true) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'returns the application setting value' do + expect(delayed_project_removal).to eq(true) + end + end end describe '#delayed_project_removal?' do diff --git a/spec/support/database/cross-database-modification-allowlist.yml b/spec/support/database/cross-database-modification-allowlist.yml index d341616635b..bff5507c9d3 100644 --- a/spec/support/database/cross-database-modification-allowlist.yml +++ b/spec/support/database/cross-database-modification-allowlist.yml @@ -1,60 +1,29 @@ -- "./ee/spec/controllers/ee/projects/jobs_controller_spec.rb" -- "./ee/spec/controllers/projects/merge_requests_controller_spec.rb" - "./ee/spec/controllers/projects/settings/access_tokens_controller_spec.rb" -- "./ee/spec/controllers/projects/subscriptions_controller_spec.rb" -- "./ee/spec/features/groups/analytics/cycle_analytics/filters_and_data_spec.rb" -- "./ee/spec/features/projects/services/user_activates_github_spec.rb" -- "./ee/spec/frontend/fixtures/analytics/value_streams.rb" -- "./ee/spec/frontend/fixtures/analytics/value_streams_code_stage.rb" -- "./ee/spec/frontend/fixtures/analytics/value_streams_issue_stage.rb" -- "./ee/spec/frontend/fixtures/analytics/value_streams_plan_stage.rb" -- "./ee/spec/frontend/fixtures/analytics/value_streams_review_stage.rb" -- "./ee/spec/frontend/fixtures/analytics/value_streams_staging_stage.rb" -- "./ee/spec/frontend/fixtures/analytics/value_streams_test_stage.rb" - "./ee/spec/graphql/mutations/dast/profiles/create_spec.rb" - "./ee/spec/graphql/mutations/dast/profiles/run_spec.rb" - "./ee/spec/graphql/mutations/dast/profiles/update_spec.rb" - "./ee/spec/graphql/mutations/dast_on_demand_scans/create_spec.rb" -- "./ee/spec/graphql/mutations/merge_requests/accept_spec.rb" -- "./ee/spec/lib/analytics/devops_adoption/snapshot_calculator_spec.rb" - "./ee/spec/lib/gitlab/ci/templates/Jobs/dast_default_branch_gitlab_ci_yaml_spec.rb" - "./ee/spec/mailers/notify_spec.rb" -- "./ee/spec/models/analytics/cycle_analytics/group_level_spec.rb" -- "./ee/spec/models/approval_merge_request_rule_spec.rb" - "./ee/spec/models/ci/bridge_spec.rb" - "./ee/spec/models/ci/build_spec.rb" - "./ee/spec/models/ci/minutes/additional_pack_spec.rb" -- "./ee/spec/models/ci/pipeline_spec.rb" -- "./ee/spec/models/concerns/approval_rule_like_spec.rb" -- "./ee/spec/models/concerns/approver_migrate_hook_spec.rb" -- "./ee/spec/models/dora/daily_metrics_spec.rb" - "./ee/spec/models/ee/ci/job_artifact_spec.rb" -- "./ee/spec/models/ee/ci/runner_spec.rb" - "./ee/spec/models/group_member_spec.rb" -- "./ee/spec/models/merge_request_spec.rb" -- "./ee/spec/models/project_spec.rb" - "./ee/spec/replicators/geo/pipeline_artifact_replicator_spec.rb" - "./ee/spec/replicators/geo/terraform_state_version_replicator_spec.rb" - "./ee/spec/requests/api/graphql/mutations/dast/profiles/create_spec.rb" - "./ee/spec/requests/api/graphql/mutations/dast/profiles/run_spec.rb" - "./ee/spec/requests/api/graphql/mutations/dast/profiles/update_spec.rb" - "./ee/spec/requests/api/graphql/mutations/dast_on_demand_scans/create_spec.rb" -- "./ee/spec/requests/api/graphql/project/pipeline/dast_profile_spec.rb" -- "./ee/spec/requests/api/graphql/project/pipelines/dast_profile_spec.rb" - "./ee/spec/services/app_sec/dast/profiles/create_service_spec.rb" - "./ee/spec/services/app_sec/dast/profiles/update_service_spec.rb" - "./ee/spec/services/app_sec/dast/scans/create_service_spec.rb" - "./ee/spec/services/app_sec/dast/scans/run_service_spec.rb" -- "./ee/spec/services/ci/compare_license_scanning_reports_service_spec.rb" -- "./ee/spec/services/ci/compare_metrics_reports_service_spec.rb" - "./ee/spec/services/ci/create_pipeline_service/dast_configuration_spec.rb" - "./ee/spec/services/ci/destroy_pipeline_service_spec.rb" -- "./ee/spec/services/ci/minutes/track_live_consumption_service_spec.rb" -- "./ee/spec/services/ci/minutes/update_build_minutes_service_spec.rb" - "./ee/spec/services/ci/retry_build_service_spec.rb" - "./ee/spec/services/ci/subscribe_bridge_service_spec.rb" -- "./ee/spec/services/ci/sync_reports_to_approval_rules_service_spec.rb" -- "./ee/spec/services/ci/trigger_downstream_subscription_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" @@ -62,39 +31,18 @@ - "./ee/spec/services/projects/transfer_service_spec.rb" - "./ee/spec/services/security/security_orchestration_policies/rule_schedule_service_spec.rb" - "./ee/spec/services/vulnerability_feedback/create_service_spec.rb" -- "./ee/spec/workers/refresh_license_compliance_checks_worker_spec.rb" - "./spec/controllers/abuse_reports_controller_spec.rb" - "./spec/controllers/admin/spam_logs_controller_spec.rb" - "./spec/controllers/admin/users_controller_spec.rb" - "./spec/controllers/omniauth_callbacks_controller_spec.rb" - "./spec/controllers/projects/issues_controller_spec.rb" -- "./spec/controllers/projects/jobs_controller_spec.rb" -- "./spec/controllers/projects/merge_requests/content_controller_spec.rb" -- "./spec/controllers/projects/merge_requests_controller_spec.rb" -- "./spec/controllers/projects/pipelines/tests_controller_spec.rb" - "./spec/controllers/projects/pipelines_controller_spec.rb" - "./spec/controllers/projects/settings/access_tokens_controller_spec.rb" -- "./spec/factories_spec.rb" -- "./spec/features/cycle_analytics_spec.rb" -- "./spec/features/groups/packages_spec.rb" - "./spec/features/issues/issue_detail_spec.rb" -- "./spec/features/merge_request/user_sees_deployment_widget_spec.rb" -- "./spec/features/merge_request/user_sees_merge_widget_spec.rb" -- "./spec/features/projects/jobs_spec.rb" -- "./spec/features/projects/packages_spec.rb" -- "./spec/features/projects/pipeline_schedules_spec.rb" - "./spec/features/projects/pipelines/pipeline_spec.rb" - "./spec/features/signed_commits_spec.rb" -- "./spec/finders/ci/pipeline_schedules_finder_spec.rb" -- "./spec/finders/ci/pipelines_finder_spec.rb" -- "./spec/frontend/fixtures/analytics.rb" -- "./spec/frontend/fixtures/pipeline_schedules.rb" -- "./spec/frontend/fixtures/pipelines.rb" -- "./spec/graphql/mutations/merge_requests/accept_spec.rb" -- "./spec/graphql/resolvers/ci/test_report_summary_resolver_spec.rb" - "./spec/helpers/issuables_helper_spec.rb" - "./spec/lib/gitlab/auth_spec.rb" -- "./spec/lib/gitlab/ci/config_spec.rb" - "./spec/lib/gitlab/ci/pipeline/chain/create_spec.rb" - "./spec/lib/gitlab/ci/pipeline/chain/seed_block_spec.rb" - "./spec/lib/gitlab/ci/pipeline/seed/build_spec.rb" @@ -103,107 +51,56 @@ - "./spec/lib/gitlab/ci/templates/Jobs/deploy_gitlab_ci_yaml_spec.rb" - "./spec/lib/gitlab/ci/templates/auto_devops_gitlab_ci_yaml_spec.rb" - "./spec/lib/gitlab/ci/templates/managed_cluster_applications_gitlab_ci_yaml_spec.rb" -- "./spec/lib/gitlab/data_builder/pipeline_spec.rb" - "./spec/lib/gitlab/email/handler/create_issue_handler_spec.rb" - "./spec/lib/gitlab/email/handler/create_merge_request_handler_spec.rb" - "./spec/lib/gitlab/email/handler/create_note_handler_spec.rb" - "./spec/lib/gitlab/email/handler/create_note_on_issuable_handler_spec.rb" -- "./spec/lib/gitlab/usage_data_spec.rb" - "./spec/lib/peek/views/active_record_spec.rb" -- "./spec/mailers/emails/pipelines_spec.rb" -- "./spec/models/ci/bridge_spec.rb" - "./spec/models/ci/build_need_spec.rb" -- "./spec/models/ci/build_spec.rb" - "./spec/models/ci/build_trace_chunk_spec.rb" - "./spec/models/ci/group_variable_spec.rb" -- "./spec/models/ci/instance_variable_spec.rb" - "./spec/models/ci/job_artifact_spec.rb" - "./spec/models/ci/job_variable_spec.rb" -- "./spec/models/ci/legacy_stage_spec.rb" -- "./spec/models/ci/pipeline_schedule_spec.rb" - "./spec/models/ci/pipeline_spec.rb" -- "./spec/models/ci/runner_namespace_spec.rb" -- "./spec/models/ci/runner_project_spec.rb" - "./spec/models/ci/runner_spec.rb" -- "./spec/models/ci/running_build_spec.rb" - "./spec/models/ci/variable_spec.rb" - "./spec/models/clusters/applications/runner_spec.rb" - "./spec/models/commit_status_spec.rb" - "./spec/models/concerns/batch_destroy_dependent_associations_spec.rb" - "./spec/models/concerns/bulk_insertable_associations_spec.rb" - "./spec/models/concerns/has_environment_scope_spec.rb" -- "./spec/models/concerns/schedulable_spec.rb" - "./spec/models/concerns/token_authenticatable_spec.rb" - "./spec/models/design_management/version_spec.rb" -- "./spec/models/environment_status_spec.rb" - "./spec/models/hooks/system_hook_spec.rb" - "./spec/models/members/project_member_spec.rb" -- "./spec/models/merge_request_spec.rb" -- "./spec/models/project_spec.rb" - "./spec/models/spam_log_spec.rb" - "./spec/models/user_spec.rb" - "./spec/models/user_status_spec.rb" -- "./spec/requests/api/admin/ci/variables_spec.rb" - "./spec/requests/api/ci/pipeline_schedules_spec.rb" - "./spec/requests/api/ci/pipelines_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/commits_spec.rb" -- "./spec/requests/api/graphql/ci/runner_spec.rb" - "./spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb" -- "./spec/requests/api/merge_requests_spec.rb" - "./spec/requests/api/resource_access_tokens_spec.rb" - "./spec/requests/api/users_spec.rb" -- "./spec/requests/projects/cycle_analytics_events_spec.rb" -- "./spec/serializers/ci/downloadable_artifact_entity_spec.rb" -- "./spec/serializers/ci/downloadable_artifact_serializer_spec.rb" -- "./spec/serializers/merge_request_widget_entity_spec.rb" -- "./spec/serializers/test_report_entity_spec.rb" -- "./spec/serializers/test_report_summary_entity_spec.rb" -- "./spec/serializers/test_suite_entity_spec.rb" -- "./spec/serializers/test_suite_summary_entity_spec.rb" -- "./spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb" -- "./spec/services/ci/compare_accessibility_reports_service_spec.rb" -- "./spec/services/ci/compare_codequality_reports_service_spec.rb" -- "./spec/services/ci/compare_reports_base_service_spec.rb" -- "./spec/services/ci/compare_test_reports_service_spec.rb" - "./spec/services/ci/create_pipeline_service/environment_spec.rb" - "./spec/services/ci/create_pipeline_service_spec.rb" - "./spec/services/ci/destroy_pipeline_service_spec.rb" -- "./spec/services/ci/disable_user_pipeline_schedules_service_spec.rb" - "./spec/services/ci/ensure_stage_service_spec.rb" - "./spec/services/ci/expire_pipeline_cache_service_spec.rb" -- "./spec/services/ci/generate_codequality_mr_diff_report_service_spec.rb" -- "./spec/services/ci/generate_coverage_reports_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_artifacts/coverage_report_service_spec.rb" -- "./spec/services/ci/pipeline_artifacts/create_code_quality_mr_diff_report_service_spec.rb" - "./spec/services/ci/pipeline_bridge_status_service_spec.rb" -- "./spec/services/ci/pipeline_schedule_service_spec.rb" - "./spec/services/ci/pipelines/add_job_service_spec.rb" - "./spec/services/ci/retry_build_service_spec.rb" -- "./spec/services/ci/update_instance_variables_service_spec.rb" -- "./spec/services/deployments/update_environment_service_spec.rb" -- "./spec/services/environments/stop_service_spec.rb" - "./spec/services/groups/transfer_service_spec.rb" -- "./spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb" -- "./spec/services/merge_requests/post_merge_service_spec.rb" - "./spec/services/projects/destroy_service_spec.rb" - "./spec/services/projects/overwrite_project_service_spec.rb" - "./spec/services/projects/transfer_service_spec.rb" - "./spec/services/resource_access_tokens/revoke_service_spec.rb" - "./spec/services/users/destroy_service_spec.rb" - "./spec/services/users/reject_service_spec.rb" -- "./spec/views/projects/artifacts/_artifact.html.haml_spec.rb" -- "./spec/views/projects/pipeline_schedules/_pipeline_schedule.html.haml_spec.rb" -- "./spec/views/shared/runners/_runner_details.html.haml_spec.rb" -- "./spec/workers/ci/pipeline_artifacts/create_quality_report_worker_spec.rb" - "./spec/workers/merge_requests/create_pipeline_worker_spec.rb" -- "./spec/workers/pipeline_schedule_worker_spec.rb" - "./spec/workers/remove_expired_members_worker_spec.rb" - "./spec/workers/repository_cleanup_worker_spec.rb" -- "./spec/workers/stage_update_worker_spec.rb" -- "./spec/workers/stuck_merge_jobs_worker_spec.rb" diff --git a/spec/support/database/prevent_cross_database_modification.rb b/spec/support/database/prevent_cross_database_modification.rb index a7d1c3a47a2..229f5026044 100644 --- a/spec/support/database/prevent_cross_database_modification.rb +++ b/spec/support/database/prevent_cross_database_modification.rb @@ -61,6 +61,7 @@ module Database return unless cross_database_context[:enabled] return if connection.pool.instance_of?(ActiveRecord::ConnectionAdapters::NullPool) + return if in_factory_bot_create? database = connection.pool.db_config.name @@ -113,6 +114,15 @@ module Database raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError, message end end + + # We ignore execution in the #create method from FactoryBot + # because it is not representative of real code we run in + # production. There are far too many false positives caused + # by instantiating objects in different `gitlab_schema` in a + # FactoryBot `create`. + def self.in_factory_bot_create? + caller_locations.any? { |l| l.path.end_with?('lib/factory_bot/evaluation.rb') && l.label == 'create' } + end end end diff --git a/spec/support_specs/database/prevent_cross_database_modification_spec.rb b/spec/support_specs/database/prevent_cross_database_modification_spec.rb index c213c461642..e2b3abe426d 100644 --- a/spec/support_specs/database/prevent_cross_database_modification_spec.rb +++ b/spec/support_specs/database/prevent_cross_database_modification_spec.rb @@ -127,6 +127,14 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do expect { run_queries }.to raise_error /Cross-database data modification/ end end + + context 'when the modification is inside a factory save! call' do + let(:runner) { create(:ci_runner, :project, projects: [build(:project)]) } + + it 'does not raise an error' do + runner + end + end end end end @@ -152,14 +160,6 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do end.not_to raise_error end - it 'raises error when complex factories are built referencing both databases' do - expect do - ApplicationRecord.transaction do - create(:ci_pipeline) - end - end.to raise_error /Cross-database data modification/ - end - it 'skips raising error on factory creation' do expect do Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'gitlab-issue') do