From 436b4264f8122902d2c5a0c17b4f3cf5a0b6e048 Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Wed, 27 Sep 2023 14:38:41 -0600 Subject: [PATCH] Skip some finalize jobs on critical security releases A collection of jobs is executed at the end of the security release via security pipeline. However, there are some jobs that only apply for regular security releases and not critical because they don't have tracking issue associated or because they're triggered on-demand. This commit updates the following jobs to be skipped if they're executed in the context of a security release: * `Finalize::CloseImplementationIssues` * `Finalize::CloseTrackingIssue` * `Finalize::NotifyNextReleaseManagers` * `Finalize::UpdateSlackBookmark` Related to https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/19661 --- .../finalize/close_implementation_issues.rb | 25 ++++++++++------ .../security/finalize/close_tracking_issue.rb | 6 ++++ .../finalize/notify_next_release_managers.rb | 6 ++++ .../finalize/update_slack_bookmark.rb | 19 +++++++----- lib/release_tools/security/issue_helper.rb | 6 +++- lib/tasks/security.rake | 2 ++ .../close_implementation_issues_spec.rb | 29 +++++++++++++++++++ .../finalize/close_tracking_issue_spec.rb | 16 ++++++++++ .../notify_next_release_managers_spec.rb | 16 ++++++++++ .../finalize/update_slack_bookmark_spec.rb | 16 ++++++++++ 10 files changed, 124 insertions(+), 17 deletions(-) diff --git a/lib/release_tools/security/finalize/close_implementation_issues.rb b/lib/release_tools/security/finalize/close_implementation_issues.rb index f518d674b..3e39a30a8 100644 --- a/lib/release_tools/security/finalize/close_implementation_issues.rb +++ b/lib/release_tools/security/finalize/close_implementation_issues.rb @@ -14,18 +14,13 @@ module ReleaseTools end def execute - return if security_issues.empty? + if critical_security_release? + logger.info('Critical security release, skipping job') - security_issues.each do |issue| - next unless issue.processed? - - logger.info('Security implementation issue processed', issue: issue.web_url) - - next if SharedStatus.dry_run? - - client.close_issue(issue.project_id, issue) + return end + close_security_issues send_slack_notification(:success) rescue Gitlab::Error::Error => ex logger.fatal(error_message, error: ex) @@ -45,6 +40,18 @@ module ReleaseTools .upcoming_security_issues_and_merge_requests end + def close_security_issues + security_issues.each do |issue| + next unless issue.processed? + + logger.info('Security implementation issue processed', issue: issue.web_url) + + next if SharedStatus.dry_run? + + client.close_issue(issue.project_id, issue) + end + end + def send_slack_notification(status) ReleaseTools::Slack::ReleaseJobEndNotifier.new( job_type: 'Close security issues', diff --git a/lib/release_tools/security/finalize/close_tracking_issue.rb b/lib/release_tools/security/finalize/close_tracking_issue.rb index 91d705dd0..8f4f65d9f 100644 --- a/lib/release_tools/security/finalize/close_tracking_issue.rb +++ b/lib/release_tools/security/finalize/close_tracking_issue.rb @@ -14,6 +14,12 @@ module ReleaseTools end def execute + if critical_security_release? + logger.info('Critical security release, skipping job') + + return + end + if security_tracking_issue.present? close_security_tracking_issue send_slack_notification(:success) diff --git a/lib/release_tools/security/finalize/notify_next_release_managers.rb b/lib/release_tools/security/finalize/notify_next_release_managers.rb index 3d730078f..6b31ba2d1 100644 --- a/lib/release_tools/security/finalize/notify_next_release_managers.rb +++ b/lib/release_tools/security/finalize/notify_next_release_managers.rb @@ -10,6 +10,12 @@ module ReleaseTools CouldNotNotifyError = Class.new(StandardError) def execute + if critical_security_release? + logger.info('Critical security release, skipping job') + + return + end + if security_tracking_issue.present? logger.info( 'Notify next release managers about the next security release', diff --git a/lib/release_tools/security/finalize/update_slack_bookmark.rb b/lib/release_tools/security/finalize/update_slack_bookmark.rb index ed8800c53..ac68049ab 100644 --- a/lib/release_tools/security/finalize/update_slack_bookmark.rb +++ b/lib/release_tools/security/finalize/update_slack_bookmark.rb @@ -11,14 +11,15 @@ module ReleaseTools CouldNotNotifyError = Class.new(StandardError) def execute - logger.info( - 'Updating slack bookmark in releases channel', - issue: security_tracking_issue&.iid - ) + if critical_security_release? + logger.info('Critical security release, skipping job') - if security_tracking_issue.present? - return if SharedStatus.dry_run? + return + end + + return if SharedStatus.dry_run? + if security_tracking_issue.present? update_bookmark send_slack_notification(:success) else @@ -28,7 +29,6 @@ module ReleaseTools end rescue StandardError => ex logger.fatal(failure_message, error: ex) - send_slack_notification(:failed) raise CouldNotNotifyError @@ -37,6 +37,11 @@ module ReleaseTools private def update_bookmark + logger.info( + 'Updating slack bookmark in releases channel', + issue: security_tracking_issue&.iid + ) + unless security_bookmark logger.fatal('Bookmark could not be found') diff --git a/lib/release_tools/security/issue_helper.rb b/lib/release_tools/security/issue_helper.rb index 023d348de..18698208a 100644 --- a/lib/release_tools/security/issue_helper.rb +++ b/lib/release_tools/security/issue_helper.rb @@ -16,12 +16,16 @@ module ReleaseTools end def reference_issue - if ReleaseTools::SharedStatus.critical_security_release? + if critical_security_release? security_task_issue else security_tracking_issue end end + + def critical_security_release? + ReleaseTools::SharedStatus.critical_security_release? + end end end end diff --git a/lib/tasks/security.rake b/lib/tasks/security.rake index 7c13cef24..3b75ad745 100644 --- a/lib/tasks/security.rake +++ b/lib/tasks/security.rake @@ -188,6 +188,8 @@ namespace :security do updater = ReleaseTools::Security::Finalize::CloseTrackingIssue.new updater.execute + return if ReleaseTools::SharedStatus.critical_security_release + security_tracking_issue = ReleaseTools::Security::TrackingIssue.new create_or_show_issue(security_tracking_issue) end diff --git a/spec/lib/release_tools/security/finalize/close_implementation_issues_spec.rb b/spec/lib/release_tools/security/finalize/close_implementation_issues_spec.rb index b8f1aa2db..6503c0f58 100644 --- a/spec/lib/release_tools/security/finalize/close_implementation_issues_spec.rb +++ b/spec/lib/release_tools/security/finalize/close_implementation_issues_spec.rb @@ -105,6 +105,9 @@ describe ReleaseTools::Security::Finalize::CloseImplementationIssues do expect(client).not_to receive(:close_issue) + expect(notifier) + .to receive(:send_notification) + without_dry_run do close_implementation_issues.execute end @@ -155,5 +158,31 @@ describe ReleaseTools::Security::Finalize::CloseImplementationIssues do end end end + + context 'with a critical security release' do + around do |ex| + env = { + 'SECURITY_RELEASE_PIPELINE' => 'true', + 'CI_JOB_URL' => 'https://example.com/foo/bar/-/jobs/1', + 'SECURITY' => 'critical' + } + + ClimateControl.modify(env) do + ex.run + end + end + + it 'does nothing' do + expect(client) + .not_to receive(:close_issue) + + expect(notifier) + .not_to receive(:send_notification) + + without_dry_run do + close_implementation_issues.execute + end + end + end end end diff --git a/spec/lib/release_tools/security/finalize/close_tracking_issue_spec.rb b/spec/lib/release_tools/security/finalize/close_tracking_issue_spec.rb index a7427717d..95095939f 100644 --- a/spec/lib/release_tools/security/finalize/close_tracking_issue_spec.rb +++ b/spec/lib/release_tools/security/finalize/close_tracking_issue_spec.rb @@ -78,5 +78,21 @@ describe ReleaseTools::Security::Finalize::CloseTrackingIssue do end end end + + context 'with a critical security release' do + it 'does nothing' do + expect(client) + .not_to receive(:close_issue) + + expect(notifier) + .not_to receive(:send_notification) + + ClimateControl.modify(SECURITY: 'critical') do + without_dry_run do + close_tracking_issue.execute + end + end + end + end end end diff --git a/spec/lib/release_tools/security/finalize/notify_next_release_managers_spec.rb b/spec/lib/release_tools/security/finalize/notify_next_release_managers_spec.rb index 09783993b..43b4ab54e 100644 --- a/spec/lib/release_tools/security/finalize/notify_next_release_managers_spec.rb +++ b/spec/lib/release_tools/security/finalize/notify_next_release_managers_spec.rb @@ -85,5 +85,21 @@ describe ReleaseTools::Security::Finalize::NotifyNextReleaseManagers do notify_next_release_managers.execute end end + + context 'with a critical security release' do + it 'does nothing' do + expect(client) + .not_to receive(:create_issue_note) + + expect(notifier) + .not_to receive(:send_notification) + + ClimateControl.modify(SECURITY: 'critical') do + without_dry_run do + notify_next_release_managers.execute + end + end + end + end end end diff --git a/spec/lib/release_tools/security/finalize/update_slack_bookmark_spec.rb b/spec/lib/release_tools/security/finalize/update_slack_bookmark_spec.rb index 1faef7a9d..8712ee6c9 100644 --- a/spec/lib/release_tools/security/finalize/update_slack_bookmark_spec.rb +++ b/spec/lib/release_tools/security/finalize/update_slack_bookmark_spec.rb @@ -142,5 +142,21 @@ describe ReleaseTools::Security::Finalize::UpdateSlackBookmark do update_bookmark end end + + context 'with a critical security release' do + it 'does nothing' do + expect(ReleaseTools::Slack::Bookmark) + .not_to receive(:edit) + + expect(notifier) + .not_to receive(:new) + + ClimateControl.modify(SECURITY: 'critical') do + without_dry_run do + update_bookmark + end + end + end + end end end -- GitLab