Add latest changes from gitlab-org/gitlab@14-8-stable-ee

9 files changed, 151 insertions, 8 deletions

diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb
index 78170fab7a7..d8da448a323 100644
--- a/app/controllers/projects/merge_requests/application_controller.rb
+++ b/app/controllers/projects/merge_requests/application_controller.rb
@@ -44,6 +44,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
       :task_num,
       :title,
       :discussion_locked,
+      :issue_iid,
       label_ids: [],
       assignee_ids: [],
       reviewer_ids: [],
diff --git a/app/services/ci/job_artifacts/destroy_associations_service.rb b/app/services/ci/job_artifacts/destroy_associations_service.rb
index 794d24eadf2..08d7f7f6f02 100644
--- a/app/services/ci/job_artifacts/destroy_associations_service.rb
+++ b/app/services/ci/job_artifacts/destroy_associations_service.rb
@@ -12,7 +12,7 @@ module Ci
 
       def destroy_records
         @job_artifacts_relation.each_batch(of: BATCH_SIZE) do |relation|
-          service = Ci::JobArtifacts::DestroyBatchService.new(relation, pick_up_at: Time.current)
+          service = Ci::JobArtifacts::DestroyBatchService.new(relation, pick_up_at: Time.current, fix_expire_at: false)
           result  = service.execute(update_stats: false)
           updates = result[:statistics_updates]
 
diff --git a/app/services/ci/job_artifacts/destroy_batch_service.rb b/app/services/ci/job_artifacts/destroy_batch_service.rb
index 866b40c32d8..d5a0a2dd885 100644
--- a/app/services/ci/job_artifacts/destroy_batch_service.rb
+++ b/app/services/ci/job_artifacts/destroy_batch_service.rb
@@ -17,13 +17,18 @@ module Ci
       # +pick_up_at+:: When to pick up for deletion of files
       # Returns:
       # +Hash+:: A hash with status and destroyed_artifacts_count keys
-      def initialize(job_artifacts, pick_up_at: nil)
+      def initialize(job_artifacts, pick_up_at: nil, fix_expire_at: fix_expire_at?)
         @job_artifacts = job_artifacts.with_destroy_preloads.to_a
         @pick_up_at = pick_up_at
+        @fix_expire_at = fix_expire_at
       end
 
       # rubocop: disable CodeReuse/ActiveRecord
       def execute(update_stats: true)
+        # Detect and fix artifacts that had `expire_at` wrongly backfilled by migration
+        # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47723
+        detect_and_fix_wrongly_expired_artifacts
+
         return success(destroyed_artifacts_count: 0, statistics_updates: {}) if @job_artifacts.empty?
 
         destroy_related_records(@job_artifacts)
@@ -89,6 +94,55 @@ module Ci
           @job_artifacts.sum { |artifact| artifact.try(:size) || 0 }
         end
       end
+
+      # This detects and fixes job artifacts that have `expire_at` wrongly backfilled by the migration
+      # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47723.
+      # These job artifacts will not be deleted and will have their `expire_at` removed.
+      #
+      # The migration would have backfilled `expire_at`
+      # to midnight on the 22nd of the month of the local timezone,
+      # storing it as UTC time in the database.
+      #
+      # If the timezone setting has changed since the migration,
+      # the `expire_at` stored in the database could have changed to a different local time other than midnight.
+      # For example:
+      # - changing timezone from UTC+02:00 to UTC+02:30 would change the `expire_at` in local time 00:00:00 to 00:30:00.
+      # - changing timezone from UTC+00:00 to UTC-01:00 would change the `expire_at` in local time 00:00:00 to 23:00:00 on the previous day (21st).
+      #
+      # Therefore job artifacts that have `expire_at` exactly on the 00, 30 or 45 minute mark
+      # on the dates 21, 22, 23 of the month will not be deleted.
+      # https://en.wikipedia.org/wiki/List_of_UTC_time_offsets
+      def detect_and_fix_wrongly_expired_artifacts
+        return unless @fix_expire_at
+
+        wrongly_expired_artifacts, @job_artifacts = @job_artifacts.partition { |artifact| wrongly_expired?(artifact) }
+
+        remove_expire_at(wrongly_expired_artifacts)
+      end
+
+      def fix_expire_at?
+        Feature.enabled?(:ci_detect_wrongly_expired_artifacts, default_enabled: :yaml)
+      end
+
+      def wrongly_expired?(artifact)
+        return false unless artifact.expire_at.present?
+
+        match_date?(artifact.expire_at) && match_time?(artifact.expire_at)
+      end
+
+      def match_date?(expire_at)
+        [21, 22, 23].include?(expire_at.day)
+      end
+
+      def match_time?(expire_at)
+        %w[00:00.000 30:00.000 45:00.000].include?(expire_at.strftime('%M:%S.%L'))
+      end
+
+      def remove_expire_at(artifacts)
+        Ci::JobArtifact.id_in(artifacts).update_all(expire_at: nil)
+
+        Gitlab::AppLogger.info(message: "Fixed expire_at from artifacts.", fixed_artifacts_expire_at_count: artifacts.count)
+      end
     end
   end
 end
diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml
index 8d6c0e29b6a..f6ed6c26752 100644
--- a/app/views/projects/issues/_new_branch.html.haml
+++ b/app/views/projects/issues/_new_branch.html.haml
@@ -6,7 +6,7 @@
   - create_mr_text = can_create_confidential_merge_request? ? _('Create confidential merge request') : _('Create merge request')
 
   - can_create_path = can_create_branch_project_issue_path(@project, @issue)
-  - create_mr_path = project_new_merge_request_path(@project, merge_request: { source_branch: @issue.to_branch_name, target_branch: @project.default_branch })
+  - create_mr_path = project_new_merge_request_path(@project, merge_request: { source_branch: @issue.to_branch_name, target_branch: @project.default_branch, issue_iid: @issue.iid })
   - create_branch_path = project_branches_path(@project, branch_name: @issue.to_branch_name, ref: @project.default_branch, issue_iid: @issue.iid, format: :json)
   - refs_path = refs_namespace_project_path(@project.namespace, @project, search: '')
 
diff --git a/config/feature_flags/development/ci_destroy_all_expired_service.yml b/config/feature_flags/development/ci_destroy_all_expired_service.yml
index 34c94529f99..0f36a8d7e30 100644
--- a/config/feature_flags/development/ci_destroy_all_expired_service.yml
+++ b/config/feature_flags/development/ci_destroy_all_expired_service.yml
@@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348786
 milestone: '14.6'
 type: development
 group: group::pipeline execution
-default_enabled: false
+default_enabled: true
diff --git a/config/feature_flags/development/ci_detect_wrongly_expired_artifacts.yml b/config/feature_flags/development/ci_detect_wrongly_expired_artifacts.yml
new file mode 100644
index 00000000000..d48747c3bf5
--- /dev/null
+++ b/config/feature_flags/development/ci_detect_wrongly_expired_artifacts.yml
@@ -0,0 +1,8 @@
+---
+name: ci_detect_wrongly_expired_artifacts
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82084
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/354955
+milestone: '14.9'
+type: development
+group: group::pipeline insights
+default_enabled: true
diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
index 167521134b1..8c80e19810e 100644
--- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
+++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
@@ -73,7 +73,9 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do
 
             expect(page).to have_content('New merge request')
             expect(page).to have_content("From #{issue.to_branch_name} into #{project.default_branch}")
-            expect(page).to have_current_path(project_new_merge_request_path(project, merge_request: { source_branch: issue.to_branch_name, target_branch: project.default_branch }))
+            expect(page).to have_content("Closes ##{issue.iid}")
+            expect(page).to have_field("Title", with: "Draft: Resolve \"Cherry-Coloured Funk\"")
+            expect(page).to have_current_path(project_new_merge_request_path(project, merge_request: { source_branch: issue.to_branch_name, target_branch: project.default_branch, issue_iid: issue.iid }))
           end
         end
 
@@ -96,7 +98,9 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do
 
             expect(page).to have_content('New merge request')
             expect(page).to have_content("From #{branch_name} into #{project.default_branch}")
-            expect(page).to have_current_path(project_new_merge_request_path(project, merge_request: { source_branch: branch_name, target_branch: project.default_branch }))
+            expect(page).to have_content("Closes ##{issue.iid}")
+            expect(page).to have_field("Title", with: "Draft: Resolve \"Cherry-Coloured Funk\"")
+            expect(page).to have_current_path(project_new_merge_request_path(project, merge_request: { source_branch: branch_name, target_branch: project.default_branch, issue_iid: issue.iid }))
           end
         end
 
diff --git a/spec/frontend/issues/create_merge_request_dropdown_spec.js b/spec/frontend/issues/create_merge_request_dropdown_spec.js
index 637b4d31999..c2cfb16fdf7 100644
--- a/spec/frontend/issues/create_merge_request_dropdown_spec.js
+++ b/spec/frontend/issues/create_merge_request_dropdown_spec.js
@@ -59,7 +59,7 @@ describe('CreateMergeRequestDropdown', () => {
   describe('updateCreatePaths', () => {
     it('escapes branch names correctly', () => {
       dropdown.createBranchPath = `${TEST_HOST}/branches?branch_name=some-branch&issue=42`;
-      dropdown.createMrPath = `${TEST_HOST}/create_merge_request?merge_request%5Bsource_branch%5D=test&merge_request%5Btarget_branch%5D=master`;
+      dropdown.createMrPath = `${TEST_HOST}/create_merge_request?merge_request%5Bsource_branch%5D=test&merge_request%5Btarget_branch%5D=master&merge_request%5Bissue_iid%5D=42`;
 
       dropdown.updateCreatePaths('branch', 'contains#hash');
 
@@ -68,7 +68,7 @@ describe('CreateMergeRequestDropdown', () => {
       );
 
       expect(dropdown.createMrPath).toBe(
-        `${TEST_HOST}/create_merge_request?merge_request%5Bsource_branch%5D=contains%23hash&merge_request%5Btarget_branch%5D=master`,
+        `${TEST_HOST}/create_merge_request?merge_request%5Bsource_branch%5D=contains%23hash&merge_request%5Btarget_branch%5D=master&merge_request%5Bissue_iid%5D=42`,
       );
     });
   });
diff --git a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb
index 0e7230c042e..67d664a617b 100644
--- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb
+++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb
@@ -102,5 +102,81 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do
         is_expected.to eq(destroyed_artifacts_count: 0, statistics_updates: {}, status: :success)
       end
     end
+
+    context 'with artifacts that has backfilled expire_at' do
+      let!(:created_on_00_30_45_minutes_on_21_22_23) do
+        [
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 00:00:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 01:30:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-22 12:00:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-22 12:30:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-23 23:00:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-23 23:30:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-23 06:45:00.000'))
+        ]
+      end
+
+      let!(:created_close_to_00_or_30_minutes) do
+        [
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 00:00:00.001')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-21 00:30:00.999'))
+        ]
+      end
+
+      let!(:created_on_00_or_30_minutes_on_other_dates) do
+        [
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-01 00:00:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-19 12:00:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-24 23:30:00.000'))
+        ]
+      end
+
+      let!(:created_at_other_times) do
+        [
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-19 00:00:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-19 00:30:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-24 00:00:00.000')),
+          create(:ci_job_artifact, expire_at: Time.zone.parse('2022-01-24 00:30:00.000'))
+        ]
+      end
+
+      let(:artifacts_to_keep) { created_on_00_30_45_minutes_on_21_22_23 }
+      let(:artifacts_to_delete) { created_close_to_00_or_30_minutes + created_on_00_or_30_minutes_on_other_dates + created_at_other_times }
+      let(:all_artifacts) { artifacts_to_keep + artifacts_to_delete }
+
+      let(:artifacts) { Ci::JobArtifact.where(id: all_artifacts.map(&:id)) }
+
+      it 'deletes job artifacts that do not have expire_at on 00, 30 or 45 minute of 21, 22, 23 of the month' do
+        expect { subject }.to change { Ci::JobArtifact.count }.by(artifacts_to_delete.size * -1)
+      end
+
+      it 'keeps job artifacts that have expire_at on 00, 30 or 45 minute of 21, 22, 23 of the month' do
+        expect { subject }.not_to change { Ci::JobArtifact.where(id: artifacts_to_keep.map(&:id)).count }
+      end
+
+      it 'removes expire_at on job artifacts that have expire_at on 00, 30 or 45 minute of 21, 22, 23 of the month' do
+        subject
+
+        expect(artifacts_to_keep.all? { |artifact| artifact.reload.expire_at.nil? }).to be(true)
+      end
+
+      context 'when feature flag is disabled' do
+        before do
+          stub_feature_flags(ci_detect_wrongly_expired_artifacts: false)
+        end
+
+        it 'deletes all job artifacts' do
+          expect { subject }.to change { Ci::JobArtifact.count }.by(all_artifacts.size * -1)
+        end
+      end
+
+      context 'when fix_expire_at is false' do
+        let(:service) { described_class.new(artifacts, pick_up_at: Time.current, fix_expire_at: false) }
+
+        it 'deletes all job artifacts' do
+          expect { subject }.to change { Ci::JobArtifact.count }.by(all_artifacts.size * -1)
+        end
+      end
+    end
   end
 end