Merge branch...

Merge branch 'philipcunningham-add-relationship-between-builds-and-dast-profiles-216514' into 'master'

Associate Build with DAST Site and Scanner profile [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!63362

ee/app/services/ee/ci/retry_build_service.rb

@@ -11,7 +11,12 @@ module RetryBuildService
         override :clone_accessors
         def clone_accessors
-          (super + %i[secrets]).freeze
+          (super + extra_accessors).freeze
+        end
+
+        override :extra_accessors
+        def extra_accessors
+          %i[dast_site_profile dast_scanner_profile secrets].freeze
         end
       end

ee/spec/factories/dast/scanner_profiles_builds.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :dast_scanner_profiles_build, class: 'Dast::ScannerProfilesBuild' do
+    dast_scanner_profile
+
+    ci_build { association :ci_build, project: dast_scanner_profile.project }
+  end
+end

ee/spec/factories/dast/site_profiles_builds.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :dast_site_profiles_build, class: 'Dast::SiteProfilesBuild' do
+    dast_site_profile
+
+    ci_build { association :ci_build, project: dast_site_profile.project }
+  end
+end

ee/spec/models/ci/build_spec.rb

@@ -129,59 +129,182 @@
         end
       end
-      context 'when there is a dast_profile associated with the pipeline' do
+      context 'dast' do
         let_it_be(:project) { create(:project, :repository) }
         let_it_be(:user) { create(:user, developer_projects: [project]) }
-        let_it_be(:dast_profile) { create(:dast_profile, project: project) }
-        let_it_be(:dast_site_profile_secret_variable) { create(:dast_site_profile_secret_variable, key: 'DAST_PASSWORD_BASE64', dast_site_profile: dast_profile.dast_site_profile) }
-
-        let(:pipeline) { create(:ci_pipeline, pipeline_params.merge!(project: project, dast_profile: dast_profile, user: user) ) }
-
-        let(:key) { dast_site_profile_secret_variable.key }
-        let(:value) { dast_site_profile_secret_variable.value }
+        let_it_be(:dast_site_profile) { create(:dast_site_profile, project: project) }
+        let_it_be(:dast_scanner_profile) { create(:dast_scanner_profile, project: project) }
+        let_it_be(:dast_profile) { create(:dast_profile, project: project, dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile) }
+        let_it_be(:dast_site_profile_secret_variable) { create(:dast_site_profile_secret_variable, key: 'DAST_PASSWORD_BASE64', dast_site_profile: dast_site_profile) }
+        let_it_be(:options) { { dast_configuration: { site_profile: dast_site_profile.name, scanner_profile: dast_scanner_profile.name } } }
         before do
           stub_licensed_features(security_on_demand_scans: true)
         end
-        shared_examples 'a pipeline with no dast on-demand variables' do
-          it 'does not include variables associated with the profile' do
-            keys = subject.to_runner_variables.map { |var| var[:key] }
-
-            expect(keys).not_to include(key)
+        shared_examples 'it includes variables' do
+          it 'includes variables from the profile' do
+            expect(subject.to_runner_variables).to include(*expected_variables.to_runner_variables)
           end
         end
-        it_behaves_like 'a pipeline with no dast on-demand variables' do
-          let(:pipeline_params) { { config_source: :parameter_source } }
+        shared_examples 'it excludes variables' do
+          it 'excludes variables from the profile' do
+            expect(subject.to_runner_variables).not_to include(*expected_variables.to_runner_variables)
+          end
         end
-        it_behaves_like 'a pipeline with no dast on-demand variables' do
-          let(:pipeline_params) { { source: :ondemand_dast_scan } }
+        context 'when there is a dast_site_profile associated with the job' do
+          let(:pipeline) { create(:ci_pipeline, project: project) }
+          let(:job) { create(:ci_build, :running, pipeline: pipeline, dast_site_profile: dast_site_profile, user: user, options: options) }
+
+          context 'when feature is enabled' do
+            it_behaves_like 'it includes variables' do
+              let(:expected_variables) { dast_site_profile.ci_variables }
+            end
+
+            context 'when user has permission' do
+              it_behaves_like 'it includes variables' do
+                let(:expected_variables) { dast_site_profile.secret_ci_variables(user) }
+              end
+            end
+
+            context 'when user does not have permission' do
+              let_it_be(:user) { create(:user) }
+
+              before do
+                project.add_guest(user)
+              end
+
+              it_behaves_like 'it excludes variables' do
+                let(:expected_variables) { dast_site_profile.secret_ci_variables(user) }
+              end
+            end
+          end
+
+          context 'when feature is disabled' do
+            before do
+              stub_feature_flags(dast_configuration_ui: false)
+            end
+
+            it_behaves_like 'it excludes variables' do
+              let(:expected_variables) { dast_site_profile.ci_variables.concat(dast_site_profile.secret_ci_variables(user)) }
+            end
+          end
         end
-        context 'when the dast on-demand pipeline is correctly configured' do
-          let(:pipeline_params) { { source: :ondemand_dast_scan, config_source: :parameter_source } }
-          it 'includes variables associated with the profile' do
-            expect(subject.to_runner_variables).to include(key: key, value: value, public: false, masked: true)
+        context 'when there is a dast_scanner_profile associated with the job' do
+          let(:pipeline) { create(:ci_pipeline, project: project, user: user) }
+          let(:job) { create(:ci_build, :running, pipeline: pipeline, dast_scanner_profile: dast_scanner_profile, options: options) }
+          context 'when feature is enabled' do
+            it_behaves_like 'it includes variables' do
+              let(:expected_variables) { dast_scanner_profile.ci_variables }
+            end
           end
-          context 'when user cannot read secrets' do
+          context 'when feature is disabled' do
             before do
-              stub_licensed_features(security_on_demand_scans: false)
+              stub_feature_flags(dast_configuration_ui: false)
             end
-            it 'does not include variables associated with the profile' do
-              expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true)
+            it_behaves_like 'it excludes variables' do
+              let(:expected_variables) { dast_scanner_profile.ci_variables }
             end
           end
-          context 'when there is no user associated with the pipeline' do
-            let_it_be(:user) { nil }
+        end
+
+        context 'when there are profiles associated with the job' do
+          let(:pipeline) { create(:ci_pipeline, project: project) }
+          let(:job) { create(:ci_build, :running, pipeline: pipeline, dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile, user: user, options: options) }
+          context 'when dast_configuration is absent from the options' do
+            let(:options) { {} }
+            it 'does not attempt look up any dast profiles', :aggregate_failures do
+              expect(job).not_to receive(:dast_site_profile)
+              expect(job).not_to receive(:dast_scanner_profile)
+
+              subject
+            end
+          end
+
+          context 'when site_profile is absent from the dast_configuration' do
+            let(:options) { { dast_configuration: { scanner_profile: dast_scanner_profile.name } } }
+
+            it 'does not attempt look up the site profile' do
+              expect(job).not_to receive(:dast_site_profile)
+
+              subject
+            end
+          end
+
+          context 'when scanner_profile is absent from the dast_configuration' do
+            let(:options) { { dast_configuration: { site_profile: dast_site_profile.name } } }
+
+            it 'does not attempt look up the scanner profile' do
+              expect(job).not_to receive(:dast_scanner_profile)
+
+              subject
+            end
+          end
+
+          context 'when both profiles are present in the dast_configuration' do
+            it 'attempts look up dast profiles', :aggregate_failures do
+              expect(job).to receive(:dast_site_profile).and_call_original.at_least(:once)
+              expect(job).to receive(:dast_scanner_profile).and_call_original.at_least(:once)
+
+              subject
+            end
+          end
+        end
+
+        context 'when there is a dast_profile associated with the pipeline' do
+          let(:pipeline) { create(:ci_pipeline, pipeline_params.merge!(project: project, dast_profile: dast_profile, user: user) ) }
+          let(:key) { dast_site_profile_secret_variable.key }
+          let(:value) { dast_site_profile_secret_variable.value }
+
+          shared_examples 'a record with no associated dast variables' do
             it 'does not include variables associated with the profile' do
-              expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true)
+              keys = subject.to_runner_variables.map { |var| var[:key] }
+
+              expect(keys).not_to include(key)
+            end
+          end
+
+          context 'when the on-demand pipeline is incorrectly configured' do
+            it_behaves_like 'a record with no associated dast variables' do
+              let(:pipeline_params) { { config_source: :parameter_source } }
+            end
+
+            it_behaves_like 'a record with no associated dast variables' do
+              let(:pipeline_params) { { source: :ondemand_dast_scan } }
+            end
+          end
+
+          context 'when the dast on-demand pipeline is correctly configured' do
+            let(:pipeline_params) { { source: :ondemand_dast_scan, config_source: :parameter_source } }
+
+            it 'includes variables associated with the profile' do
+              expect(subject.to_runner_variables).to include(key: key, value: value, public: false, masked: true)
+            end
+
+            context 'when user cannot read secrets' do
+              before do
+                stub_licensed_features(security_on_demand_scans: false)
+              end
+
+              it 'does not include variables associated with the profile' do
+                expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true)
+              end
+            end
+
+            context 'when there is no user associated with the pipeline' do
+              let_it_be(:user) { nil }
+
+              it 'does not include variables associated with the profile' do
+                expect(subject.to_runner_variables).not_to include(key: key, value: value, public: false, masked: true)
+              end
             end
           end
         end

ee/spec/models/dast/scanner_profiles_build_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Dast::ScannerProfilesBuild, type: :model do
+  subject { create(:dast_scanner_profiles_build) }
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:ci_build).class_name('Ci::Build').inverse_of(:dast_scanner_profiles_build).required }
+    it { is_expected.to belong_to(:dast_scanner_profile).class_name('DastScannerProfile').inverse_of(:dast_scanner_profiles_builds).required }
+  end
+
+  describe 'validations' do
+    it { is_expected.to be_valid }
+    it { is_expected.to validate_presence_of(:ci_build_id) }
+    it { is_expected.to validate_presence_of(:dast_scanner_profile_id) }
+
+    context 'when the ci_build.project_id and dast_scanner_profile.project_id do not match' do
+      let(:ci_build) { build(:ci_build, project_id: 1) }
+      let(:scanner_profile) { build(:dast_scanner_profile, project_id: 2) }
+
+      subject { build(:dast_scanner_profiles_build, ci_build: ci_build, dast_scanner_profile: scanner_profile) }
+
+      it 'is not valid', :aggregate_failures do
+        expect(subject).not_to be_valid
+        expect(subject.errors.full_messages).to include('Ci build project_id must match dast_scanner_profile.project_id')
+      end
+    end
+  end
+end

ee/spec/models/dast/site_profiles_build_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Dast::SiteProfilesBuild, type: :model do
+  subject { create(:dast_site_profiles_build) }
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:ci_build).class_name('Ci::Build').inverse_of(:dast_site_profiles_build).required }
+    it { is_expected.to belong_to(:dast_site_profile).class_name('DastSiteProfile').inverse_of(:dast_site_profiles_builds).required }
+  end
+
+  describe 'validations' do
+    it { is_expected.to be_valid }
+    it { is_expected.to validate_presence_of(:ci_build_id) }
+    it { is_expected.to validate_presence_of(:dast_site_profile_id) }
+
+    context 'when the ci_build.project_id and dast_site_profile.project_id do not match' do
+      let(:ci_build) { build(:ci_build, project_id: 1) }
+      let(:site_profile) { build(:dast_site_profile, project_id: 2) }
+
+      subject { build(:dast_site_profiles_build, ci_build: ci_build, dast_site_profile: site_profile) }
+
+      it 'is not valid', :aggregate_failures do
+        expect(subject).not_to be_valid
+        expect(subject.errors.full_messages).to include('Ci build project_id must match dast_site_profile.project_id')
+      end
+    end
+  end
+end

ee/spec/models/dast_scanner_profile_spec.rb

@@ -7,6 +7,8 @@
   describe 'associations' do
     it { is_expected.to belong_to(:project) }
+    it { is_expected.to have_many(:dast_scanner_profiles_builds).class_name('Dast::ScannerProfilesBuild').with_foreign_key(:dast_scanner_profile_id).inverse_of(:dast_scanner_profile) }
+    it { is_expected.to have_many(:ci_builds).class_name('Ci::Build').through(:dast_scanner_profiles_builds) }
   end
   describe 'validations' do

ee/spec/models/dast_site_profile_spec.rb

@@ -11,6 +11,8 @@
     it { is_expected.to belong_to(:project) }
     it { is_expected.to belong_to(:dast_site) }
     it { is_expected.to have_many(:secret_variables).class_name('Dast::SiteProfileSecretVariable') }
+    it { is_expected.to have_many(:dast_site_profiles_builds).class_name('Dast::SiteProfilesBuild').with_foreign_key(:dast_site_profile_id).inverse_of(:dast_site_profile) }
+    it { is_expected.to have_many(:ci_builds).class_name('Ci::Build').through(:dast_site_profiles_builds) }
   end
   describe 'validations' do

ee/spec/services/ci/retry_build_service_spec.rb

@@ -28,6 +28,20 @@
         project.add_developer(user)
       end
+      context 'dast' do
+        let(:dast_site_profile) { create(:dast_site_profile, project: project) }
+        let(:dast_scanner_profile) { create(:dast_scanner_profile, project: project) }
+
+        before do
+          build.update!(dast_site_profile: dast_site_profile, dast_scanner_profile: dast_scanner_profile)
+        end
+
+        it 'clones the profile associations', :aggregate_failures do
+          expect(new_build.dast_site_profile).to eq(dast_site_profile)
+          expect(new_build.dast_scanner_profile).to eq(dast_scanner_profile)
+        end
+      end
+
       context 'when build has secrets' do
         let(:secrets) do
           {

spec/services/ci/retry_build_service_spec.rb

@@ -30,7 +30,7 @@
     project.add_reporter(reporter)
   end
-  clone_accessors = described_class.clone_accessors
+  clone_accessors = described_class.clone_accessors.without(described_class.extra_accessors)
   reject_accessors =
     %i[id status user token token_encrypted coverage trace runner
@@ -98,7 +98,7 @@
       end
       clone_accessors.each do |attribute|
-        it "clones #{attribute} build attribute" do
+        it "clones #{attribute} build attribute", :aggregate_failures do
           expect(attribute).not_to be_in(forbidden_associations), "association #{attribute} must be `belongs_to`"
           expect(build.send(attribute)).not_to be_nil
           expect(new_build.send(attribute)).not_to be_nil
@@ -134,7 +134,7 @@
       end
     end
-    it 'has correct number of known attributes' do
+    it 'has correct number of known attributes', :aggregate_failures do
       processed_accessors = clone_accessors + reject_accessors
       known_accessors = processed_accessors + ignore_accessors
@@ -146,9 +146,10 @@
         Ci::Build.attribute_names.map(&:to_sym) +
         Ci::Build.attribute_aliases.keys.map(&:to_sym) +
         Ci::Build.reflect_on_all_associations.map(&:name) +
-        [:tag_list, :needs_attributes]
-
-      current_accessors << :secrets if Gitlab.ee?
+        [:tag_list, :needs_attributes] -
+        # ee-specific accessors should be tested in ee/spec/services/ci/retry_build_service_spec.rb instead
+        described_class.extra_accessors -
+        [:dast_site_profiles_build, :dast_scanner_profiles_build] # join tables
       current_accessors.uniq!