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

16 files changed, 195 insertions, 100 deletions

diff --git a/app/models/concerns/runners_token_prefixable.rb b/app/models/concerns/runners_token_prefixable.rb
new file mode 100644
index 00000000000..1aea874337e
--- /dev/null
+++ b/app/models/concerns/runners_token_prefixable.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module RunnersTokenPrefixable
+  extend ActiveSupport::Concern
+
+  # Prefix for runners_token which can be used to invalidate existing tokens.
+  # The value chosen here is GR (for Gitlab Runner) combined with the rotation
+  # date (20220225) decimal to hex encoded.
+  RUNNERS_TOKEN_PREFIX = 'GR1348941'
+
+  def runners_token_prefix
+    RUNNERS_TOKEN_PREFIX
+  end
+end
diff --git a/app/models/group.rb b/app/models/group.rb
index 4d637d0c445..2315fe2d44f 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -17,14 +17,10 @@ class Group < Namespace
   include GroupAPICompatibility
   include EachBatch
   include BulkMemberAccessLoad
+  include RunnersTokenPrefixable
 
   extend ::Gitlab::Utils::Override
 
-  # Prefix for runners_token which can be used to invalidate existing tokens.
-  # The value chosen here is GR (for Gitlab Runner) combined with the rotation
-  # date (20220225) decimal to hex encoded.
-  RUNNERS_TOKEN_PREFIX = 'GR1348941'
-
   def self.sti_name
     'Group'
   end
@@ -117,7 +113,7 @@ class Group < Namespace
 
   add_authentication_token_field :runners_token,
                                  encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required },
-                                 prefix: ->(instance) { instance.runners_token_prefix }
+                                 prefix: RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX
 
   after_create :post_create_hook
   after_destroy :post_destroy_hook
@@ -661,10 +657,6 @@ class Group < Namespace
     ensure_runners_token!
   end
 
-  def runners_token_prefix
-    Feature.enabled?(:groups_runners_token_prefix, self, default_enabled: :yaml) ? RUNNERS_TOKEN_PREFIX : ''
-  end
-
   override :format_runners_token
   def format_runners_token(token)
     "#{runners_token_prefix}#{token}"
diff --git a/app/models/project.rb b/app/models/project.rb
index 1a4a0b6f105..e651e6bfd92 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -37,6 +37,7 @@ class Project < ApplicationRecord
   include EachBatch
   include GitlabRoutingHelper
   include BulkMemberAccessLoad
+  include RunnersTokenPrefixable
 
   extend Gitlab::Cache::RequestCache
   extend Gitlab::Utils::Override
@@ -73,11 +74,6 @@ class Project < ApplicationRecord
 
   GL_REPOSITORY_TYPES = [Gitlab::GlRepository::PROJECT, Gitlab::GlRepository::WIKI, Gitlab::GlRepository::DESIGN].freeze
 
-  # Prefix for runners_token which can be used to invalidate existing tokens.
-  # The value chosen here is GR (for Gitlab Runner) combined with the rotation
-  # date (20220225) decimal to hex encoded.
-  RUNNERS_TOKEN_PREFIX = 'GR1348941'
-
   cache_markdown_field :description, pipeline: :description
 
   default_value_for :packages_enabled, true
@@ -100,7 +96,7 @@ class Project < ApplicationRecord
 
   add_authentication_token_field :runners_token,
                                  encrypted: -> { Feature.enabled?(:projects_tokens_optional_encryption, default_enabled: true) ? :optional : :required },
-                                 prefix: ->(instance) { instance.runners_token_prefix }
+                                 prefix: RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX
 
   before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? }
 
@@ -1850,10 +1846,6 @@ class Project < ApplicationRecord
     ensure_runners_token!
   end
 
-  def runners_token_prefix
-    Feature.enabled?(:projects_runners_token_prefix, self, default_enabled: :yaml) ? RUNNERS_TOKEN_PREFIX : ''
-  end
-
   override :format_runners_token
   def format_runners_token(token)
     "#{runners_token_prefix}#{token}"
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/config/feature_flags/development/ci_bulk_insert_tags.yml b/config/feature_flags/development/ci_bulk_insert_tags.yml
index 6b8ad4ef39d..52a3e22379c 100644
--- a/config/feature_flags/development/ci_bulk_insert_tags.yml
+++ b/config/feature_flags/development/ci_bulk_insert_tags.yml
@@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346124
 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/config/feature_flags/development/groups_runners_token_prefix.yml b/config/feature_flags/development/groups_runners_token_prefix.yml
deleted file mode 100644
index 87b87266673..00000000000
--- a/config/feature_flags/development/groups_runners_token_prefix.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: groups_runners_token_prefix
-introduced_by_url: 
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353805
-milestone: '14.9'
-type: development
-group: group::database
-default_enabled: true
diff --git a/config/feature_flags/development/projects_runners_token_prefix.yml b/config/feature_flags/development/projects_runners_token_prefix.yml
deleted file mode 100644
index 5dd21d115f6..00000000000
--- a/config/feature_flags/development/projects_runners_token_prefix.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: projects_runners_token_prefix
-introduced_by_url: 
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353805
-milestone: '14.9'
-type: development
-group: group::database
-default_enabled: true
diff --git a/spec/models/concerns/runners_token_prefixable_spec.rb b/spec/models/concerns/runners_token_prefixable_spec.rb
new file mode 100644
index 00000000000..6127203987f
--- /dev/null
+++ b/spec/models/concerns/runners_token_prefixable_spec.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe RunnersTokenPrefixable do
+  before do
+    stub_const('DummyModel', Class.new)
+    DummyModel.class_eval do
+      include RunnersTokenPrefixable
+    end
+  end
+
+  describe '.runners_token_prefix' do
+    subject { DummyModel.new }
+
+    it 'returns RUNNERS_TOKEN_PREFIX' do
+      expect(subject.runners_token_prefix).to eq(RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX)
+    end
+  end
+end
diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb
index e59eaa6dd80..7a789aedd97 100644
--- a/spec/models/concerns/token_authenticatable_spec.rb
+++ b/spec/models/concerns/token_authenticatable_spec.rb
@@ -297,7 +297,7 @@ RSpec.shared_examples 'prefixed token rotation' do
 
     context 'token is not set' do
       it 'generates a new token' do
-        expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/)
+        expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/)
         expect(instance).not_to be_persisted
       end
     end
@@ -308,26 +308,14 @@ RSpec.shared_examples 'prefixed token rotation' do
       end
 
       it 'generates a new token' do
-        expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/)
+        expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/)
         expect(instance).not_to be_persisted
       end
-
-      context 'feature flag is disabled' do
-        before do
-          flag = "#{described_class.name.downcase.pluralize}_runners_token_prefix"
-          stub_feature_flags(flag => false)
-        end
-
-        it 'leaves the token unchanged' do
-          expect { subject }.not_to change(instance, :runners_token)
-          expect(instance).not_to be_persisted
-        end
-      end
     end
 
     context 'token is set and matches prefix' do
       before do
-        instance.set_runners_token(instance.class::RUNNERS_TOKEN_PREFIX + '-abcdef')
+        instance.set_runners_token(RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX + '-abcdef')
       end
 
       it 'leaves the token unchanged' do
@@ -342,7 +330,7 @@ RSpec.shared_examples 'prefixed token rotation' do
 
     context 'token is not set' do
       it 'generates a new token' do
-        expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/)
+        expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/)
         expect(instance).to be_persisted
       end
     end
@@ -353,25 +341,14 @@ RSpec.shared_examples 'prefixed token rotation' do
       end
 
       it 'generates a new token' do
-        expect(subject).to match(/^#{instance.class::RUNNERS_TOKEN_PREFIX}/)
+        expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/)
         expect(instance).to be_persisted
       end
-
-      context 'feature flag is disabled' do
-        before do
-          flag = "#{described_class.name.downcase.pluralize}_runners_token_prefix"
-          stub_feature_flags(flag => false)
-        end
-
-        it 'leaves the token unchanged' do
-          expect { subject }.not_to change(instance, :runners_token)
-        end
-      end
     end
 
     context 'token is set and matches prefix' do
       before do
-        instance.set_runners_token(instance.class::RUNNERS_TOKEN_PREFIX + '-abcdef')
+        instance.set_runners_token(RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX + '-abcdef')
         instance.save!
       end
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 29a3bca4421..68edabc2b1e 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -2781,6 +2781,6 @@ RSpec.describe Group do
 
     subject { group }
 
-    it_behaves_like 'it has a prefixable runners_token', :groups_runners_token_prefix
+    it_behaves_like 'it has a prefixable runners_token'
   end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 0b4e50a376d..7a529d739c1 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -7556,7 +7556,7 @@ RSpec.describe Project, factory_default: :keep do
 
     subject { project }
 
-    it_behaves_like 'it has a prefixable runners_token', :projects_runners_token_prefix
+    it_behaves_like 'it has a prefixable runners_token'
   end
 
   private
diff --git a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb
index e71f1a4266a..ddd87d61621 100644
--- a/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb
+++ b/spec/services/ci/job_artifacts/destroy_all_expired_service_spec.rb
@@ -40,7 +40,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s
             # COMMIT
             # SELECT next expired ci_job_artifacts
 
-            expect(log.count).to be_within(1).of(10)
+            expect(log.count).to be_within(1).of(11)
           end
         end
 
@@ -51,7 +51,7 @@ RSpec.describe Ci::JobArtifacts::DestroyAllExpiredService, :clean_gitlab_redis_s
 
           it 'performs the smallest number of queries for job_artifacts' do
             log = ActiveRecord::QueryRecorder.new { subject }
-            expect(log.count).to be_within(1).of(8)
+            expect(log.count).to be_within(1).of(9)
           end
 
           context 'with several locked-unknown artifact records' do
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
diff --git a/spec/support/shared_examples/models/runners_token_prefix_shared_examples.rb b/spec/support/shared_examples/models/runners_token_prefix_shared_examples.rb
index bbce67ae7b9..4dce445ac73 100644
--- a/spec/support/shared_examples/models/runners_token_prefix_shared_examples.rb
+++ b/spec/support/shared_examples/models/runners_token_prefix_shared_examples.rb
@@ -1,35 +1,13 @@
 # frozen_string_literal: true
 
-RSpec.shared_examples 'it has a prefixable runners_token' do |feature_flag|
-  context 'feature flag enabled' do
-    before do
-      stub_feature_flags(feature_flag => [subject])
+RSpec.shared_examples 'it has a prefixable runners_token' do
+  describe '#runners_token' do
+    it 'has a runners_token_prefix' do
+      expect(subject.runners_token_prefix).not_to be_empty
     end
 
-    describe '#runners_token' do
-      it 'has a runners_token_prefix' do
-        expect(subject.runners_token_prefix).not_to be_empty
-      end
-
-      it 'starts with the runners_token_prefix' do
-        expect(subject.runners_token).to start_with(subject.runners_token_prefix)
-      end
-    end
-  end
-
-  context 'feature flag disabled' do
-    before do
-      stub_feature_flags(feature_flag => false)
-    end
-
-    describe '#runners_token' do
-      it 'does not have a runners_token_prefix' do
-        expect(subject.runners_token_prefix).to be_empty
-      end
-
-      it 'starts with the runners_token_prefix' do
-        expect(subject.runners_token).to start_with(subject.runners_token_prefix)
-      end
+    it 'starts with the runners_token_prefix' do
+      expect(subject.runners_token).to start_with(subject.runners_token_prefix)
     end
   end
 end