diff --git a/lib/release_tools/security/implementation_issue.rb b/lib/release_tools/security/implementation_issue.rb index 9139844da2543b24c7564907f52fe2d5fc8de620..f78365d54279636d9725b6351141607784fa4748 100644 --- a/lib/release_tools/security/implementation_issue.rb +++ b/lib/release_tools/security/implementation_issue.rb @@ -126,6 +126,16 @@ module ReleaseTools merge_requests.all? { |mr| mr.state == MERGED } end + def linked_to_security_tracking_issue? + @linked_to_security_tracking_issue ||= begin + linked_security_issues = ReleaseTools::Security::IssueCrawler.new.related_security_issues + + linked_security_issues.any? do |linked_issue| + linked_issue.iid == iid && linked_issue.project_id == project_id + end + end + end + def allowed_to_early_merge? merge_request_targeting_default_branch&.state == OPENED && PROJECTS_ALLOWED_TO_EARLY_MERGE.include?(merge_request_targeting_default_branch.project_id) diff --git a/lib/release_tools/security/issue_crawler.rb b/lib/release_tools/security/issue_crawler.rb index 49bc83b891819008d3150f7a5408bec10a2b66b6..e96e7369e6b203279fc7b1192c6222ed9d544d42 100644 --- a/lib/release_tools/security/issue_crawler.rb +++ b/lib/release_tools/security/issue_crawler.rb @@ -62,7 +62,7 @@ module ReleaseTools # Returns all security issues for the upcoming security release. def upcoming_security_issues_and_merge_requests if release_issue - security_issues_and_merge_requests_for(release_issue.iid) + security_issues_and_merge_requests_for(related_security_issues) else [] end @@ -93,21 +93,24 @@ module ReleaseTools related_issues end - # Returns all issues on the security mirrors for projects under managed versioning. + # Returns issues that are related to the security release tracking issue. + def related_security_issues + security_issues_for(release_issue.iid) + end + + # Returns issues on the security mirrors for projects under managed versioning that + # are ready to be evaluated for the next security release. def evaluable_security_issues ReleaseTools::ManagedVersioning::PROJECTS.flat_map do |project| GitlabClient.issues(project.security_path, labels: [SECURITY_TARGET_LABEL], state: OPENED) end.compact end - # Returns all security issues and merge requests for the given root - # security issue. + # Returns all security issues and merge requests for a given set of + # security issues. # - # The `release_issue_iid` is the IID of the (confidential) security issue - # opened in the gitlab-org/gitlab project. - def security_issues_and_merge_requests_for(release_issue_iid) - related_issues = security_issues_for(release_issue_iid) - + # `related_issues` is an array of issues returned from GitlabClient + def security_issues_and_merge_requests_for(related_issues) Parallel.map(related_issues, in_threads: Etc.nprocessors) do |issue| mrs = [] diff --git a/spec/factories.rb b/spec/factories.rb index 90090973d0ffcd7f932a335ec7ccc1d8966ad20d..4d9d71488dd8019c1fd29fea0c2fcb1006a9b5d3 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -84,6 +84,7 @@ FactoryBot.define do factory :issue, parent: :gitlab_response do id iid { id } + project_id { generate(:id) } state { 'opened' } assignees { [] } labels { [] } diff --git a/spec/lib/release_tools/security/implementation_issue_spec.rb b/spec/lib/release_tools/security/implementation_issue_spec.rb index f62ea70de6bdb5bef5f40ecf649396e537453602..5bf4fe23a85d0daf2a86f662181175fca53a9b00 100644 --- a/spec/lib/release_tools/security/implementation_issue_spec.rb +++ b/spec/lib/release_tools/security/implementation_issue_spec.rb @@ -521,4 +521,36 @@ describe ReleaseTools::Security::ImplementationIssue do end end end + + describe '#linked_to_security_tracking_issue?' do + let(:issue_crawler) { stub_const('ReleaseTools::Security::IssueCrawler', spy) } + + let(:issue2_response) do + double( + 'Gitlab::ObjectifiedHash', + iid: 11, + project_id: project.iid + ) + end + + subject(:issue) { described_class.new(issue_response, merge_requests) } + + it 'returns false if the security implementation issue is not linked' do + allow(issue_crawler).to receive(:related_security_issues).and_return([]) + + expect(issue).not_to be_linked_to_security_tracking_issue + end + + it 'returns true if the security implementation issue is linked' do + allow(issue_crawler).to receive(:related_security_issues).and_return([issue2_response, issue_response]) + + expect(issue).to be_linked_to_security_tracking_issue + end + + it 'memoizes the result' do + expect(issue_crawler).to receive(:related_security_issues).once + + 2.times { issue.linked_to_security_tracking_issue? } + end + end end diff --git a/spec/lib/release_tools/security/issue_crawler_spec.rb b/spec/lib/release_tools/security/issue_crawler_spec.rb index a60d4fe4c0d6ac1b3b0ccbbab190b51afd582604..72d2a2f77b7bcf64e6dcd4eed4bebb3518b8108d 100644 --- a/spec/lib/release_tools/security/issue_crawler_spec.rb +++ b/spec/lib/release_tools/security/issue_crawler_spec.rb @@ -3,12 +3,47 @@ require 'spec_helper' describe ReleaseTools::Security::IssueCrawler do + let(:issue1) do + create( + :issue, + project_id: 1, + iid: 1, + due_date: '2020-01-04', + labels: [described_class::SECURITY_LABEL], + state: described_class::OPENED, + web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/1' + ) + end + + let(:issue2) do + create( + :issue, + project_id: 1, + iid: 2, + due_date: '2020-01-01', + labels: [], + state: described_class::OPENED, + web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/2' + ) + end + + let(:issue3) do + create( + :issue, + project_id: 2, + iid: 1, + labels: [described_class::SECURITY_LABEL], + state: described_class::OPENED, + web_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/1' + ) + end + + let(:page) { Gitlab::PaginatedResponse.new([issue1, issue2]) } + + subject(:crawler) { described_class.new } + describe '#security_release_issues' do it 'returns all security release issues' do - issue1 = create(:issue, due_date: '2020-01-04') - issue2 = create(:issue, due_date: '2020-01-01') - page = Gitlab::PaginatedResponse.new([issue1, issue2]) - allow(ReleaseTools::GitlabClient) .to receive(:issues) .with( @@ -20,7 +55,7 @@ describe ReleaseTools::Security::IssueCrawler do ) .and_return(page) - expect(described_class.new.security_release_issues) + expect(crawler.security_release_issues) .to eq([issue2, issue1]) end @@ -39,17 +74,13 @@ describe ReleaseTools::Security::IssueCrawler do ) .and_return(page) - expect { described_class.new.security_release_issues } + expect { crawler.security_release_issues } .to raise_error(RuntimeError) end end describe '#release_issue' do it 'returns first security release issue' do - issue1 = create(:issue, due_date: '2020-01-04') - issue2 = create(:issue, due_date: '2020-01-01') - page = Gitlab::PaginatedResponse.new([issue1, issue2]) - allow(ReleaseTools::GitlabClient) .to receive(:issues) .with( @@ -61,7 +92,7 @@ describe ReleaseTools::Security::IssueCrawler do ) .and_return(page) - expect(described_class.new.release_issue) + expect(crawler.release_issue) .to eq(issue2) end end @@ -69,8 +100,6 @@ describe ReleaseTools::Security::IssueCrawler do describe '#upcoming_security_issues_and_merge_requests' do context 'when there are no security release issues' do it 'returns an empty Array' do - crawler = described_class.new - allow(crawler).to receive(:security_release_issues).and_return([]) expect(crawler.upcoming_security_issues_and_merge_requests).to eq([]) @@ -79,64 +108,38 @@ describe ReleaseTools::Security::IssueCrawler do context 'when there are security release issues' do it 'returns all security issues and merge requests for the most recent issue' do - crawler = described_class.new - issue1 = create(:issue, iid: 1) - issue2 = create(:issue, iid: 2) + implementation_issue1 = create(:security_implementation_issue, issue: issue1) + implementation_issue2 = create(:security_implementation_issue, issue: issue2) allow(crawler) .to receive(:security_release_issues) + .and_return([issue3]) + + allow(crawler) + .to receive(:related_security_issues) .and_return([issue1, issue2]) expect(crawler) .to receive(:security_issues_and_merge_requests_for) - .with(issue1.iid) + .with([issue1, issue2]).and_return([implementation_issue1, implementation_issue2]) crawler.upcoming_security_issues_and_merge_requests end end end - describe '#security_issues_for' do - let(:references) do - double( - 'Gitlab::ObjectifiedHash', - short: 432, - relative: '#432', - full: "gitlab-org/security/gitlab#432" - ) - end + describe '#related_security_issues' do + it 'returns issues related to the given release issue iid' do + allow(crawler).to receive(:release_issue).and_return(issue1) - it 'returns the security issues related to the release issue' do - issue1 = create( - :issue, - project_id: 1, - iid: 1, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/1', - references: references - ) + expect(crawler).to receive(:security_issues_for).with(issue1.iid).and_return([issue1]) - issue2 = create( - :issue, - project_id: 1, - iid: 2, - labels: [], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/2', - references: references - ) - - issue3 = create( - :issue, - project_id: 2, - iid: 1, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/1', - references: references - ) + expect(crawler.related_security_issues).to eq([issue1]) + end + end + describe '#security_issues_for' do + it 'returns the security issues related to the release issue' do issue_page = Gitlab::PaginatedResponse.new([issue1, issue2, issue3]) allow(ReleaseTools::GitlabClient.client) @@ -144,7 +147,7 @@ describe ReleaseTools::Security::IssueCrawler do .with(described_class::PUBLIC_PROJECT, 1) .and_return(issue_page) - issues = described_class.new.security_issues_for(1) + issues = crawler.security_issues_for(1) expect(issues.length).to eq(1) expect(issues[0].project_id).to eq(1) @@ -152,34 +155,21 @@ describe ReleaseTools::Security::IssueCrawler do end it 'filters out closed issues' do - issue1 = create( - :issue, - project_id: 1, - iid: 1, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/1', - references: references - ) - issue2 = create( :issue, project_id: 1, iid: 2, labels: [], state: 'closed', - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/2', - references: references + web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/2' ) - issue_page = Gitlab::PaginatedResponse.new([issue1, issue2]) - allow(ReleaseTools::GitlabClient.client) .to receive(:issue_links) .with(described_class::PUBLIC_PROJECT, 1) - .and_return(issue_page) + .and_return(page) - issues = described_class.new.security_issues_for(1) + issues = crawler.security_issues_for(1) expect(issues.length).to eq(1) expect(issues).not_to include(issue2) @@ -187,9 +177,6 @@ describe ReleaseTools::Security::IssueCrawler do end describe '#evaluable_security_issues' do - let(:issue1) { create(:issue, project_id: 1, iid: 1) } - let(:issue2) { create(:issue, project_id: 1, iid: 2) } - let(:issue3) { create(:issue, project_id: 1, iid: 3) } let(:issue4) { create(:issue, project_id: 2, iid: 1) } let(:project1_issue_page) { Gitlab::PaginatedResponse.new([issue1, issue2, issue3]) } @@ -213,53 +200,14 @@ describe ReleaseTools::Security::IssueCrawler do .with(ReleaseTools::Project::GitlabPages.security_path, labels: [described_class::SECURITY_TARGET_LABEL], state: described_class::OPENED) .and_return(nil) - issues = described_class.new.evaluable_security_issues + issues = crawler.evaluable_security_issues expect(issues).to eq([issue1, issue2, issue3, issue4]) end end describe '#security_issues_and_merge_requests_for' do - let(:references) do - double( - 'Gitlab::ObjectifiedHash', - short: 431, - relative: '#431', - full: "gitlab-org/security/gitlab#431" - ) - end - - it 'returns the security issues and merge requests related to the release issue' do - issue1 = create( - :issue, - project_id: 1, - iid: 1, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/1', - references: references - ) - - issue2 = create( - :issue, - project_id: 1, - iid: 2, - labels: [], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/2', - references: references - ) - - issue3 = create( - :issue, - project_id: 2, - iid: 1, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/1', - references: references - ) - + it 'returns the security issues and merge requests related to the given issues' do mr1 = create( :merge_request, labels: [described_class::SECURITY_LABEL], @@ -294,72 +242,25 @@ describe ReleaseTools::Security::IssueCrawler do .with(1, 1) .and_return(mr_page) - issues = described_class.new.security_issues_and_merge_requests_for(1) - - expect(issues.length).to eq(1) - expect(issues[0].project_id).to eq(1) - expect(issues[0].iid).to eq(1) - expect(issues[0].merge_requests).to eq([mr1]) - end - - it 'filters out closed issues' do - issue1 = create( - :issue, - project_id: 1, - iid: 1, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/1', - references: references - ) - - issue2 = create( - :issue, - project_id: 1, - iid: 2, - labels: [], - state: 'closed', - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/2', - references: references - ) - - mr1 = create( - :merge_request, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1' - ) - - issue_page = Gitlab::PaginatedResponse.new([issue1, issue2]) - mr_page = Gitlab::PaginatedResponse.new([mr1]) - - allow(ReleaseTools::GitlabClient.client) - .to receive(:issue_links) - .with(described_class::PUBLIC_PROJECT, 1) - .and_return(issue_page) + allow(ReleaseTools::GitlabClient) + .to receive(:related_merge_requests) + .with(1, 2) + .and_return(mr_page) allow(ReleaseTools::GitlabClient) .to receive(:related_merge_requests) - .with(1, 1) + .with(2, 1) .and_return(mr_page) - issues = described_class.new.security_issues_and_merge_requests_for(1) + issues = crawler.security_issues_and_merge_requests_for(issue_page) - expect(issues.length).to eq(1) - expect(issues).not_to include(issue2) + expect(issues.length).to eq(3) + expect(issues[0].project_id).to eq(1) + expect(issues[0].iid).to eq(1) + expect(issues[0].merge_requests).to eq([mr1]) end it 'filters out closed merge requests' do - issue1 = create( - :issue, - project_id: 1, - iid: 1, - labels: [described_class::SECURITY_LABEL], - state: described_class::OPENED, - web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/issues/1', - references: references - ) - mr1 = create( :merge_request, labels: [described_class::SECURITY_LABEL], @@ -381,21 +282,15 @@ describe ReleaseTools::Security::IssueCrawler do web_url: 'https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/3' ) - release_issue = 12_345 issue_page = Gitlab::PaginatedResponse.new([issue1]) mr_page = Gitlab::PaginatedResponse.new([mr1, mr2, mr3]) - allow(ReleaseTools::GitlabClient.client) - .to receive(:issue_links) - .with(described_class::PUBLIC_PROJECT, release_issue) - .and_return(issue_page) - allow(ReleaseTools::GitlabClient) .to receive(:related_merge_requests) .with(issue1.project_id, issue1.iid) .and_return(mr_page) - issues = described_class.new.security_issues_and_merge_requests_for(release_issue) + issues = crawler.security_issues_and_merge_requests_for(issue_page) expect(issues[0].merge_requests.length).to eq(2) expect(issues[0].merge_requests).to match_array([mr1, mr3])