Change the interface of Security artifact parsers

Previously we were instantiating the parser objects without passing any
initialization arguments. Because of this, we were passing the report
and the JSON text to the parse method and all the other utility methods
which was causing inflation on method signatures.

By just passing the report and the JSON text as the initialization args
to the instances, we can avoid passing them in between utility methods.

ee/app/models/ee/ci/build.rb

@@ -164,7 +164,7 @@ module EE
 
       def parse_security_artifact_blob(security_report, blob)
         report_clone = security_report.clone_as_blank
-        ::Gitlab::Ci::Parsers.fabricate!(security_report.type).parse!(blob, report_clone)
+        ::Gitlab::Ci::Parsers.fabricate!(security_report.type, blob, report_clone).parse!
         security_report.merge!(report_clone)
       end
 

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

@@ -120,7 +120,7 @@ module EE
 
         report = ::Gitlab::Ci::Reports::Security::Report.new(file_type, job.pipeline, nil).tap do |report|
           each_blob do |blob|
-            ::Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, report)
+            ::Gitlab::Ci::Parsers.fabricate!(file_type, blob, report).parse!
           end
         end
 

ee/lib/gitlab/ci/parsers/security/common.rb

@@ -7,8 +7,16 @@ module Gitlab
         class Common
           SecurityReportParserError = Class.new(Gitlab::Ci::Parsers::ParserError)
 
-          def parse!(json_data, report)
-            report_data = parse_report(json_data)
+          def self.parse!(json_data, report)
+            new(json_data, report).parse!
+          end
+
+          def initialize(json_data, report)
+            @json_data = json_data
+            @report = report
+          end
+
+          def parse!
             raise SecurityReportParserError, "Invalid report format" unless report_data.is_a?(Hash)
 
             create_scanner(report, report_data.dig('scan', 'scanner'))
@@ -26,10 +34,12 @@ module Gitlab
             raise SecurityReportParserError, "#{report.type} security report parsing failed"
           end
 
-          protected
+          private
+
+          attr_reader :json_data, :report
 
-          def parse_report(json_data)
-            Gitlab::Json.parse!(json_data)
+          def report_data
+            @report_data ||= Gitlab::Json.parse!(json_data)
           end
 
           # map remediations to relevant vulnerabilities
@@ -151,8 +161,6 @@ module Gitlab
             raise NotImplementedError
           end
 
-          private
-
           def finding_name(data, identifiers, location)
             return data['message'] if data['message'].present?
             return data['name'] if data['name'].present?

ee/lib/gitlab/ci/parsers/security/concerns/deprecated_syntax.rb

@@ -14,17 +14,19 @@ module Gitlab
               override :parse_report
             end
 
-            def parse_report(json_data)
-              report = super
+            def report_data
+              @report_data ||= begin
+                data = super
 
-              if report.is_a?(Array)
-                report = {
-                  "version" => self.class::DEPRECATED_REPORT_VERSION,
-                  "vulnerabilities" => report
-                }
-              end
+                if data.is_a?(Array)
+                  data = {
+                    "version" => self.class::DEPRECATED_REPORT_VERSION,
+                    "vulnerabilities" => data
+                  }
+                end
 
-              report
+                data
+              end
             end
           end
         end

ee/lib/gitlab/ci/parsers/security/dast.rb

@@ -5,20 +5,20 @@ module Gitlab
     module Parsers
       module Security
         class Dast < Common
-          def parse!(json_data, report)
-            report_data = super
+          def parse!
+            super
 
             report.scanned_resources = create_scanned_resources(report_data.dig('scan', 'scanned_resources'))
           end
 
           private
 
-          def parse_report(json_data)
-            report = super
-
-            return Formatters::Dast.new(report).format if Formatters::Dast.satisfies?(report)
-
-            report
+          def report_data
+            @report_data ||= begin
+              super.then do |data|
+                Formatters::Dast.satisfies?(data) ? Formatters::Dast.new(data).format : data
+              end
+            end
           end
 
           def create_scanned_resources(scanned_resources)

ee/spec/finders/security/findings_finder_spec.rb

@@ -56,10 +56,10 @@ RSpec.describe Security::FindingsFinder do
 
         before(:all) do
           ds_content = File.read(artifact_ds.file.path)
-          Gitlab::Ci::Parsers::Security::DependencyScanning.new.parse!(ds_content, report_ds)
+          Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(ds_content, report_ds)
           report_ds.merge!(report_ds)
           sast_content = File.read(artifact_sast.file.path)
-          Gitlab::Ci::Parsers::Security::Sast.new.parse!(sast_content, report_sast)
+          Gitlab::Ci::Parsers::Security::Sast.parse!(sast_content, report_sast)
           report_sast.merge!(report_sast)
 
           { artifact_ds => report_ds, artifact_sast => report_sast }.each do |artifact, report|

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

@@ -8,13 +8,14 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
 
     let(:artifact) { build(:ee_ci_job_artifact, :common_security_report) }
     let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) }
-    let(:parser) { described_class.new }
     let(:location) { ::Gitlab::Ci::Reports::Security::Locations::DependencyScanning.new(file_path: 'yarn/yarn.lock', package_version: 'v2', package_name: 'saml2') }
 
     before do
-      allow(parser).to receive(:create_location).and_return(location)
+      allow_next_instance_of(described_class) do |parser|
+        allow(parser).to receive(:create_location).and_return(location)
+      end
 
-      artifact.each_blob { |blob| parser.parse!(blob, report) }
+      artifact.each_blob { |blob| described_class.parse!(blob, report) }
     end
 
     describe 'parsing finding.name' do
@@ -142,9 +143,8 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
       end
 
       it 'returns nil when scan is not a hash' do
-        parser =  described_class.new
         empty_report = Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago)
-        parser.parse!({}.to_json, empty_report)
+        described_class.parse!({}.to_json, empty_report)
 
         expect(empty_report.scan).to be(nil)
       end

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

@@ -3,15 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Parsers::Security::ContainerScanning do
-  let(:parser) { described_class.new }
   let(:project) { artifact.project }
   let(:pipeline) { artifact.job.pipeline }
   let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) }
 
   before do
-    artifact.each_blob do |blob|
-      parser.parse!(blob, report)
-    end
+    artifact.each_blob { |blob| described_class.parse!(blob, report) }
   end
 
   describe '#parse!' do

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

@@ -6,14 +6,11 @@ RSpec.describe Gitlab::Ci::Parsers::Security::CoverageFuzzing do
   let(:project) { artifact.project }
   let(:pipeline) { artifact.job.pipeline }
   let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) }
-  let(:parser) { described_class.new }
   let(:artifact) { create(:ee_ci_job_artifact, :coverage_fuzzing) }
 
   describe '#parse!' do
     before do
-      artifact.each_blob do |blob|
-        parser.parse!(blob, report)
-      end
+      artifact.each_blob { |blob| described_class.parse!(blob, report) }
     end
 
     it 'parses all identifiers and findings' do

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

@@ -10,7 +10,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Dast do
     let(:pipeline) { artifact.job.pipeline }
     let(:artifact) { create(:ee_ci_job_artifact, :dast) }
     let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) }
-    let(:parser) { described_class.new }
 
     where(:report_format,
           :occurrence_count,
@@ -32,9 +31,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Dast do
       let(:artifact) { create(:ee_ci_job_artifact, report_format) }
 
       before do
-        artifact.each_blob do |blob|
-          parser.parse!(blob, report)
-        end
+        artifact.each_blob { |blob| described_class.parse!(blob, report) }
       end
 
       it 'parses all identifiers, findings and scanned resources' do
@@ -75,9 +72,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Dast do
       let(:artifact) { create(:ee_ci_job_artifact, 'dast') }
 
       before do
-        artifact.each_blob do |blob|
-          parser.parse!(blob, report)
-        end
+        artifact.each_blob { |blob| described_class.parse!(blob, report) }
       end
 
       let(:raw_json) do
@@ -98,7 +93,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Dast do
       end
 
       it 'skips invalid URLs' do
-        parser.parse!(raw_json.to_json, report)
+        described_class.parse!(raw_json.to_json, report)
         expect(report.scanned_resources).to be_empty
       end
 

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

@@ -10,7 +10,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyScanning do
     let(:pipeline) { artifact.job.pipeline }
     let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning) }
     let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) }
-    let(:parser) { described_class.new }
 
     where(:report_format, :occurrence_count, :identifier_count, :scanner_count, :file_path, :package_name, :package_version, :version) do
       :dependency_scanning             | 4 | 7 | 3 | 'app/pom.xml' | 'io.netty/netty' | '3.9.1.Final' | '1.3'
@@ -22,9 +21,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyScanning do
       let(:artifact) { create(:ee_ci_job_artifact, report_format) }
 
       before do
-        artifact.each_blob do |blob|
-          parser.parse!(blob, report)
-        end
+        artifact.each_blob { |blob| described_class.parse!(blob, report) }
       end
 
       it "parses all identifiers and findings" do
@@ -56,7 +53,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyScanning do
         report_hash[:vulnerabilities][0][:location] = nil
       end
 
-      it { expect { parser.parse!(report_hash.to_json, report) }.not_to raise_error }
+      it { expect { described_class.parse!(report_hash.to_json, report) }.not_to raise_error }
     end
 
     context "when parsing a vulnerability with a missing cve" do
@@ -66,16 +63,14 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyScanning do
         report_hash[:vulnerabilities][0][:cve] = nil
       end
 
-      it { expect { parser.parse!(report_hash.to_json, report) }.not_to raise_error }
+      it { expect { described_class.parse!(report_hash.to_json, report) }.not_to raise_error }
     end
 
     context "when vulnerabilities have remediations" do
       let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning_remediation) }
 
       before do
-        artifact.each_blob do |blob|
-          parser.parse!(blob, report)
-        end
+        artifact.each_blob { |blob| described_class.parse!(blob, report) }
       end
 
       it "generates occurrence with expected remediation" do

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

@@ -10,8 +10,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Sast do
 
     let(:created_at) { 2.weeks.ago }
 
-    subject(:parser) { described_class.new }
-
     context "when parsing valid reports" do
       where(:report_format, :scanner_length) do
         :sast               | 4
@@ -23,9 +21,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Sast do
         let(:artifact) { create(:ee_ci_job_artifact, report_format) }
 
         before do
-          artifact.each_blob do |blob|
-            parser.parse!(blob, report)
-          end
+          artifact.each_blob { |blob| described_class.parse!(blob, report) }
         end
 
         it "parses all identifiers and findings" do
@@ -57,7 +53,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Sast do
       let(:report) { Gitlab::Ci::Reports::Security::Report.new('sast', pipeline, created_at) }
       let(:blob) { Gitlab::Json.generate({}) }
 
-      it { expect(parser.parse!(blob, report)).to be_empty }
+      it { expect(described_class.parse!(blob, report)).to be_empty }
     end
   end
 end

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

@@ -8,8 +8,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::SecretDetection do
 
     let(:created_at) { 2.weeks.ago }
 
-    subject(:parser) { described_class.new }
-
     context "when parsing valid reports" do
       where(report_format: %i(secret_detection))
 
@@ -18,9 +16,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::SecretDetection do
         let(:artifact) { create(:ee_ci_job_artifact, report_format) }
 
         before do
-          artifact.each_blob do |blob|
-            parser.parse!(blob, report)
-          end
+          artifact.each_blob { |blob| described_class.parse!(blob, report) }
         end
 
         it "parses all identifiers and findings" do
@@ -52,7 +48,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::SecretDetection do
       let(:report) { Gitlab::Ci::Reports::Security::Report.new('secret_detection', pipeline, created_at) }
       let(:blob) { Gitlab::Json.generate({}) }
 
-      it { expect(parser.parse!(blob, report)).to be_empty }
+      it { expect(described_class.parse!(blob, report)).to be_empty }
     end
   end
 end

lib/gitlab/ci/parsers.rb

@@ -15,8 +15,8 @@ module Gitlab
         }
       end
 
-      def self.fabricate!(file_type)
-        parsers.fetch(file_type.to_sym).new
+      def self.fabricate!(file_type, *args)
+        parsers.fetch(file_type.to_sym).new(*args)
       rescue KeyError
         raise ParserNotFoundError, "Cannot find any parser matching file type '#{file_type}'"
       end