Merge branch '32773-include-test-results-within-release-evidence-5' into 'master'

Add pipeline parameter for release evidence collection

See merge request gitlab-org/gitlab!33963

app/controllers/projects/tags_controller.rb

@@ -41,16 +41,20 @@ class Projects::TagsController < Projects::ApplicationController
   # rubocop: enable CodeReuse/ActiveRecord
 
   def create
+    # TODO: remove this with the release creation moved to it's own form https://gitlab.com/gitlab-org/gitlab/-/issues/214245
+    evidence_pipeline = find_evidence_pipeline
+
     result = ::Tags::CreateService.new(@project, current_user)
       .execute(params[:tag_name], params[:ref], params[:message])
 
     if result[:status] == :success
-      # Release creation with Tags was deprecated in GitLab 11.7
+      # TODO: remove this with the release creation moved to it's own form https://gitlab.com/gitlab-org/gitlab/-/issues/214245
       if params[:release_description].present?
         release_params = {
           tag: params[:tag_name],
           name: params[:tag_name],
-          description: params[:release_description]
+          description: params[:release_description],
+          evidence_pipeline: evidence_pipeline
         }
 
         Releases::CreateService
@@ -93,4 +97,14 @@ class Projects::TagsController < Projects::ApplicationController
       end
     end
   end
+
+  private
+
+  # TODO: remove this with the release creation moved to it's own form https://gitlab.com/gitlab-org/gitlab/-/issues/214245
+  def find_evidence_pipeline
+    evidence_pipeline_sha = @project.repository.commit(params[:ref])&.sha
+    return unless evidence_pipeline_sha
+
+    @project.ci_pipelines.for_sha(evidence_pipeline_sha).last
+  end
 end

app/services/releases/create_evidence_service.rb

@@ -2,8 +2,9 @@
 
 module Releases
   class CreateEvidenceService
-    def initialize(release)
+    def initialize(release, pipeline: nil)
       @release = release
+      @pipeline = pipeline
     end
 
     def execute

app/services/releases/create_service.rb

@@ -9,11 +9,16 @@ module Releases
       return error('Release already exists', 409) if release
       return error("Milestone(s) not found: #{inexistent_milestones.join(', ')}", 400) if inexistent_milestones.any?
 
+      # should be found before the creation of new tag
+      # because tag creation can spawn new pipeline
+      # which won't have any data for evidence yet
+      evidence_pipeline = find_evidence_pipeline
+
       tag = ensure_tag
 
       return tag unless tag.is_a?(Gitlab::Git::Tag)
 
-      create_release(tag)
+      create_release(tag, evidence_pipeline)
     end
 
     def find_or_build_release
@@ -42,14 +47,14 @@ module Releases
       Ability.allowed?(current_user, :create_release, project)
     end
 
-    def create_release(tag)
+    def create_release(tag, evidence_pipeline)
       release = build_release(tag)
 
       release.save!
 
       notify_create_release(release)
 
-      create_evidence!(release)
+      create_evidence!(release, evidence_pipeline)
 
       success(tag: tag, release: release)
     rescue => e
@@ -73,13 +78,25 @@ module Releases
       )
     end
 
-    def create_evidence!(release)
+    def find_evidence_pipeline
+      # TODO: remove this with the release creation moved to it's own form https://gitlab.com/gitlab-org/gitlab/-/issues/214245
+      return params[:evidence_pipeline] if params[:evidence_pipeline]
+
+      sha = existing_tag&.dereferenced_target&.sha
+      sha ||= repository.commit(ref)&.sha
+
+      return unless sha
+
+      project.ci_pipelines.for_sha(sha).last
+    end
+
+    def create_evidence!(release, pipeline)
       return if release.historical_release?
 
       if release.upcoming_release?
-        CreateEvidenceWorker.perform_at(release.released_at, release.id)
+        CreateEvidenceWorker.perform_at(release.released_at, release.id, pipeline&.id)
       else
-        CreateEvidenceWorker.perform_async(release.id)
+        CreateEvidenceWorker.perform_async(release.id, pipeline&.id)
       end
     end
   end

app/workers/create_evidence_worker.rb

@@ -6,10 +6,15 @@ class CreateEvidenceWorker # rubocop:disable Scalability/IdempotentWorker
   feature_category :release_evidence
   weight 2
 
-  def perform(release_id)
+  # pipeline_id is optional for backward compatibility with existing jobs
+  # caller should always try to provide the pipeline and pass nil only
+  # if pipeline is absent
+  def perform(release_id, pipeline_id = nil)
     release = Release.find_by_id(release_id)
     return unless release
 
-    ::Releases::CreateEvidenceService.new(release).execute
+    pipeline = Ci::Pipeline.find_by_id(pipeline_id)
+
+    ::Releases::CreateEvidenceService.new(release, pipeline: pipeline).execute
   end
 end

spec/controllers/projects/tags_controller_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe Projects::TagsController do
   let(:project) { create(:project, :public, :repository) }
   let!(:release) { create(:release, project: project) }
   let!(:invalid_release) { create(:release, project: project, tag: 'does-not-exist') }
+  let(:user) { create(:user) }
 
   describe 'GET index' do
     before do
@@ -61,4 +62,69 @@ RSpec.describe Projects::TagsController do
       end
     end
   end
+
+  describe 'POST #create' do
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    let(:release_description) { nil }
+    let(:request) do
+      post(:create, params: {
+             namespace_id: project.namespace.to_param,
+             project_id: project,
+             tag_name: '1.0',
+             ref: 'master',
+             release_description: release_description
+           })
+    end
+
+    it 'creates tag' do
+      request
+
+      expect(response).to have_gitlab_http_status(:found)
+      expect(project.repository.find_tag('1.0')).to be_present
+    end
+
+    # TODO: remove this with the release creation moved to it's own form https://gitlab.com/gitlab-org/gitlab/-/issues/214245
+    context 'when release description is set' do
+      let(:release_description) { 'some release description' }
+
+      it 'creates tag and release' do
+        request
+
+        expect(response).to have_gitlab_http_status(:found)
+        expect(project.repository.find_tag('1.0')).to be_present
+
+        release = project.releases.find_by_tag!('1.0')
+
+        expect(release).to be_present
+        expect(release.description).to eq(release_description)
+      end
+
+      it 'passes the last pipeline for evidence creation', :sidekiq_inline do
+        sha = project.repository.commit('master').sha
+        create(:ci_empty_pipeline, sha: sha, project: project) # old pipeline
+        pipeline = create(:ci_empty_pipeline, sha: sha, project: project)
+
+        # simulating pipeline creation by new tag
+        expect_any_instance_of(Repository).to receive(:add_tag).and_wrap_original do |m, *args|
+          create(:ci_empty_pipeline, sha: sha, project: project)
+          m.call(*args)
+        end
+
+        expect_next_instance_of(Releases::CreateEvidenceService, anything, pipeline: pipeline) do |service|
+          expect(service).to receive(:execute).and_call_original
+        end
+
+        request
+
+        release = project.releases.find_by_tag!('1.0')
+
+        expect(release).to be_present
+        expect(release.description).to eq(release_description)
+      end
+    end
+  end
 end

spec/services/releases/create_service_spec.rb

@@ -188,6 +188,7 @@ describe Releases::CreateService do
   end
 
   context 'Evidence collection' do
+    let(:sha) { project.repository.commit('master').sha }
     let(:params) do
       {
         name: 'New release',
@@ -229,6 +230,75 @@ describe Releases::CreateService do
       end
     end
 
+    shared_examples 'uses the right pipeline for evidence' do
+      it 'creates evidence without pipeline if it does not exist', :sidekiq_inline do
+        expect_next_instance_of(Releases::CreateEvidenceService, anything, pipeline: nil) do |service|
+          expect(service).to receive(:execute).and_call_original
+        end
+
+        expect { subject }.to change(Releases::Evidence, :count).by(1)
+      end
+
+      it 'uses the last pipeline for evidence', :sidekiq_inline do
+        create(:ci_empty_pipeline, sha: sha, project: project) # old pipeline
+        pipeline = create(:ci_empty_pipeline, sha: sha, project: project)
+
+        expect_next_instance_of(Releases::CreateEvidenceService, anything, pipeline: pipeline) do |service|
+          expect(service).to receive(:execute).and_call_original
+        end
+
+        expect { subject }.to change(Releases::Evidence, :count).by(1)
+      end
+
+      context 'when old evidence_pipeline is passed to service' do
+        let!(:old_pipeline) { create(:ci_empty_pipeline, sha: sha, project: project) }
+        let!(:new_pipeline) { create(:ci_empty_pipeline, sha: sha, project: project) }
+        let(:params) do
+          super().merge(
+            evidence_pipeline: old_pipeline
+          )
+        end
+
+        it 'uses the old pipeline for evidence', :sidekiq_inline do
+          expect_next_instance_of(Releases::CreateEvidenceService, anything, pipeline: old_pipeline) do |service|
+            expect(service).to receive(:execute).and_call_original
+          end
+
+          expect { subject }.to change(Releases::Evidence, :count).by(1)
+        end
+      end
+
+      it 'pipeline is still being used for evidence if new pipeline is being created for tag', :sidekiq_inline do
+        pipeline = create(:ci_empty_pipeline, sha: sha, project: project)
+
+        expect(project.repository).to receive(:add_tag).and_wrap_original do |m, *args|
+          create(:ci_empty_pipeline, sha: sha, project: project)
+          m.call(*args)
+        end
+
+        expect_next_instance_of(Releases::CreateEvidenceService, anything, pipeline: pipeline) do |service|
+          expect(service).to receive(:execute).and_call_original
+        end
+
+        expect { subject }.to change(Releases::Evidence, :count).by(1)
+      end
+
+      it 'uses the last pipeline for evidence when tag is already created', :sidekiq_inline do
+        Tags::CreateService.new(project, user).execute('v0.1', 'master', nil)
+
+        expect(project.repository.find_tag('v0.1')).to be_present
+
+        create(:ci_empty_pipeline, sha: sha, project: project) # old pipeline
+        pipeline = create(:ci_empty_pipeline, sha: sha, project: project)
+
+        expect_next_instance_of(Releases::CreateEvidenceService, anything, pipeline: pipeline) do |service|
+          expect(service).to receive(:execute).and_call_original
+        end
+
+        expect { subject }.to change(Releases::Evidence, :count).by(1)
+      end
+    end
+
     context 'immediate release' do
       let(:released_at) { nil }
 
@@ -257,6 +327,8 @@ describe Releases::CreateService do
 
         expect(last_release.upcoming_release?).to be_falsy
       end
+
+      include_examples 'uses the right pipeline for evidence'
     end
 
     context 'upcoming release' do
@@ -287,6 +359,8 @@ describe Releases::CreateService do
 
         expect(last_release.upcoming_release?).to be_truthy
       end
+
+      include_examples 'uses the right pipeline for evidence'
     end
   end
 end

spec/workers/create_evidence_worker_spec.rb

@@ -3,13 +3,24 @@
 require 'spec_helper'
 
 describe CreateEvidenceWorker do
-  let(:release) { create(:release) }
+  let(:project) { create(:project, :repository) }
+  let(:release) { create(:release, project: project) }
+  let(:pipeline) { create(:ci_empty_pipeline, sha: release.sha, project: project) }
 
+  # support old scheduled workers without pipeline
   it 'creates a new Evidence record' do
-    expect_next_instance_of(::Releases::CreateEvidenceService, release) do |service|
+    expect_next_instance_of(::Releases::CreateEvidenceService, release, pipeline: nil) do |service|
       expect(service).to receive(:execute).and_call_original
     end
 
     expect { described_class.new.perform(release.id) }.to change(Releases::Evidence, :count).by(1)
   end
+
+  it 'creates a new Evidence record with pipeline' do
+    expect_next_instance_of(::Releases::CreateEvidenceService, release, pipeline: pipeline) do |service|
+      expect(service).to receive(:execute).and_call_original
+    end
+
+    expect { described_class.new.perform(release.id, pipeline.id) }.to change(Releases::Evidence, :count).by(1)
+  end
 end