Skip to content

Validate MRs are present in security issues

Steve Abrams requested to merge reject-no-mr-reduced-backports into master

What does this MR do and why?

Adds validation to check that there is at least one MR associated with a security implementation issue when checking readiness. This was previously able to be bypassed if the ~"reduced backports" label was present.

Testing

I modified the local code so it would only evaluate my test issue:

git diff
diff --git a/lib/release_tools/security/issue_crawler.rb b/lib/release_tools/security/issue_crawler.rb
index 6c189910..733b723e 100644
--- a/lib/release_tools/security/issue_crawler.rb
+++ b/lib/release_tools/security/issue_crawler.rb
@@ -98,7 +98,7 @@ module ReleaseTools
       # Returns issues that are related to the security release tracking issue.
       def related_security_issues
         security_issues_and_merge_requests_for(
-          security_issues_for(release_issue.iid)
+          security_issues_for(424529)
         )
       end

@@ -106,7 +106,7 @@ module ReleaseTools
       # are ready to be evaluated for the next security release.
       def evaluable_security_issues
         found_issues = ReleaseTools::ManagedVersioning::PROJECTS.flat_map do |project|
-          GitlabClient.issues(project.security_path, labels: [SECURITY_TARGET_LABEL], state: OPENED)
+          GitlabClient.issues(project.security_path, labels: [SECURITY_TARGET_LABEL, 'test'], state: OPENED)
         end.compact

         security_issues_and_merge_requests_for(found_issues)
@@ -115,7 +115,7 @@ module ReleaseTools
       def notifiable_security_issues_for(project)
         found_issues = GitlabClient.issues(
           project.security_path,
-          labels: [SECURITY_NOTIFICATIONS_LABEL],
+          labels: [SECURITY_TARGET_LABEL, 'test'],
           state: OPENED
         )

diff --git a/lib/release_tools/security/target_issues_processor.rb b/lib/release_tools/security/target_issues_processor.rb
index d207b02a..50acc964 100644
--- a/lib/release_tools/security/target_issues_processor.rb
+++ b/lib/release_tools/security/target_issues_processor.rb
@@ -16,9 +16,11 @@ module ReleaseTools
         logger.info("#{security_target_issues.count} target issues found. They will be evaluated and considered for linking to the security release tracking issue: #{security_release_tracking_issue.web_url}.")

         security_target_issues.each do |target_issue|
+          next unless target_issue.iid == 967
+
           case [target_issue.ready_to_be_processed?, linked_to_security_tracking_issue?(target_issue)]
           when [true, true]
-            logger.info("#{target_issue.web_url} is already linked to the security release tracking issue and still ready to be processed.")
+            # logger.info("#{target_issue.web_url} is already linked to the security release tracking issue and still ready to be processed.")

             nil
           when [false, false]
@@ -30,16 +32,16 @@ module ReleaseTools
             logger.info("#{target_issue.web_url} is ready to be processed and will be linked to the security release tracking issue.")

             link(target_issue)
-            TargetIssueNotifier.notify(:linked, target_issue)
-            SecurityReleaseTrackingIssueNotifier.notify(target_issue)
-            update_security_issue_table
+            # TargetIssueNotifier.notify(:linked, target_issue)
+            # SecurityReleaseTrackingIssueNotifier.notify(target_issue)
+            # update_security_issue_table
           when [false, true]
             logger.info("#{target_issue.web_url} will be unlinked from  the security release tracking issue as it is no longer ready to be processed.")

-            unlink(target_issue)
-            TargetIssueNotifier.notify(:unlinked, target_issue)
-            remove_security_target_label(target_issue)
-            update_security_issue_table
+            # unlink(target_issue)
+            # TargetIssueNotifier.notify(:unlinked, target_issue)
+            # remove_security_target_label(target_issue)
+            # update_security_issue_table
           end
         end
       end

I ran the processor twice ReleaseTools::Security::TargetIssuesProcessor.new.execute:

Scenario Comment Link Screenshot
Without ~"reduced backports" label https://gitlab.com/gitlab-org/security/gitlab/-/issues/967#note_1600887958 Screenshot_2023-10-12_at_11.38.30_AM
With ~"reduced backports" label https://gitlab.com/gitlab-org/security/gitlab/-/issues/967#note_1600888721 Screenshot_2023-10-12_at_11.43.33_AM

Author Check-list

  • [-] Has documentation been updated?
Edited by Steve Abrams

Merge request reports