Merge branch...

Merge branch '386323-projects-vulnerabilityfeedbackcontroller-create-creates-a-subtransaction' into 'master'

Move vulnerability creation out of transaction

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107595



Merged-by: Stan Hu <stanhu@gmail.com>
Approved-by: Michał Zając <mzajac@gitlab.com>
Approved-by: Gregory Havenga <11164960-ghavenga@users.noreply.gitlab.com>
Approved-by: Stan Hu <stanhu@gmail.com>
Co-authored-by: Jonathan Schafer <jschafer@gitlab.com>

ee/app/services/vulnerability_feedback/create_service.rb

@@ -57,6 +57,26 @@ def success(vulnerability_feedback)
     end
 
     def create_issue
+      raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_security_resource, project)
+
+      # If we send a vulnerability_id, it should be valid. We can create a new Vulnerability if there's not one,
+      # but if we expect there to be one and it's not, something went wrong. This has to be outside of the
+      # transaction due to the subtransaction created in ee/app/services/vulnerabilities/create_service.rb.
+      # See issue https://gitlab.com/gitlab-org/gitlab/-/issues/386323 for details.
+      vulnerability_id = vulnerability_feedback.vulnerability_data[:vulnerability_id]
+      vulnerability = if vulnerability_id
+                        project.vulnerabilities.find_by_id(vulnerability_id)
+                      else
+                        vulnerability_response = create_vulnerability
+
+                        if vulnerability_response.error?
+                          vulnerability_feedback.errors.add(:vulnerability, vulnerability_response.message)
+                          return
+                        end
+
+                        vulnerability_response.payload[:vulnerability]
+                      end
+
       # Wrap Feedback and Issue creation in the same transaction
       ApplicationRecord.transaction do
         issue = @params[:issue]
@@ -75,7 +95,7 @@ def create_issue
           issue = result[:issue]
         end
 
-        issue_link_result = create_vulnerability_issue_link(vulnerability_feedback.vulnerability_data[:vulnerability_id], issue)
+        issue_link_result = create_vulnerability_issue_link(vulnerability, issue)
 
         if issue_link_result&.error?
           vulnerability_feedback.errors.add(:issue_link, issue_link_result.message)
@@ -113,23 +133,7 @@ def rollback_merge_request(merge_request)
         ::Branches::DeleteService.new(project, current_user).execute(branch_name)
     end
 
-    def create_vulnerability_issue_link(vulnerability_id, issue)
-      raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_security_resource, project)
-
-      # If we send a vulnerability_id, it should be valid. We can create a new Vulnerability if there's not one,
-      # but if we expect there to be one and it's not, something went wrong.
-      vulnerability = if vulnerability_id
-                        project.vulnerabilities.find_by_id(vulnerability_id)
-                      else
-                        vulnerability_response = create_vulnerability
-
-                        if vulnerability_response.error?
-                          return vulnerability_response
-                        end
-
-                        vulnerability_response.payload[:vulnerability]
-                      end
-
+    def create_vulnerability_issue_link(vulnerability, issue)
       VulnerabilityIssueLinks::CreateService
         .new(current_user, vulnerability, issue, link_type: Vulnerabilities::IssueLink.link_types[:created])
         .execute