Change cleanup policies to ignore protected tags

app/models/container_registry/protection/tag_rule.rb

@@ -7,6 +7,7 @@ class TagRule < ApplicationRecord
 
       ACCESS_LEVELS = Gitlab::Access.sym_options_with_admin.slice(:maintainer, :owner, :admin).freeze
       MAX_TAG_RULES_PER_PROJECT = 5
+      DELETE_ACTIONS = ['delete'].freeze
 
       enum :minimum_access_level_for_delete, ACCESS_LEVELS, prefix: true
       enum :minimum_access_level_for_push, ACCESS_LEVELS, prefix: true
@@ -26,6 +27,14 @@ class TagRule < ApplicationRecord
         where(conditions.reduce(:or))
       }
 
+      scope :for_delete_and_access, ->(access_level) do
+        for_actions_and_access(DELETE_ACTIONS, access_level)
+      end
+
+      scope :tag_name_patterns_for_project, ->(project_id) do
+        select(:tag_name_pattern).where(project_id: project_id)
+      end
+
       def push_restricted?(access_level)
         Gitlab::Access.sym_options_with_admin[minimum_access_level_for_push.to_sym] > access_level
       end

app/services/projects/container_repository/cleanup_tags_base_service.rb

@@ -21,6 +21,29 @@ def filter_by_name!(tags)
         end
       end
 
+      def filter_out_protected!(tags)
+        return if Feature.disabled?(:container_registry_protected_tags, project)
+
+        tag_rules = ::ContainerRegistry::Protection::TagRule.tag_name_patterns_for_project(project.id)
+
+        if current_user
+          return if current_user.can_admin_all_resources?
+
+          user_access_level = current_user.max_member_access_for_project(project.id)
+          tag_rules = tag_rules.for_delete_and_access(user_access_level)
+        end
+
+        return if tag_rules.blank?
+
+        patterns = tag_rules.map { |rule| ::Gitlab::UntrustedRegexp.new(rule.tag_name_pattern) }
+
+        tags.reject! do |tag|
+          patterns.detect do |pattern|
+            pattern.match?(tag.name)
+          end
+        end
+      end
+
       # Should return [tags_to_delete, tags_to_keep]
       def partition_by_keep_n(tags)
         return [tags, []] unless keep_n

app/services/projects/container_repository/gitlab/cleanup_tags_service.rb

@@ -34,6 +34,9 @@ def execute_for_tags(tags, overall_result)
           tags = filter_by_keep_n(tags)
           tags = filter_by_older_than(tags)
 
+          # As a most expensive operation it should follow after the other filters.
+          filter_out_protected!(tags) unless skip_protected_tags?
+
           overall_result[:before_delete_size] += tags.size
           overall_result[:original_size] += original_size
 
@@ -76,6 +79,10 @@ def pushed_at(tag)
         def timeout_disabled?
           params['disable_timeout'] || false
         end
+
+        def skip_protected_tags?
+          !!params['skip_protected_tags']
+        end
       end
     end
   end

app/workers/container_registry/delete_container_repository_worker.rb

@@ -19,7 +19,8 @@ class DeleteContainerRepositoryWorker
     CLEANUP_TAGS_SERVICE_PARAMS = {
       'name_regex_delete' => '.*',
       'keep_latest' => false,
-      'container_expiration_policy' => true # to avoid permissions checks
+      'container_expiration_policy' => true, # to avoid permissions checks
+      'skip_protected_tags' => true
     }.freeze
 
     def perform_work

doc/api/container_registry.md

@@ -352,6 +352,7 @@ if successful, and performs the following operations:
 - It keeps N latest matching tags (if `keep_n` is specified).
 - It only removes tags that are older than X amount of time (if `older_than` is
   specified).
+- It excludes [protected tags](../user/packages/container_registry/protected_container_tags.md).
 - It schedules the asynchronous job to be executed in the background.
 
 These operations are executed asynchronously and can take time to get executed.

spec/models/container_registry/protection/tag_rule_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe ContainerRegistry::Protection::TagRule, type: :model, feature_category: :container_registry do
+  using RSpec::Parameterized::TableSyntax
+
   it_behaves_like 'having unique enum values'
 
   describe 'relationships' do
@@ -141,8 +143,6 @@
       rule_four
     end
 
-    using RSpec::Parameterized::TableSyntax
-
     where(:user_access_level, :actions, :expected_rules) do
       Gitlab::Access::DEVELOPER  | ['push']         | lazy { [rule_one, rule_two, rule_three, rule_four] }
       Gitlab::Access::DEVELOPER  | ['delete']       | lazy { [rule_one, rule_two, rule_three, rule_four] }
@@ -169,6 +169,66 @@
     end
   end
 
+  describe '.for_delete_and_access' do
+    let_it_be(:rule_one) do
+      create(:container_registry_protection_tag_rule,
+        tag_name_pattern: 'one',
+        minimum_access_level_for_push: :maintainer,
+        minimum_access_level_for_delete: :maintainer)
+    end
+
+    let_it_be(:rule_two) do
+      create(:container_registry_protection_tag_rule,
+        tag_name_pattern: 'two',
+        minimum_access_level_for_push: :owner,
+        minimum_access_level_for_delete: :maintainer)
+    end
+
+    let_it_be(:rule_three) do
+      create(:container_registry_protection_tag_rule,
+        tag_name_pattern: 'three',
+        minimum_access_level_for_push: :maintainer,
+        minimum_access_level_for_delete: :admin)
+    end
+
+    let_it_be(:rule_four) do
+      create(:container_registry_protection_tag_rule,
+        tag_name_pattern: 'four',
+        minimum_access_level_for_push: :admin,
+        minimum_access_level_for_delete: :admin)
+    end
+
+    where(:user_access_level, :expected_rules) do
+      Gitlab::Access::DEVELOPER  | [ref(:rule_one), ref(:rule_two), ref(:rule_three), ref(:rule_four)]
+      Gitlab::Access::MAINTAINER | [ref(:rule_three), ref(:rule_four)]
+      Gitlab::Access::OWNER      | [ref(:rule_three), ref(:rule_four)]
+      Gitlab::Access::ADMIN      | []
+    end
+
+    with_them do
+      subject { described_class.for_delete_and_access(user_access_level) }
+
+      it 'returns the expected rules' do
+        is_expected.to match_array(expected_rules)
+      end
+    end
+  end
+
+  describe '.tag_name_patterns_for_projects' do
+    let_it_be(:rule) { create(:container_registry_protection_tag_rule) }
+    let_it_be(:rule2) { create(:container_registry_protection_tag_rule) }
+
+    subject(:result) { described_class.tag_name_patterns_for_project(rule.project_id) }
+
+    it 'contains matched rule' do
+      expect(result.pluck(:tag_name_pattern)).to contain_exactly(rule.tag_name_pattern)
+    end
+
+    it 'selects only the tag_name_pattern' do
+      expect(result.select_values).to contain_exactly(:tag_name_pattern)
+    end
+  end
+
   describe '#push_restricted?' do
     let(:rule) do
       create(

spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb

@@ -7,13 +7,12 @@
 
   include_context 'for a cleanup tags service'
 
-  let_it_be(:user) { create(:user) }
   let_it_be(:user) { create(:user) }
   let_it_be(:project, reload: true) { create(:project, :private) }
 
   let(:repository) { create(:container_repository, :root, project: project) }
   let(:service) { described_class.new(container_repository: repository, current_user: user, params: params) }
-  let(:tags) { %w[latest A Ba Bb C D E] }
+  let(:tags) { %w[latest A Ba Bb C D 17-8-stable] }
 
   before do
     project.add_maintainer(user) if user
@@ -37,7 +36,7 @@
         'Bb' => six_days_ago,
         'C' => one_month_ago,
         'D' => nil,
-        'E' => nil
+        '17-8-stable' => nil
       }
     )
   end
@@ -49,17 +48,17 @@
       let(:tags_page_size) { 2 }
 
       it_behaves_like 'when regex matching everything is specified',
-        delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[E]]
+        delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[17-8-stable]]
 
       it_behaves_like 'when regex matching everything is specified and latest is not kept',
-        delete_expectations: [%w[latest A], %w[Ba Bb], %w[C D], %w[E]]
+        delete_expectations: [%w[latest A], %w[Ba Bb], %w[C D], %w[17-8-stable]]
 
       it_behaves_like 'when delete regex matching specific tags is used'
 
       it_behaves_like 'when delete regex matching specific tags is used with overriding allow regex'
 
       it_behaves_like 'with allow regex value',
-        delete_expectations: [%w[A], %w[C D], %w[E]]
+        delete_expectations: [%w[A], %w[C D], %w[17-8-stable]]
 
       it_behaves_like 'when keeping only N tags',
         delete_expectations: [%w[Bb]]
@@ -87,6 +86,40 @@
       it_behaves_like 'when running a container_expiration_policy',
         delete_expectations: [%w[Bb], %w[C]]
 
+      it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+        delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[17-8-stable]]
+
+      context 'with admin minimum_access_level_for_delete' do
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A], %w[Ba Bb], %w[C D]],
+          minimum_access_level_for_delete: :admin
+      end
+
+      context 'without user' do
+        let(:user) { nil }
+
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A], %w[Ba Bb], %w[C D]]
+      end
+
+      context 'with the skip_protected_tags param' do
+        let(:params) do
+          { 'skip_protected_tags' => true }
+        end
+
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[17-8-stable]]
+      end
+
+      context 'with the container_registry_protected_tags disabled' do
+        before do
+          stub_feature_flags(container_registry_protected_tags: false)
+        end
+
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[17-8-stable]]
+      end
+
       context 'with a timeout' do
         let(:params) do
           { 'name_regex_delete' => '.*' }
@@ -113,7 +146,7 @@
           end
 
           it_behaves_like 'when regex matching everything is specified',
-            delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[E]]
+            delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[17-8-stable]]
         end
       end
     end
@@ -122,14 +155,14 @@
       let(:tags_page_size) { 1000 }
 
       it_behaves_like 'when regex matching everything is specified',
-        delete_expectations: [%w[A Ba Bb C D E]]
+        delete_expectations: [%w[A Ba Bb C D 17-8-stable]]
 
       it_behaves_like 'when delete regex matching specific tags is used'
 
       it_behaves_like 'when delete regex matching specific tags is used with overriding allow regex'
 
       it_behaves_like 'with allow regex value',
-        delete_expectations: [%w[A C D E]]
+        delete_expectations: [%w[A C D 17-8-stable]]
 
       it_behaves_like 'when keeping only N tags',
         delete_expectations: [%w[Ba Bb C]]
@@ -148,6 +181,40 @@
 
       it_behaves_like 'when running a container_expiration_policy',
         delete_expectations: [%w[Ba Bb C]]
+
+      it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+        delete_expectations: [%w[A Ba Bb C D 17-8-stable]]
+
+      context 'with admin minimum_access_level_for_delete' do
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A Ba Bb C D]],
+          minimum_access_level_for_delete: :admin
+      end
+
+      context 'without user' do
+        let(:user) { nil }
+
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A Ba Bb C D]]
+      end
+
+      context 'with the skip_protected_tags param' do
+        let(:params) do
+          { 'skip_protected_tags' => true }
+        end
+
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A Ba Bb C D 17-8-stable]]
+      end
+
+      context 'with the container_registry_protected_tags disabled' do
+        before do
+          stub_feature_flags(container_registry_protected_tags: false)
+        end
+
+        it_behaves_like 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$',
+          delete_expectations: [%w[A Ba Bb C D 17-8-stable]]
+      end
     end
 
     context 'with no tags page' do

spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb

@@ -194,6 +194,28 @@
   end
 end
 
+RSpec.shared_examples 'with protected rule having pattern ^\d{1,2}-\d{1,2}-stable$' do
+  |delete_expectations:, service_response_extra: {}, supports_caching: false,
+   minimum_access_level_for_delete: :maintainer|
+  let_it_be(:rule) do
+    create(
+      :container_registry_protection_tag_rule,
+      tag_name_pattern: '^\d{1,2}-\d{1,2}-stable$',
+      project: project,
+      minimum_access_level_for_delete: minimum_access_level_for_delete
+    )
+  end
+
+  let(:params) do
+    { 'name_regex_delete' => '.*' }
+  end
+
+  it_behaves_like 'removing the expected tags',
+    service_response_extra: service_response_extra,
+    supports_caching: supports_caching,
+    delete_expectations: delete_expectations
+end
+
 RSpec.shared_examples 'not removing anything' do |service_response_extra: {}, supports_caching: false|
   it 'does not remove anything' do
     expect(Projects::ContainerRepository::DeleteTagsService).not_to receive(:new)