Automatic merge of gitlab-org/gitlab master

.rubocop_manual_todo.yml

@@ -2499,7 +2499,6 @@ Database/MultipleDatabases:
     - 'ee/spec/services/ee/merge_requests/update_service_spec.rb'
     - 'lib/backup/database.rb'
     - 'lib/after_commit_queue.rb'
-    - 'lib/api/rubygem_packages.rb'
     - 'lib/backup/manager.rb'
     - 'lib/gitlab/current_settings.rb'
     - 'lib/gitlab/database/load_balancing/load_balancer.rb'
@@ -2535,29 +2534,20 @@ Database/MultipleDatabases:
     - 'lib/gitlab/sherlock/query.rb'
     - 'lib/system_check/orphans/repository_check.rb'
     - 'spec/db/schema_spec.rb'
-    - 'spec/features/admin/dashboard_spec.rb'
     - 'spec/initializers/database_config_spec.rb'
-    - 'spec/initializers/lograge_spec.rb'
     - 'spec/lib/backup/manager_spec.rb'
     - 'spec/lib/gitlab/current_settings_spec.rb'
     - 'spec/lib/gitlab/database_spec.rb'
-    - 'spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb'
-    - 'spec/lib/gitlab/import_export/project/tree_saver_spec.rb'
     - 'spec/lib/gitlab/metrics/subscribers/active_record_spec.rb'
-    - 'spec/lib/gitlab/pagination/keyset/order_spec.rb'
     - 'spec/lib/gitlab/profiler_spec.rb'
-    - 'spec/lib/gitlab/query_limiting/active_support_subscriber_spec.rb'
-    - 'spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb'
     - 'spec/lib/gitlab/usage_data_metrics_spec.rb'
     - 'spec/lib/gitlab/usage_data_queries_spec.rb'
     - 'spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/constraints_spec.rb'
     - 'spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/joins_spec.rb'
     - 'spec/lib/gitlab/usage/metrics/instrumentations/database_metric_spec.rb'
     - 'spec/lib/gitlab/utils/usage_data_spec.rb'
-    - 'spec/models/application_setting_spec.rb'
     - 'spec/models/project_feature_usage_spec.rb'
     - 'spec/models/users_statistics_spec.rb'
-    - 'spec/requests/api/statistics_spec.rb'
     - 'spec/services/users/activity_service_spec.rb'
     - 'spec/support/caching.rb'
     - 'spec/support/gitlab/usage/metrics_instrumentation_shared_examples.rb'
@@ -2569,5 +2559,3 @@ Database/MultipleDatabases:
     - 'spec/support/helpers/usage_data_helpers.rb'
     - 'spec/tasks/gitlab/backup_rake_spec.rb'
     - 'spec/tasks/gitlab/db_rake_spec.rb'
-    - 'spec/workers/analytics/usage_trends/counter_job_worker_spec.rb'
-    - 'spec/workers/users/create_statistics_worker_spec.rb'

ee/app/services/security/store_reports_service.rb

@@ -10,9 +10,9 @@ module Security
     end
 
     def execute
-      store_reports
-      mark_project_as_vulnerable!
       set_latest_pipeline!
+      mark_project_as_vulnerable!
+      store_reports
 
       errors.any? ? error(full_errors) : success
     end

ee/lib/gitlab/ci/reports/dependency_list/vulnerability.rb

@@ -19,7 +19,7 @@ module Gitlab
           end
 
           def hash
-            name.hash ^ severity.hash
+            name.hash ^ severity.hash ^ id.hash ^ url.hash
           end
 
           def to_hash

ee/spec/lib/gitlab/ci/reports/dependency_list/report_spec.rb

@@ -135,7 +135,8 @@ RSpec.describe Gitlab::Ci::Reports::DependencyList::Report do
 
     it 'does not duplicate same vulnerability for dependency' do
       vulnerabilities = [{ name: 'problem', severity: 'high', id: 2, url: 'some_url_2' },
-                         { name: 'problem2', severity: 'medium', id: 4, url: 'some_url_4' }]
+                         { name: 'problem2', severity: 'medium', id: 4, url: 'some_url_4' },
+                         { name: 'problem3', severity: 'medium', id: nil, url: nil }]
 
       dependency[:vulnerabilities] = [vulnerabilities.first]
       with_extra_vuln_from_another_report = dependency.dup.merge(vulnerabilities: vulnerabilities)

ee/spec/services/security/store_reports_service_spec.rb

@@ -41,6 +41,26 @@ RSpec.describe Security::StoreReportsService do
         expect { execute_service_object }.to change { project.reload.vulnerability_statistic&.latest_pipeline_id }.from(nil).to(pipeline.id)
       end
 
+      context 'when the StoreReportService raises an error' do
+        let(:error) { RuntimeError.new('foo') }
+
+        before do
+          allow_next_instance_of(Security::StoreReportService) do |service_object|
+            allow(service_object).to receive(:execute).and_raise(error)
+          end
+        end
+
+        it 'marks the project as vulnerable' do
+          expect { execute_service_object }.to raise_error(error)
+                                           .and change { project.reload.project_setting.has_vulnerabilities }.from(false).to(true)
+        end
+
+        it 'updates the `latest_pipeline_id` attribute of the associated `vulnerability_statistic` record' do
+          expect { execute_service_object }.to raise_error(error)
+                                           .and change { project.reload.vulnerability_statistic&.latest_pipeline_id }.from(nil).to(pipeline.id)
+        end
+      end
+
       context 'when StoreReportService returns an error for a report' do
         let(:reports) { Gitlab::Ci::Reports::Security::Reports.new(pipeline) }
         let(:sast_report) { reports.get_report('sast', sast_artifact) }

lib/api/rubygem_packages.rb

@@ -101,7 +101,7 @@ module API
 
             package_file = nil
 
-            ActiveRecord::Base.transaction do
+            ApplicationRecord.transaction do
               package = ::Packages::CreateTemporaryPackageService.new(
                 user_project, current_user, declared_params.merge(build: current_authenticated_job)
               ).execute(:rubygems, name: ::Packages::Rubygems::TEMPORARY_PACKAGE_NAME)

qa/qa/ee/fixtures/secure_license_files/.gitlab-ci.yml

@@ -2,9 +2,7 @@ include:
   template: License-Scanning.gitlab-ci.yml
 
 license_scanning:
-  tags:
-    - qa
-    - test
+  tags: [secure_license]
   script:
     - echo "Skipped"
   artifacts:

qa/qa/ee/fixtures/secure_premade_reports/.gitlab-ci.yml

@@ -6,9 +6,7 @@ include:
   template: License-Scanning.gitlab-ci.yml
 
 dependency_scanning:
-  tags:
-    - qa
-    - test
+  tags: [secure_report]
   script:
     - echo "Skipped"
   artifacts:
@@ -16,9 +14,7 @@ dependency_scanning:
       dependency_scanning: gl-dependency-scanning-report.json
 
 container_scanning:
-  tags:
-    - qa
-    - test
+  tags: [secure_report]
   only: null # Template defaults to feature branches only
   variables:
     GIT_STRATEGY: fetch # Template defaults to none, which stops fetching the premade report
@@ -29,9 +25,7 @@ container_scanning:
       container_scanning: gl-container-scanning-report.json
 
 sast:
-  tags:
-    - qa
-    - test
+  tags: [secure_report]
   only: null # Template defaults to feature branches only
   script:
     - echo "Skipped"
@@ -40,9 +34,7 @@ sast:
       sast: gl-sast-report.json
 
 dast:
-  tags:
-    - qa
-    - test
+  tags: [secure_report]
   only: null # Template defaults to feature branches only
   script:
     - echo "Skipped"
@@ -51,9 +43,7 @@ dast:
       dast: gl-dast-report.json
 
 license_scanning:
-  tags:
-    - qa
-    - test
+  tags: [secure_report]
   script:
     - echo "Skipped"
   artifacts:

qa/qa/ee/fixtures/secure_sast_enable_from_ui_files/.gitlab-ci.yml

@@ -2,9 +2,7 @@ include:
   template: License-Scanning.gitlab-ci.yml
 
 .sast-analyzer:
-  tags:
-    - qa
-    - test
+  tags: [secure_sast]
   script:
     - echo "Skipped"
   artifacts:
@@ -12,9 +10,7 @@ include:
       sast: gl-sast-report.json
 
 license_scanning:
-  tags:
-    - qa
-    - test
+  tags: [secure_sast]
   script:
     - echo "Skipped"
   artifacts:

qa/qa/specs/features/ee/browser_ui/secure/create_merge_request_with_secure_spec.rb

@@ -30,7 +30,7 @@ module QA
         @runner = Resource::Runner.fabricate! do |runner|
           runner.project = @project
           runner.name = @executor
-          runner.tags = %w[qa test]
+          runner.tags = ['secure_report']
         end
 
         # Push fixture to generate Secure reports

qa/qa/specs/features/ee/browser_ui/secure/enable_sast_from_configuration_spec.rb

@@ -48,7 +48,7 @@ module QA
         Resource::Runner.fabricate! do |runner|
           runner.project = project
           runner.name = "runner-for-#{project.name}"
-          runner.tags = %w[qa test]
+          runner.tags = ['secure_sast']
         end
       end
 

qa/qa/specs/features/ee/browser_ui/secure/license_compliance_spec.rb

@@ -17,7 +17,7 @@ module QA
         @runner = Resource::Runner.fabricate! do |runner|
           runner.project = @project
           runner.name = "runner-for-#{@project.name}"
-          runner.tags = %w[qa test]
+          runner.tags = ['secure_license']
         end
       end
 

qa/qa/specs/features/ee/browser_ui/secure/merge_request_license_widget_spec.rb

@@ -24,7 +24,7 @@ module QA
         @runner = Resource::Runner.fabricate! do |runner|
           runner.project = @project
           runner.name = executor
-          runner.tags = %w[qa test]
+          runner.tags = ['secure_license']
         end
 
         Resource::Repository::ProjectPush.fabricate! do |project_push|

qa/qa/specs/features/ee/browser_ui/secure/project_security_dashboard_spec.rb

@@ -22,7 +22,7 @@ module QA
         @runner = Resource::Runner.fabricate! do |runner|
           runner.project = @project
           runner.name = @executor
-          runner.tags = %w[qa test]
+          runner.tags = ['secure_report']
         end
 
         # Push fixture to generate Secure reports

qa/qa/specs/features/ee/browser_ui/secure/security_reports_spec.rb

@@ -44,7 +44,7 @@ module QA
           @runner = Resource::Runner.fabricate_via_api! do |runner|
             runner.project = @project
             runner.name = "runner-for-#{@project.name}"
-            runner.tags = %w[qa test]
+            runner.tags = ['secure_report']
           end
 
           # Push fixture to generate Secure reports

qa/qa/specs/features/ee/browser_ui/secure/vulnerability_management_spec.rb

@@ -30,7 +30,7 @@ module QA
         @runner = Resource::Runner.fabricate! do |runner|
           runner.project = @project
           runner.name = @executor
-          runner.tags = %w[qa test]
+          runner.tags = ['secure_report']
         end
 
         @source = Resource::Repository::ProjectPush.fabricate! do |push|

spec/features/admin/dashboard_spec.rb

@@ -19,8 +19,8 @@ RSpec.describe 'admin visits dashboard' do
 
       # Make sure the fork_networks & fork_networks reltuples have been updated
       # to get a correct count on postgresql
-      ActiveRecord::Base.connection.execute('ANALYZE fork_networks')
-      ActiveRecord::Base.connection.execute('ANALYZE fork_network_members')
+      ForkNetwork.connection.execute('ANALYZE fork_networks')
+      ForkNetwork.connection.execute('ANALYZE fork_network_members')
 
       visit admin_root_path
 

spec/initializers/lograge_spec.rb

@@ -212,7 +212,7 @@ RSpec.describe 'lograge', type: :request do
       end
 
       before do
-        ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);')
+        ApplicationRecord.connection.execute('SELECT pg_sleep(0.1);')
       end
 
       context 'when RequestStore is enabled', :request_store do

spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb

@@ -190,7 +190,7 @@ RSpec.describe Gitlab::ImportExport::FastHashSerializer do
   end
 
   it 'does not complain about non UTF-8 characters in MR diff files' do
-    ActiveRecord::Base.connection.execute("UPDATE merge_request_diff_files SET diff = '---\n- :diff: !binary |-\n    LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n    KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n    YXR'")
+    MergeRequest.connection.execute("UPDATE merge_request_diff_files SET diff = '---\n- :diff: !binary |-\n    LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n    KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n    YXR'")
 
     expect(subject['merge_requests'].first['merge_request_diff']).not_to be_empty
   end

spec/lib/gitlab/import_export/project/tree_saver_spec.rb

@@ -386,7 +386,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
       end
 
       it 'does not complain about non UTF-8 characters in MR diff files' do
-        ActiveRecord::Base.connection.execute("UPDATE merge_request_diff_files SET diff = '---\n- :diff: !binary |-\n    LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n    KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n    YXR'")
+        MergeRequestDiffFile.connection.execute("UPDATE merge_request_diff_files SET diff = '---\n- :diff: !binary |-\n    LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n    KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n    YXR'")
 
         expect(project_tree_saver.save).to be true
       end

spec/lib/gitlab/pagination/keyset/order_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do
     let(:order) { nil }
 
     def run_query(query)
-      ActiveRecord::Base.connection.execute(query).to_a
+      ApplicationRecord.connection.execute(query).to_a
     end
 
     def build_query(order:, where_conditions: nil, limit: nil)

spec/lib/gitlab/query_limiting/active_support_subscriber_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe Gitlab::QueryLimiting::ActiveSupportSubscriber do
 
     context 'when the query is actually a rails cache hit' do
       it 'does not increment the number of executed SQL queries' do
-        ActiveRecord::Base.connection.cache do
+        User.connection.cache do
           User.count
           User.count
         end

spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb

@@ -256,7 +256,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
           expect(logger).to receive(:info).with(expected_end_payload_with_db).ordered
 
           call_subject(job, 'test_queue') do
-            ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);')
+            ApplicationRecord.connection.execute('SELECT pg_sleep(0.1);')
           end
         end
 
@@ -267,7 +267,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
           expect(logger).to receive(:info).with(expected_end_payload).ordered
 
           call_subject(job.dup, 'test_queue') do
-            ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);')
+            ApplicationRecord.connection.execute('SELECT pg_sleep(0.1);')
           end
 
           Gitlab::SafeRequestStore.clear!
@@ -298,7 +298,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
           allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
         end
 
-        let(:db_config_name) { ::Gitlab::Database.db_config_name(ActiveRecord::Base.connection) }
+        let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.connection) }
 
         let(:expected_end_payload_with_db) do
           expected_end_payload.merge(

spec/models/application_setting_spec.rb

@@ -927,7 +927,7 @@ RSpec.describe ApplicationSetting do
 
   context 'when ApplicationSettings does not have a primary key' do
     before do
-      allow(ActiveRecord::Base.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil)
+      allow(described_class.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil)
     end
 
     it 'raises an exception' do

spec/requests/api/statistics_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe API::Statistics, 'Statistics' do
         # Make sure the reltuples have been updated
         # to get a correct count on postgresql
         tables_to_analyze.each do |table|
-          ActiveRecord::Base.connection.execute("ANALYZE #{table}")
+          ApplicationRecord.connection.execute("ANALYZE #{table}")
         end
 
         get api(path, admin)