Merge branch 'move_methods' into 'master'

Move sast_reports and secret_detection_reports to CE [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!48200

app/controllers/projects/merge_requests_controller.rb

@@ -168,6 +168,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     }
   end
 
+  def sast_reports
+    reports_response(merge_request.compare_sast_reports(current_user), head_pipeline)
+  end
+
+  def secret_detection_reports
+    reports_response(merge_request.compare_secret_detection_reports(current_user), head_pipeline)
+  end
+
   def context_commits
     return render_404 unless project.context_commits_enabled?
 

app/models/ci/job_artifact.rb

@@ -19,6 +19,8 @@ module Ci
     NON_ERASABLE_FILE_TYPES = %w[trace].freeze
     TERRAFORM_REPORT_FILE_TYPES = %w[terraform].freeze
     UNSUPPORTED_FILE_TYPES = %i[license_management].freeze
+    SAST_REPORT_TYPES = %w[sast].freeze
+    SECRET_DETECTION_REPORT_TYPES = %w[secret_detection].freeze
     DEFAULT_FILE_NAMES = {
       archive: nil,
       metadata: nil,
@@ -150,6 +152,14 @@ module Ci
       with_file_types(REPORT_TYPES.keys.map(&:to_s))
     end
 
+    scope :sast_reports, -> do
+      with_file_types(SAST_REPORT_TYPES)
+    end
+
+    scope :secret_detection_reports, -> do
+      with_file_types(SECRET_DETECTION_REPORT_TYPES)
+    end
+
     scope :test_reports, -> do
       with_file_types(TEST_REPORT_FILE_TYPES)
     end

app/models/ci/pipeline.rb

@@ -1219,6 +1219,16 @@ module Ci
       false
     end
 
+    def security_reports(report_types: [])
+      reports_scope = report_types.empty? ? ::Ci::JobArtifact.security_reports : ::Ci::JobArtifact.security_reports(file_types: report_types)
+
+      ::Gitlab::Ci::Reports::Security::Reports.new(self).tap do |security_reports|
+        latest_report_builds(reports_scope).each do |build|
+          build.collect_security_reports!(security_reports)
+        end
+      end
+    end
+
     private
 
     def add_message(severity, content)

app/models/merge_request.rb

@@ -1554,6 +1554,26 @@ class MergeRequest < ApplicationRecord
     end || { status: :parsing }
   end
 
+  def has_sast_reports?
+    !!actual_head_pipeline&.has_reports?(::Ci::JobArtifact.sast_reports)
+  end
+
+  def has_secret_detection_reports?
+    !!actual_head_pipeline&.has_reports?(::Ci::JobArtifact.secret_detection_reports)
+  end
+
+  def compare_sast_reports(current_user)
+    return missing_report_error("SAST") unless has_sast_reports?
+
+    compare_reports(::Ci::CompareSecurityReportsService, current_user, 'sast')
+  end
+
+  def compare_secret_detection_reports(current_user)
+    return missing_report_error("secret detection") unless has_secret_detection_reports?
+
+    compare_reports(::Ci::CompareSecurityReportsService, current_user, 'secret_detection')
+  end
+
   def calculate_reactive_cache(identifier, current_user_id = nil, report_type = nil, *args)
     service_class = identifier.constantize
 
@@ -1799,8 +1819,19 @@ class MergeRequest < ApplicationRecord
     merge_request_reviewers.find_by(user_id: user.id)
   end
 
+  def enabled_reports
+    {
+      sast: report_type_enabled?(:sast),
+      secret_detection: report_type_enabled?(:secret_detection)
+    }
+  end
+
   private
 
+  def missing_report_error(report_type)
+    { status: :error, status_reason: "This merge request does not have #{report_type} reports" }
+  end
+
   def with_rebase_lock
     if Feature.enabled?(:merge_request_rebase_nowait_lock, default_enabled: true)
       with_retried_nowait_lock { yield }
@@ -1842,6 +1873,10 @@ class MergeRequest < ApplicationRecord
     key = Gitlab::Routing.url_helpers.cached_widget_project_json_merge_request_path(project, self, format: :json)
     Gitlab::EtagCaching::Store.new.touch(key)
   end
+
+  def report_type_enabled?(report_type)
+    !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(report_type)
+  end
 end
 
 MergeRequest.prepend_if_ee('::EE::MergeRequest')

app/serializers/merge_request_widget_entity.rb

@@ -133,6 +133,10 @@ class MergeRequestWidgetEntity < Grape::Entity
     help_page_path('user/application_security/index.md', anchor: 'viewing-security-scan-information-in-merge-requests')
   end
 
+  expose :enabled_reports do |merge_request|
+    merge_request.enabled_reports
+  end
+
   private
 
   delegate :current_user, to: :request

ee/app/controllers/ee/projects/merge_requests_controller.rb

@@ -46,14 +46,6 @@ module EE
         reports_response(merge_request.compare_dependency_scanning_reports(current_user), head_pipeline)
       end
 
-      def sast_reports
-        reports_response(merge_request.compare_sast_reports(current_user), head_pipeline)
-      end
-
-      def secret_detection_reports
-        reports_response(merge_request.compare_secret_detection_reports(current_user), head_pipeline)
-      end
-
       def dast_reports
         reports_response(merge_request.compare_dast_reports(current_user), head_pipeline)
       end

ee/app/models/ee/ci/job_artifact.rb

@@ -17,8 +17,6 @@ module EE
       DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze
       METRICS_REPORT_FILE_TYPES = %w[metrics].freeze
       CONTAINER_SCANNING_REPORT_TYPES = %w[container_scanning].freeze
-      SAST_REPORT_TYPES = %w[sast].freeze
-      SECRET_DETECTION_REPORT_TYPES = %w[secret_detection].freeze
       DAST_REPORT_TYPES = %w[dast].freeze
       REQUIREMENTS_REPORT_FILE_TYPES = %w[requirements].freeze
       COVERAGE_FUZZING_REPORT_TYPES = %w[coverage_fuzzing].freeze
@@ -46,14 +44,6 @@ module EE
         with_file_types(CONTAINER_SCANNING_REPORT_TYPES)
       end
 
-      scope :sast_reports, -> do
-        with_file_types(SAST_REPORT_TYPES)
-      end
-
-      scope :secret_detection_reports, -> do
-        with_file_types(SECRET_DETECTION_REPORT_TYPES)
-      end
-
       scope :dast_reports, -> do
         with_file_types(DAST_REPORT_TYPES)
       end

ee/app/models/ee/ci/pipeline.rb

@@ -99,16 +99,6 @@ module EE
         batch_lookup_report_artifact_for_file_type(:license_scanning).present?
       end
 
-      def security_reports(report_types: [])
-        reports_scope = report_types.empty? ? ::Ci::JobArtifact.security_reports : ::Ci::JobArtifact.security_reports(file_types: report_types)
-
-        ::Gitlab::Ci::Reports::Security::Reports.new(self).tap do |security_reports|
-          latest_report_builds(reports_scope).each do |build|
-            build.collect_security_reports!(security_reports)
-          end
-        end
-      end
-
       def license_scanning_report
         ::Gitlab::Ci::Reports::LicenseScanning::Report.new.tap do |license_management_report|
           latest_report_builds(::Ci::JobArtifact.license_scanning_reports).each do |build|

ee/app/models/ee/merge_request.rb

@@ -193,26 +193,6 @@ module EE
       compare_reports(::Ci::CompareSecurityReportsService, current_user, 'container_scanning')
     end
 
-    def has_sast_reports?
-      !!actual_head_pipeline&.has_reports?(::Ci::JobArtifact.sast_reports)
-    end
-
-    def has_secret_detection_reports?
-      !!actual_head_pipeline&.has_reports?(::Ci::JobArtifact.secret_detection_reports)
-    end
-
-    def compare_sast_reports(current_user)
-      return missing_report_error("SAST") unless has_sast_reports?
-
-      compare_reports(::Ci::CompareSecurityReportsService, current_user, 'sast')
-    end
-
-    def compare_secret_detection_reports(current_user)
-      return missing_report_error("secret detection") unless has_secret_detection_reports?
-
-      compare_reports(::Ci::CompareSecurityReportsService, current_user, 'secret_detection')
-    end
-
     def has_dast_reports?
       !!actual_head_pipeline&.has_reports?(::Ci::JobArtifact.dast_reports)
     end
@@ -287,13 +267,5 @@ module EE
         ApprovalWrappedRule.wrap(self, rule).approved?
       end
     end
-
-    def missing_report_error(report_type)
-      { status: :error, status_reason: "This merge request does not have #{report_type} reports" }
-    end
-
-    def report_type_enabled?(report_type)
-      !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(report_type)
-    end
   end
 end

ee/app/serializers/ee/merge_request_widget_entity.rb

@@ -31,10 +31,6 @@ module EE
         end
       end
 
-      expose :enabled_reports do |merge_request|
-        merge_request.enabled_reports
-      end
-
       expose :license_scanning, if: -> (mr, _) { can?(current_user, :read_licenses, mr.target_project) } do
         expose :managed_licenses_path do |merge_request|
           expose_path(api_v4_projects_managed_licenses_path(id: merge_request.target_project.id))

ee/changelogs/unreleased/move_methods.yml

+---
+title: Move sast_reports and secret_detection_reports to CE
+merge_request: 48200
+author:
+type: changed

ee/spec/factories/ci/job_artifacts.rb

@@ -8,7 +8,7 @@ FactoryBot.define do
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-sast-report.json'), 'application/json')
+          Rails.root.join('spec/fixtures/security_reports/master/gl-sast-report.json'), 'application/json')
       end
     end
 
@@ -28,7 +28,7 @@ FactoryBot.define do
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-secret-detection-report.json'), 'application/json')
+          Rails.root.join('spec/fixtures/security_reports/master/gl-secret-detection-report.json'), 'application/json')
       end
     end
 

ee/spec/factories/merge_requests.rb

@@ -121,30 +121,6 @@ FactoryBot.define do
       end
     end
 
-    trait :with_sast_reports do
-      after(:build) do |merge_request|
-        merge_request.head_pipeline = build(
-          :ee_ci_pipeline,
-          :success,
-          :with_sast_report,
-          project: merge_request.source_project,
-          ref: merge_request.source_branch,
-          sha: merge_request.diff_head_sha)
-      end
-    end
-
-    trait :with_secret_detection_reports do
-      after(:build) do |merge_request|
-        merge_request.head_pipeline = build(
-          :ee_ci_pipeline,
-          :success,
-          :with_secret_detection_report,
-          project: merge_request.source_project,
-          ref: merge_request.source_branch,
-          sha: merge_request.diff_head_sha)
-      end
-    end
-
     trait :with_coverage_fuzzing_reports do
       after(:build) do |merge_request|
         merge_request.head_pipeline = build(

ee/spec/lib/gitlab/ci/parsers/security/dependency_scanning_spec.rb

@@ -47,7 +47,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyScanning do
     end
 
     context "when parsing a vulnerability with a missing location" do
-      let(:report_hash) { Gitlab::Json.parse(fixture_file('security_reports/master/gl-sast-report.json', dir: 'ee'), symbolize_names: true) }
+      let(:report_hash) { Gitlab::Json.parse(fixture_file('security_reports/master/gl-sast-report.json'), symbolize_names: true) }
 
       before do
         report_hash[:vulnerabilities][0][:location] = nil
@@ -57,7 +57,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyScanning do
     end
 
     context "when parsing a vulnerability with a missing cve" do
-      let(:report_hash) { Gitlab::Json.parse(fixture_file('security_reports/master/gl-sast-report.json', dir: 'ee'), symbolize_names: true) }
+      let(:report_hash) { Gitlab::Json.parse(fixture_file('security_reports/master/gl-sast-report.json'), symbolize_names: true) }
 
       before do
         report_hash[:vulnerabilities][0][:cve] = nil

ee/spec/models/merge_request_spec.rb

@@ -241,6 +241,7 @@ RSpec.describe MergeRequest do
       :license_scanning    | :with_license_management_reports  | :license_scanning
       :license_scanning    | :with_license_scanning_reports    | :license_scanning
       :coverage_fuzzing    | :with_coverage_fuzzing_reports    | :coverage_fuzzing
+      :secret_detection    | :with_secret_detection_reports    | :secret_detection
       :api_fuzzing         | :with_api_fuzzing_reports         | :api_fuzzing
     end
 
@@ -353,50 +354,6 @@ RSpec.describe MergeRequest do
     end
   end
 
-  describe '#has_sast_reports?' do
-    subject { merge_request.has_sast_reports? }
-
-    let(:project) { create(:project, :repository) }
-
-    before do
-      stub_licensed_features(sast: true)
-    end
-
-    context 'when head pipeline has sast reports' do
-      let(:merge_request) { create(:ee_merge_request, :with_sast_reports, source_project: project) }
-
-      it { is_expected.to be_truthy }
-    end
-
-    context 'when head pipeline does not have sast reports' do
-      let(:merge_request) { create(:ee_merge_request, source_project: project) }
-
-      it { is_expected.to be_falsey }
-    end
-  end
-
-  describe '#has_secret_detection_reports?' do
-    subject { merge_request.has_secret_detection_reports? }
-
-    let(:project) { create(:project, :repository) }
-
-    before do
-      stub_licensed_features(secret_detection: true)
-    end
-
-    context 'when head pipeline has secret detection reports' do
-      let(:merge_request) { create(:ee_merge_request, :with_secret_detection_reports, source_project: project) }
-
-      it { is_expected.to be_truthy }
-    end
-
-    context 'when head pipeline does not have secrets detection reports' do
-      let(:merge_request) { create(:ee_merge_request, source_project: project) }
-
-      it { is_expected.to be_falsey }
-    end
-  end
-
   describe '#has_dast_reports?' do
     subject { merge_request.has_dast_reports? }
 

spec/factories/ci/builds.rb

@@ -290,6 +290,24 @@ FactoryBot.define do
       end
     end
 
+    trait :codequality_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ci_job_artifact, :codequality, job: build)
+      end
+    end
+
+    trait :sast_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ci_job_artifact, :sast, job: build)
+      end
+    end
+
+    trait :secret_detection_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ci_job_artifact, :secret_detection, job: build)
+      end
+    end
+
     trait :test_reports do
       after(:build) do |build|
         build.job_artifacts << create(:ci_job_artifact, :junit, job: build)

spec/factories/ci/job_artifacts.rb

@@ -269,6 +269,26 @@ FactoryBot.define do
       end
     end
 
+    trait :sast do
+      file_type { :sast }
+      file_format { :raw }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/security_reports/master/gl-sast-report.json'), 'application/json')
+      end
+    end
+
+    trait :secret_detection do
+      file_type { :secret_detection }
+      file_format { :raw }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/security_reports/master/gl-secret-detection-report.json'), 'application/json')
+      end
+    end
+
     trait :lsif do
       file_type { :lsif }
       file_format { :zip }

spec/factories/ci/pipelines.rb

@@ -93,6 +93,30 @@ FactoryBot.define do
         end
       end
 
+      trait :with_codequality_report do
+        status { :success }
+
+        after(:build) do |pipeline, evaluator|
+          pipeline.builds << build(:ci_build, :codequality_report, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
+      trait :with_sast_report do
+        status { :success }
+
+        after(:build) do |pipeline, evaluator|
+          pipeline.builds << build(:ci_build, :sast_report, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
+      trait :with_secret_detection_report do
+        status { :success }
+
+        after(:build) do |pipeline, evaluator|
+          pipeline.builds << build(:ci_build, :secret_detection_report, pipeline: pipeline, project: pipeline.project)
+        end
+      end
+
       trait :with_test_reports do
         status { :success }
 

spec/factories/merge_requests.rb

@@ -221,6 +221,30 @@ FactoryBot.define do
       end
     end
 
+    trait :with_sast_reports do
+      after(:build) do |merge_request|
+        merge_request.head_pipeline = build(
+          :ci_pipeline,
+          :success,
+          :with_sast_report,
+          project: merge_request.source_project,
+          ref: merge_request.source_branch,
+          sha: merge_request.diff_head_sha)
+      end
+    end
+
+    trait :with_secret_detection_reports do
+      after(:build) do |merge_request|
+        merge_request.head_pipeline = build(
+          :ci_pipeline,
+          :success,
+          :with_secret_detection_report,
+          project: merge_request.source_project,
+          ref: merge_request.source_branch,
+          sha: merge_request.diff_head_sha)
+      end
+    end
+
     trait :with_exposed_artifacts do
       after(:build) do |merge_request|
         merge_request.head_pipeline = build(

spec/models/merge_request_spec.rb

@@ -2054,6 +2054,50 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
+  describe '#has_sast_reports?' do
+    subject { merge_request.has_sast_reports? }
+
+    let(:project) { create(:project, :repository) }
+
+    before do
+      stub_licensed_features(sast: true)
+    end
+
+    context 'when head pipeline has sast reports' do
+      let(:merge_request) { create(:merge_request, :with_sast_reports, source_project: project) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when head pipeline does not have sast reports' do
+      let(:merge_request) { create(:merge_request, source_project: project) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
+  describe '#has_secret_detection_reports?' do
+    subject { merge_request.has_secret_detection_reports? }
+
+    let(:project) { create(:project, :repository) }
+
+    before do
+      stub_licensed_features(secret_detection: true)
+    end
+
+    context 'when head pipeline has secret detection reports' do
+      let(:merge_request) { create(:merge_request, :with_secret_detection_reports, source_project: project) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when head pipeline does not have secrets detection reports' do
+      let(:merge_request) { create(:merge_request, source_project: project) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#calculate_reactive_cache' do
     let(:merge_request) { create(:merge_request) }
 
@@ -4587,4 +4631,34 @@ RSpec.describe MergeRequest, factory_default: :keep do
         .from(nil).to(ref)
     end
   end
+
+  describe '#enabled_reports' do
+    let(:project) { create(:project, :repository) }
+
+    where(:report_type, :with_reports, :feature) do
+      :sast                | :with_sast_reports                | :sast
+      :secret_detection    | :with_secret_detection_reports    | :secret_detection
+    end
+
+    with_them do
+      subject { merge_request.enabled_reports[report_type] }
+
+      before do
+        stub_feature_flags(drop_license_management_artifact: false)
+        stub_licensed_features({ feature => true })
+      end
+
+      context "when head pipeline has reports" do
+        let(:merge_request) { create(:merge_request, with_reports, source_project: project) }
+
+        it { is_expected.to be_truthy }
+      end
+
+      context "when head pipeline does not have reports" do
+        let(:merge_request) { create(:merge_request, source_project: project) }
+
+        it { is_expected.to be_falsy }
+      end
+    end
+  end
 end