Validate MRs are present in security issues
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 | |
With ~"reduced backports" label | https://gitlab.com/gitlab-org/security/gitlab/-/issues/967#note_1600888721 |
Author Check-list
- [-] Has documentation been updated?
Edited by Steve Abrams