Merge branch '208322-fix-build-sha-check' into 'master'

Add pages_smart_check_outdated_sha feature flag

See merge request gitlab-org/gitlab!67303

app/services/projects/update_pages_service.rb

@@ -149,7 +149,9 @@ module Projects
       File.open(artifacts_path) do |file|
         deployment = project.pages_deployments.create!(file: file,
                                                        file_count: entries_count,
-                                                       file_sha256: sha256)
+                                                       file_sha256: sha256,
+                                                       ci_build_id: build.id
+                                                      )
 
         validate_outdated_sha!
 
@@ -236,7 +238,17 @@ module Projects
     end
 
     def validate_outdated_sha!
-      raise InvalidStateError, 'build SHA is outdated for this ref' unless latest?
+      return if latest?
+
+      if Feature.enabled?(:pages_smart_check_outdated_sha, project, default_enabled: :yaml)
+        # use pipeline_id in case the build is retried
+        last_deployed_pipeline_id = project.pages_metadatum&.pages_deployment&.ci_build&.pipeline_id
+
+        return unless last_deployed_pipeline_id
+        return if last_deployed_pipeline_id <= build.pipeline_id
+      end
+
+      raise InvalidStateError, 'build SHA is outdated for this ref'
     end
 
     def latest?

config/feature_flags/development/pages_smart_check_outdated_sha.yml

+---
+name: pages_smart_check_outdated_sha
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67303
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/336574
+milestone: '14.2'
+type: development
+group: group::release
+default_enabled: false

spec/services/projects/update_pages_service_spec.rb

@@ -4,6 +4,8 @@ require "spec_helper"
 
 RSpec.describe Projects::UpdatePagesService do
   let_it_be(:project, refind: true) { create(:project, :repository) }
+
+  let_it_be(:old_pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) }
   let_it_be(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) }
 
   let(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') }
@@ -94,6 +96,7 @@ RSpec.describe Projects::UpdatePagesService do
         expect(deployment.file_count).to eq(3)
         expect(deployment.file_sha256).to eq(artifacts_archive.file_sha256)
         expect(project.pages_metadatum.reload.pages_deployment_id).to eq(deployment.id)
+        expect(deployment.ci_build_id).to eq(build.id)
       end
 
       it 'fails if another deployment is in progress' do
@@ -106,19 +109,6 @@ RSpec.describe Projects::UpdatePagesService do
         end
       end
 
-      it 'fails if sha on branch was updated before deployment was uploaded' do
-        expect(subject).to receive(:create_pages_deployment).and_wrap_original do |m, *args|
-          build.update!(ref: 'feature')
-          m.call(*args)
-        end
-
-        expect(execute).not_to eq(:success)
-        expect(project.pages_metadatum).not_to be_deployed
-
-        expect(deploy_status).to be_failed
-        expect(deploy_status.description).to eq('build SHA is outdated for this ref')
-      end
-
       it 'does not fail if pages_metadata is absent' do
         project.pages_metadatum.destroy!
         project.reload
@@ -190,16 +180,6 @@ RSpec.describe Projects::UpdatePagesService do
         expect(ProjectPagesMetadatum.find_by_project_id(project)).to be_nil
       end
 
-      it 'fails if sha on branch is not latest' do
-        build.update!(ref: 'feature')
-
-        expect(execute).not_to eq(:success)
-        expect(project.pages_metadatum).not_to be_deployed
-
-        expect(deploy_status).to be_failed
-        expect(deploy_status.description).to eq('build SHA is outdated for this ref')
-      end
-
       context 'when using empty file' do
         let(:file) { empty_file }
 
@@ -274,6 +254,75 @@ RSpec.describe Projects::UpdatePagesService do
           expect(execute).to eq(:success)
         end
       end
+
+      context "when sha on branch was updated before deployment was uploaded" do
+        before do
+          expect(subject).to receive(:create_pages_deployment).and_wrap_original do |m, *args|
+            build.update!(ref: 'feature')
+            m.call(*args)
+          end
+        end
+
+        shared_examples 'fails with outdated reference message' do
+          it 'fails' do
+            expect(execute).not_to eq(:success)
+            expect(project.reload.pages_metadatum).not_to be_deployed
+
+            expect(deploy_status).to be_failed
+            expect(deploy_status.description).to eq('build SHA is outdated for this ref')
+          end
+        end
+
+        shared_examples 'successfully deploys' do
+          it 'succeeds' do
+            expect do
+              expect(execute).to eq(:success)
+            end.to change { project.pages_deployments.count }.by(1)
+
+            deployment = project.pages_deployments.last
+            expect(deployment.ci_build_id).to eq(build.id)
+          end
+        end
+
+        include_examples 'successfully deploys'
+
+        context 'when pages_smart_check_outdated_sha feature flag is disabled' do
+          before do
+            stub_feature_flags(pages_smart_check_outdated_sha: false)
+          end
+
+          include_examples 'fails with outdated reference message'
+        end
+
+        context 'when old deployment present' do
+          before do
+            old_build = create(:ci_build, pipeline: old_pipeline, ref: 'HEAD')
+            old_deployment = create(:pages_deployment, ci_build: old_build, project: project)
+            project.update_pages_deployment!(old_deployment)
+          end
+
+          include_examples 'successfully deploys'
+
+          context 'when pages_smart_check_outdated_sha feature flag is disabled' do
+            before do
+              stub_feature_flags(pages_smart_check_outdated_sha: false)
+            end
+
+            include_examples 'fails with outdated reference message'
+          end
+        end
+
+        context 'when newer deployment present' do
+          before do
+            new_pipeline = create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha)
+            new_build = create(:ci_build, pipeline: new_pipeline, ref: 'HEAD')
+            new_deployment = create(:pages_deployment, ci_build: new_build, project: project)
+            project.update_pages_deployment!(new_deployment)
+          end
+
+          include_examples 'fails with outdated reference message'
+        end
+      end
     end
   end