Merge branch '276011_remove_store_security_findings_feature_flag' into 'master'

Remove `store_security_findings` feature flag related code

See merge request gitlab-org/gitlab!48357

ee/app/finders/security/findings_finder.rb

@@ -29,7 +29,7 @@ module Security
     end
 
     def execute
-      return unless can_use_security_findings?
+      return unless has_security_findings?
 
       ResultSet.new(security_findings, findings)
     end
@@ -39,10 +39,6 @@ module Security
     attr_reader :pipeline, :params
     delegate :project, :has_security_findings?, to: :pipeline, private: true
 
-    def can_use_security_findings?
-      Feature.enabled?(:store_security_findings, project) && has_security_findings?
-    end
-
     def findings
       security_findings.map(&method(:build_vulnerability_finding))
     end

ee/app/services/security/store_scan_service.rb

@@ -19,8 +19,6 @@ module Security
     end
 
     def execute
-      return security_scan unless Feature.enabled?(:store_security_findings, project)
-
       StoreFindingsMetadataService.execute(security_scan, security_report)
       deduplicate_findings? ? update_deduplicated_findings : register_finding_keys
 

ee/changelogs/unreleased/276011_remove_store_security_findings_feature_flag.yml

+---
+title: Remove `store_security_findings` feature flag
+merge_request: 48357
+author:
+type: changed

ee/config/feature_flags/development/store_security_findings.yml

----
-name: store_security_findings
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44312
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/276011
-milestone: '13.6'
-type: development
-group: group::threat insights
-default_enabled: false

ee/spec/services/security/store_scan_service_spec.rb

@@ -41,121 +41,89 @@ RSpec.describe Security::StoreScanService do
       known_keys.add(finding_key)
     end
 
-    context 'when the `store_security_findings` feature is not enabled' do
-      before do
-        stub_feature_flags(store_security_findings: false)
-      end
-
-      it 'does not call the `Security::StoreFindingsMetadataService`' do
-        store_scan
-
-        expect(Security::StoreFindingsMetadataService).not_to have_received(:execute)
-      end
-
-      context 'when the security scan already exists for the artifact' do
-        let_it_be(:security_scan) { create(:security_scan, build: artifact.job, scan_type: :sast) }
+    it 'calls the `Security::StoreFindingsMetadataService` to store findings' do
+      store_scan
 
-        it 'does not create a new security scan' do
-          expect { store_scan }.not_to change { artifact.job.security_scans.count }
-        end
-      end
-
-      context 'when the security scan does not exist for the artifact' do
-        it 'creates a new security scan' do
-          expect { store_scan }.to change { artifact.job.security_scans.sast.count }.by(1)
-        end
-      end
+      expect(Security::StoreFindingsMetadataService).to have_received(:execute)
     end
 
-    context 'when the `store_security_findings` feature is enabled' do
-      before do
-        stub_feature_flags(store_security_findings: artifact.project)
+    context 'when the security scan already exists for the artifact' do
+      let_it_be(:security_scan) { create(:security_scan, build: artifact.job, scan_type: :sast) }
+      let_it_be(:unique_security_finding) do
+        create(:security_finding,
+               scan: security_scan,
+               position: 0)
       end
 
-      it 'calls the `Security::StoreFindingsMetadataService` to store findings' do
-        store_scan
-
-        expect(Security::StoreFindingsMetadataService).to have_received(:execute)
+      let_it_be(:duplicated_security_finding) do
+        create(:security_finding,
+               scan: security_scan,
+               position: 5)
       end
 
-      context 'when the security scan already exists for the artifact' do
-        let_it_be(:security_scan) { create(:security_scan, build: artifact.job, scan_type: :sast) }
-        let_it_be(:unique_security_finding) do
-          create(:security_finding,
-                 scan: security_scan,
-                 position: 0)
-        end
+      it 'does not create a new security scan' do
+        expect { store_scan }.not_to change { artifact.job.security_scans.count }
+      end
 
-        let_it_be(:duplicated_security_finding) do
-          create(:security_finding,
-                 scan: security_scan,
-                 position: 5)
+      context 'when the `deduplicate` param is set as false' do
+        it 'does not change the deduplicated flag of duplicated finding' do
+          expect { store_scan }.not_to change { duplicated_security_finding.reload.deduplicated }.from(false)
         end
 
-        it 'does not create a new security scan' do
-          expect { store_scan }.not_to change { artifact.job.security_scans.count }
+        it 'does not change the deduplicated flag of unique finding' do
+          expect { store_scan }.not_to change { unique_security_finding.reload.deduplicated }.from(false)
         end
+      end
 
-        context 'when the `deduplicate` param is set as false' do
-          it 'does not change the deduplicated flag of duplicated finding' do
-            expect { store_scan }.not_to change { duplicated_security_finding.reload.deduplicated }.from(false)
-          end
+      context 'when the `deduplicate` param is set as true' do
+        let(:deduplicate) { true }
 
-          it 'does not change the deduplicated flag of unique finding' do
-            expect { store_scan }.not_to change { unique_security_finding.reload.deduplicated }.from(false)
-          end
+        it 'does not change the deduplicated flag of duplicated finding false' do
+          expect { store_scan }.not_to change { duplicated_security_finding.reload.deduplicated }.from(false)
         end
 
-        context 'when the `deduplicate` param is set as true' do
-          let(:deduplicate) { true }
+        it 'sets the deduplicated flag of unique finding as true' do
+          expect { store_scan }.to change { unique_security_finding.reload.deduplicated }.to(true)
+        end
+      end
+    end
 
-          it 'does not change the deduplicated flag of duplicated finding false' do
-            expect { store_scan }.not_to change { duplicated_security_finding.reload.deduplicated }.from(false)
-          end
+    context 'when the security scan does not exist for the artifact' do
+      let(:unique_finding_attribute) do
+        -> { Security::Finding.by_position(0).first&.deduplicated }
+      end
 
-          it 'sets the deduplicated flag of unique finding as true' do
-            expect { store_scan }.to change { unique_security_finding.reload.deduplicated }.to(true)
-          end
-        end
+      let(:duplicated_finding_attribute) do
+        -> { Security::Finding.by_position(5).first&.deduplicated }
       end
 
-      context 'when the security scan does not exist for the artifact' do
-        let(:unique_finding_attribute) do
-          -> { Security::Finding.by_position(0).first&.deduplicated }
-        end
+      before do
+        allow(Security::StoreFindingsMetadataService).to receive(:execute).and_call_original
+      end
 
-        let(:duplicated_finding_attribute) do
-          -> { Security::Finding.by_position(5).first&.deduplicated }
-        end
+      it 'creates a new security scan' do
+        expect { store_scan }.to change { artifact.job.security_scans.sast.count }.by(1)
+      end
 
-        before do
-          allow(Security::StoreFindingsMetadataService).to receive(:execute).and_call_original
+      context 'when the `deduplicate` param is set as false' do
+        it 'sets the deduplicated flag of duplicated finding as false' do
+          expect { store_scan }.to change { duplicated_finding_attribute.call }.to(false)
         end
 
-        it 'creates a new security scan' do
-          expect { store_scan }.to change { artifact.job.security_scans.sast.count }.by(1)
+        it 'sets the deduplicated flag of unique finding as true' do
+          expect { store_scan }.to change { unique_finding_attribute.call }.to(true)
         end
+      end
 
-        context 'when the `deduplicate` param is set as false' do
-          it 'sets the deduplicated flag of duplicated finding as false' do
-            expect { store_scan }.to change { duplicated_finding_attribute.call }.to(false)
-          end
+      context 'when the `deduplicate` param is set as true' do
+        let(:deduplicate) { true }
 
-          it 'sets the deduplicated flag of unique finding as true' do
-            expect { store_scan }.to change { unique_finding_attribute.call }.to(true)
-          end
+        it 'sets the deduplicated flag of duplicated finding false' do
+          expect { store_scan }.to change { duplicated_finding_attribute.call }.to(false)
         end
 
-        context 'when the `deduplicate` param is set as true' do
-          let(:deduplicate) { true }
-
-          it 'sets the deduplicated flag of duplicated finding false' do
-            expect { store_scan }.to change { duplicated_finding_attribute.call }.to(false)
-          end
-
-          it 'sets the deduplicated flag of unique finding as true' do
-            expect { store_scan }.to change { unique_finding_attribute.call }.to(true)
-          end
+        it 'sets the deduplicated flag of unique finding as true' do
+          expect { store_scan }.to change { unique_finding_attribute.call }.to(true)
         end
       end
     end