Merge branch '27072-name-regex-allow-bulk-api' into 'master'

Add name_regex_keep param to tag bulk delete api

See merge request gitlab-org/gitlab!25484

app/services/projects/container_repository/cleanup_tags_service.rb

@@ -61,10 +61,15 @@ module Projects
       end
 
       def filter_by_name(tags)
-        regex = Gitlab::UntrustedRegexp.new("\\A#{params['name_regex']}\\z")
+        # Technical Debt: https://gitlab.com/gitlab-org/gitlab/issues/207267
+        # name_regex to be removed when container_expiration_policies is updated
+        # to have both regex columns
+        regex_delete = Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_delete'] || params['name_regex']}\\z")
+        regex_retain = Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_keep']}\\z")
 
         tags.select do |tag|
-          regex.scan(tag.name).any?
+          # regex_retain will override any overlapping matches by regex_delete
+          regex_delete.match?(tag.name) && !regex_retain.match?(tag.name)
         end
       end
 

changelogs/unreleased/27072-name-regex-allow-bulk-api.yml

+---
+title: Add name_regex_keep param to container registry bulk delete API endpoint
+merge_request: 25484
+author:
+type: added

doc/api/container_registry.md

@@ -231,7 +231,9 @@ DELETE /projects/:id/registry/repositories/:repository_id/tags
 | --------- | ---- | -------- | ----------- |
 | `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user. |
 | `repository_id` | integer | yes | The ID of registry repository. |
-| `name_regex` | string | yes | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to delete. To delete all tags specify `.*`.|
+| `name_regex` | string | no | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to delete. To delete all tags specify `.*`. **Note:** `name_regex` is deprecated in favor of `name_regex_delete`.|
+| `name_regex_delete` | string | yes | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to delete. To delete all tags specify `.*`.|
+| `name_regex_keep` | string | no | The [re2](https://github.com/google/re2/wiki/Syntax) regex of the name to keep. This value will override any matches from `name_regex_delete`. Note: setting to `.*` will result in a no-op. |
 | `keep_n` | integer | no | The amount of latest tags of given name to keep. |
 | `older_than` | string | no | Tags to delete that are older than the given time, written in human readable form `1h`, `1d`, `1month`. |
 
@@ -239,7 +241,7 @@ This API call performs the following operations:
 
 1. It orders all tags by creation date. The creation date is the time of the
    manifest creation, not the time of tag push.
-1. It removes only the tags matching the given `name_regex`.
+1. It removes only the tags matching the given `name_regex_delete` (or deprecated `name_regex`), keeping any that match `name_regex_keep`.
 1. It never removes the tag named `latest`.
 1. It keeps N latest matching tags (if `keep_n` is specified).
 1. It only removes tags that are older than X amount of time (if `older_than` is specified).
@@ -261,17 +263,23 @@ Examples:
    and remove ones that are older than 2 days:
 
    ```shell
-   curl --request DELETE --data 'name_regex=[0-9a-z]{40}' --data 'keep_n=5' --data 'older_than=2d' --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories/2/tags"
+   curl --request DELETE --data 'name_regex_delete=[0-9a-z]{40}' --data 'keep_n=5' --data 'older_than=2d' --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories/2/tags"
    ```
 
 1. Remove all tags, but keep always the latest 5:
 
    ```shell
-   curl --request DELETE --data 'name_regex=.*' --data 'keep_n=5' --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories/2/tags"
+   curl --request DELETE --data 'name_regex_delete=.*' --data 'keep_n=5' --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories/2/tags"
+   ```
+
+1. Remove all tags, but keep always tags beginning with `stable`:
+
+   ```shell
+   curl --request DELETE --data 'name_regex_delete=.*' --data 'name_regex_keep=stable.*' --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories/2/tags"
    ```
 
 1. Remove all tags that are older than 1 month:
 
    ```shell
-   curl --request DELETE --data 'name_regex=.*' --data 'older_than=1month' --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories/2/tags"
+   curl --request DELETE --data 'name_regex_delete=.*' --data 'older_than=1month' --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories/2/tags"
    ```

lib/api/project_container_repositories.rb

@@ -69,7 +69,16 @@ module API
       end
       params do
         requires :repository_id, type: Integer, desc: 'The ID of the repository'
-        requires :name_regex, type: String, desc: 'The tag name regexp to delete, specify .* to delete all'
+        optional :name_regex_delete, type: String, desc: 'The tag name regexp to delete, specify .* to delete all'
+        # require either name_regex (deprecated) or name_regex_delete, it is ok to have both
+        given name_regex_delete: ->(val) { val.nil? } do
+          requires :name_regex, type: String, desc: 'The tag name regexp to delete, specify .* to delete all'
+        end
+        optional :name_regex, type: String, desc: 'The tag name regexp to delete, specify .* to delete all'
+        given name_regex: ->(val) { val.nil? } do
+          requires :name_regex_delete, type: String, desc: 'The tag name regexp to delete, specify .* to delete all'
+        end
+        optional :name_regex_keep, type: String, desc: 'The tag name regexp to retain'
         optional :keep_n, type: Integer, desc: 'Keep n of latest tags with matching name'
         optional :older_than, type: String, desc: 'Delete older than: 1h, 1d, 1month'
       end

spec/requests/api/project_container_repositories_spec.rb

@@ -109,7 +109,7 @@ describe API::ProjectContainerRepositories do
 
     context 'disallowed' do
       let(:params) do
-        { name_regex: 'v10.*' }
+        { name_regex_delete: 'v10.*' }
       end
 
       it_behaves_like 'rejected container repository access', :developer, :forbidden
@@ -130,16 +130,33 @@ describe API::ProjectContainerRepositories do
         end
       end
 
+      context 'without name_regex' do
+        let(:params) do
+          { keep_n: 100,
+            older_than: '1 day',
+            other: 'some value' }
+        end
+
+        it 'returns bad request' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+      end
+
       context 'passes all declared parameters' do
         let(:params) do
-          { name_regex: 'v10.*',
+          { name_regex_delete: 'v10.*',
+            name_regex_keep: 'v10.1.*',
             keep_n: 100,
             older_than: '1 day',
             other: 'some value' }
         end
 
         let(:worker_params) do
-          { name_regex: 'v10.*',
+          { name_regex: nil,
+            name_regex_delete: 'v10.*',
+            name_regex_keep: 'v10.1.*',
             keep_n: 100,
             older_than: '1 day',
             container_expiration_policy: false }
@@ -174,6 +191,38 @@ describe API::ProjectContainerRepositories do
           end
         end
       end
+
+      context 'with deprecated name_regex param' do
+        let(:params) do
+          { name_regex: 'v10.*',
+            name_regex_keep: 'v10.1.*',
+            keep_n: 100,
+            older_than: '1 day',
+            other: 'some value' }
+        end
+
+        let(:worker_params) do
+          { name_regex: 'v10.*',
+            name_regex_delete: nil,
+            name_regex_keep: 'v10.1.*',
+            keep_n: 100,
+            older_than: '1 day',
+            container_expiration_policy: false }
+        end
+
+        let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" }
+
+        it 'schedules cleanup of tags repository' do
+          stub_last_activity_update
+          stub_exclusive_lease(lease_key, timeout: 1.hour)
+          expect(CleanupContainerRepositoryWorker).to receive(:perform_async)
+            .with(maintainer.id, root_repository.id, worker_params)
+
+          subject
+
+          expect(response).to have_gitlab_http_status(:accepted)
+        end
+      end
     end
   end
 

spec/services/projects/container_repository/cleanup_tags_service_spec.rb

@@ -48,25 +48,37 @@ describe Projects::ContainerRepository::CleanupTagsService do
     end
 
     context 'when regex matching everything is specified' do
+      shared_examples 'removes all matches' do
+        it 'does remove B* and C' do
+          # The :A cannot be removed as config is shared with :latest
+          # The :E cannot be removed as it does not have valid manifest
+
+          expect_delete('sha256:configB').twice
+          expect_delete('sha256:configC')
+          expect_delete('sha256:configD')
+
+          is_expected.to include(status: :success, deleted: %w(D Bb Ba C))
+        end
+      end
+
       let(:params) do
-        { 'name_regex' => '.*' }
+        { 'name_regex_delete' => '.*' }
       end
 
-      it 'does remove B* and C' do
-        # The :A cannot be removed as config is shared with :latest
-        # The :E cannot be removed as it does not have valid manifest
+      it_behaves_like 'removes all matches'
 
-        expect_delete('sha256:configB').twice
-        expect_delete('sha256:configC')
-        expect_delete('sha256:configD')
+      context 'with deprecated name_regex param' do
+        let(:params) do
+          { 'name_regex' => '.*' }
+        end
 
-        is_expected.to include(status: :success, deleted: %w(D Bb Ba C))
+        it_behaves_like 'removes all matches'
       end
     end
 
-    context 'when regex matching specific tags is used' do
+    context 'when delete regex matching specific tags is used' do
       let(:params) do
-        { 'name_regex' => 'C|D' }
+        { 'name_regex_delete' => 'C|D' }
       end
 
       it 'does remove C and D' do
@@ -75,11 +87,37 @@ describe Projects::ContainerRepository::CleanupTagsService do
 
         is_expected.to include(status: :success, deleted: %w(D C))
       end
+
+      context 'with overriding allow regex' do
+        let(:params) do
+          { 'name_regex_delete' => 'C|D',
+            'name_regex_keep' => 'C' }
+        end
+
+        it 'does not remove C' do
+          expect_delete('sha256:configD')
+
+          is_expected.to include(status: :success, deleted: %w(D))
+        end
+      end
+
+      context 'with name_regex_delete overriding deprecated name_regex' do
+        let(:params) do
+          { 'name_regex' => 'C|D',
+            'name_regex_delete' => 'D' }
+        end
+
+        it 'does not remove C' do
+          expect_delete('sha256:configD')
+
+          is_expected.to include(status: :success, deleted: %w(D))
+        end
+      end
     end
 
     context 'when removing a tagged image that is used by another tag' do
       let(:params) do
-        { 'name_regex' => 'Ba' }
+        { 'name_regex_delete' => 'Ba' }
       end
 
       it 'does not remove the tag' do
@@ -89,9 +127,23 @@ describe Projects::ContainerRepository::CleanupTagsService do
       end
     end
 
+    context 'with allow regex value' do
+      let(:params) do
+        { 'name_regex_delete' => '.*',
+          'name_regex_keep' => 'B.*' }
+      end
+
+      it 'does not remove B*' do
+        expect_delete('sha256:configC')
+        expect_delete('sha256:configD')
+
+        is_expected.to include(status: :success, deleted: %w(D C))
+      end
+    end
+
     context 'when removing keeping only 3' do
       let(:params) do
-        { 'name_regex' => '.*',
+        { 'name_regex_delete' => '.*',
           'keep_n' => 3 }
       end
 
@@ -104,7 +156,7 @@ describe Projects::ContainerRepository::CleanupTagsService do
 
     context 'when removing older than 1 day' do
       let(:params) do
-        { 'name_regex' => '.*',
+        { 'name_regex_delete' => '.*',
           'older_than' => '1 day' }
       end
 
@@ -118,7 +170,7 @@ describe Projects::ContainerRepository::CleanupTagsService do
 
     context 'when combining all parameters' do
       let(:params) do
-        { 'name_regex' => '.*',
+        { 'name_regex_delete' => '.*',
           'keep_n' => 1,
           'older_than' => '1 day' }
       end
@@ -136,7 +188,7 @@ describe Projects::ContainerRepository::CleanupTagsService do
 
       context 'with valid container_expiration_policy param' do
         let(:params) do
-          { 'name_regex' => '.*',
+          { 'name_regex_delete' => '.*',
             'keep_n' => 1,
             'older_than' => '1 day',
             'container_expiration_policy' => true }
@@ -152,7 +204,7 @@ describe Projects::ContainerRepository::CleanupTagsService do
 
       context 'without container_expiration_policy param' do
         let(:params) do
-          { 'name_regex' => '.*',
+          { 'name_regex_delete' => '.*',
             'keep_n' => 1,
             'older_than' => '1 day' }
         end