Merge branch '26866-api-endpoint-to-list-the-docker-images-tags-of-a-group' into 'master'

Resolve "API endpoint to list the Docker images/tags of a group"

See merge request gitlab-org/gitlab-ce!30817

app/finders/container_repositories_finder.rb

+# frozen_string_literal: true
+
+class ContainerRepositoriesFinder
+  # id: group or project id
+  # container_type: :group or :project
+  def initialize(id:, container_type:)
+    @id = id
+    @type = container_type.to_sym
+  end
+
+  def execute
+    if project_type?
+      project.container_repositories
+    else
+      group.container_repositories
+    end
+  end
+
+  private
+
+  attr_reader :id, :type
+
+  def project_type?
+    type == :project
+  end
+
+  def project
+    Project.find(id)
+  end
+
+  def group
+    Group.find(id)
+  end
+end

app/models/group.rb

@@ -44,6 +44,8 @@ class Group < Namespace
   has_many :cluster_groups, class_name: 'Clusters::Group'
   has_many :clusters, through: :cluster_groups, class_name: 'Clusters::Cluster'
 
+  has_many :container_repositories, through: :projects
+
   has_many :todos
 
   accepts_nested_attributes_for :variables, allow_destroy: true

app/policies/group_policy.rb

@@ -68,6 +68,7 @@ class GroupPolicy < BasePolicy
   rule { developer }.enable :admin_milestone
 
   rule { reporter }.policy do
+    enable :read_container_image
     enable :admin_label
     enable :admin_list
     enable :admin_issue

changelogs/unreleased/26866-api-endpoint-to-list-the-docker-images-tags-of-a-group.yml

+---
+title: Add API endpoints to return container repositories and tags from the group
+  level
+merge_request: 30817
+author:
+type: added

doc/api/container_registry.md

@@ -6,6 +6,8 @@ This is the API docs of the [GitLab Container Registry](../user/project/containe
 
 ## List registry repositories
 
+### Within a project
+
 Get a list of registry repositories in a project.
 
 ```
@@ -14,7 +16,8 @@ GET /projects/:id/registry/repositories
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user. |
+| `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) accessible by the authenticated user. |
+| `tags`      | boolean | no | If the param is included as true, each repository will include an array of `"tags"` in the response. |
 
 ```bash
 curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/5/registry/repositories"
@@ -28,6 +31,7 @@ Example response:
     "id": 1,
     "name": "",
     "path": "group/project",
+    "project_id": 9,
     "location": "gitlab.example.com:5000/group/project",
     "created_at": "2019-01-10T13:38:57.391Z"
   },
@@ -35,12 +39,77 @@ Example response:
     "id": 2,
     "name": "releases",
     "path": "group/project/releases",
+    "project_id": 9,
     "location": "gitlab.example.com:5000/group/project/releases",
     "created_at": "2019-01-10T13:39:08.229Z"
   }
 ]
 ```
 
+### Within a group
+
+Get a list of registry repositories in a group.
+
+```
+GET /groups/:id/registry/repositories
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) accessible by the authenticated user. |
+| `tags`      | boolean | no | If the param is included as true, each repository will include an array of `"tags"` in the response. |
+
+```bash
+curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/groups/2/registry/repositories?tags=1"
+```
+
+Example response:
+
+```json
+[
+  {
+    "id": 1,
+    "name": "",
+    "path": "group/project",
+    "project_id": 9,
+    "location": "gitlab.example.com:5000/group/project",
+    "created_at": "2019-01-10T13:38:57.391Z",
+    "tags": [
+      {
+        "name": "0.0.1",
+        "path": "group/project:0.0.1",
+        "location": "gitlab.example.com:5000/group/project:0.0.1"
+      }
+    ]
+  },
+  {
+    "id": 2,
+    "name": "",
+    "path": "group/other_project",
+    "project_id": 11,
+    "location": "gitlab.example.com:5000/group/other_project",
+    "created_at": "2019-01-10T13:39:08.229Z",
+    "tags": [
+      {
+        "name": "0.0.1",
+        "path": "group/other_project:0.0.1",
+        "location": "gitlab.example.com:5000/group/other_project:0.0.1"
+      },
+      {
+        "name": "0.0.2",
+        "path": "group/other_project:0.0.2",
+        "location": "gitlab.example.com:5000/group/other_project:0.0.2"
+      },
+      {
+        "name": "latest",
+        "path": "group/other_project:latest",
+        "location": "gitlab.example.com:5000/group/other_project:latest"
+      }
+    ]
+  }
+]
+```
+
 ## Delete registry repository
 
 Delete a repository in registry.
@@ -62,6 +131,8 @@ curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" "https://git
 
 ## List repository tags
 
+### Within a project
+
 Get a list of tags for given registry repository.
 
 ```
@@ -70,7 +141,7 @@ GET /projects/:id/registry/repositories/:repository_id/tags
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user. |
+| `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) accessible by the authenticated user. |
 | `repository_id` | integer | yes | The ID of registry repository. |
 
 ```bash
@@ -104,7 +175,7 @@ GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user. |
+| `id`      | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) accessible by the authenticated user. |
 | `repository_id` | integer | yes | The ID of registry repository. |
 | `tag_name` | string | yes | The name of tag. |
 

lib/api/api.rb

@@ -104,7 +104,6 @@ module API
     mount ::API::BroadcastMessages
     mount ::API::Commits
     mount ::API::CommitStatuses
-    mount ::API::ContainerRegistry
     mount ::API::DeployKeys
     mount ::API::Deployments
     mount ::API::Environments
@@ -116,6 +115,7 @@ module API
     mount ::API::GroupLabels
     mount ::API::GroupMilestones
     mount ::API::Groups
+    mount ::API::GroupContainerRepositories
     mount ::API::GroupVariables
     mount ::API::ImportGithub
     mount ::API::Internal
@@ -138,6 +138,7 @@ module API
     mount ::API::Pipelines
     mount ::API::PipelineSchedules
     mount ::API::ProjectClusters
+    mount ::API::ProjectContainerRepositories
     mount ::API::ProjectEvents
     mount ::API::ProjectExport
     mount ::API::ProjectImport

lib/api/entities/container_registry.rb

@@ -3,18 +3,20 @@
 module API
   module Entities
     module ContainerRegistry
-      class Repository < Grape::Entity
-        expose :id
+      class Tag < Grape::Entity
         expose :name
         expose :path
         expose :location
-        expose :created_at
       end
 
-      class Tag < Grape::Entity
+      class Repository < Grape::Entity
+        expose :id
         expose :name
         expose :path
+        expose :project_id
         expose :location
+        expose :created_at
+        expose :tags, using: Tag, if: -> (_, options) { options[:tags] }
       end
 
       class TagDetails < Tag

lib/api/group_container_repositories.rb

+# frozen_string_literal: true
+
+module API
+  class GroupContainerRepositories < Grape::API
+    include PaginationParams
+
+    before { authorize_read_group_container_images! }
+
+    REPOSITORY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(
+      tag_name: API::NO_SLASH_URL_PART_REGEX)
+
+    params do
+      requires :id, type: String, desc: "Group's ID or path"
+    end
+    resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+      desc 'Get a list of all repositories within a group' do
+        detail 'This feature was introduced in GitLab 12.2.'
+        success Entities::ContainerRegistry::Repository
+      end
+      params do
+        use :pagination
+        optional :tags, type: Boolean, default: false, desc: 'Determines if tags should be included'
+      end
+      get ':id/registry/repositories' do
+        repositories = ContainerRepositoriesFinder.new(
+          id: user_group.id, container_type: :group
+        ).execute
+
+        present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags]
+      end
+    end
+
+    helpers do
+      def authorize_read_group_container_images!
+        authorize! :read_container_image, user_group
+      end
+    end
+  end
+end

lib/api/container_registry.rb → lib/api/project_container_repositories.rb

 # frozen_string_literal: true
 
 module API
-  class ContainerRegistry < Grape::API
+  class ProjectContainerRepositories < Grape::API
     include PaginationParams
 
-    REGISTRY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(
+    REPOSITORY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(
       tag_name: API::NO_SLASH_URL_PART_REGEX)
 
     before { error!('404 Not Found', 404) unless Feature.enabled?(:container_registry_api, user_project, default_enabled: true) }
@@ -20,11 +20,14 @@ module API
       end
       params do
         use :pagination
+        optional :tags, type: Boolean, default: false, desc: 'Determines if tags should be included'
       end
       get ':id/registry/repositories' do
-        repositories = user_project.container_repositories.ordered
+        repositories = ContainerRepositoriesFinder.new(
+          id: user_project.id, container_type: :project
+        ).execute
 
-        present paginate(repositories), with: Entities::ContainerRegistry::Repository
+        present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags]
       end
 
       desc 'Delete repository' do
@@ -33,7 +36,7 @@ module API
       params do
         requires :repository_id, type: Integer, desc: 'The ID of the repository'
       end
-      delete ':id/registry/repositories/:repository_id', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do
+      delete ':id/registry/repositories/:repository_id', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do
         authorize_admin_container_image!
 
         DeleteContainerRepositoryWorker.perform_async(current_user.id, repository.id)
@@ -49,7 +52,7 @@ module API
         requires :repository_id, type: Integer, desc: 'The ID of the repository'
         use :pagination
       end
-      get ':id/registry/repositories/:repository_id/tags', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do
+      get ':id/registry/repositories/:repository_id/tags', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do
         authorize_read_container_image!
 
         tags = Kaminari.paginate_array(repository.tags)
@@ -65,7 +68,7 @@ module API
         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
-      delete ':id/registry/repositories/:repository_id/tags', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do
+      delete ':id/registry/repositories/:repository_id/tags', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do
         authorize_admin_container_image!
 
         message = 'This request has already been made. You can run this at most once an hour for a given container repository'
@@ -85,7 +88,7 @@ module API
         requires :repository_id, type: Integer, desc: 'The ID of the repository'
         requires :tag_name, type: String, desc: 'The name of the tag'
       end
-      get ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do
+      get ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do
         authorize_read_container_image!
         validate_tag!
 
@@ -99,7 +102,7 @@ module API
         requires :repository_id, type: Integer, desc: 'The ID of the repository'
         requires :tag_name, type: String, desc: 'The name of the tag'
       end
-      delete ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do
+      delete ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do
         authorize_destroy_container_image!
         validate_tag!
 

spec/finders/container_repositories_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ContainerRepositoriesFinder do
+  let(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
+  let(:project_repository) { create(:container_repository, project: project) }
+
+  describe '#execute' do
+    let(:id) { nil }
+
+    subject { described_class.new(id: id, container_type: container_type).execute }
+
+    context 'when container_type is group' do
+      let(:other_project) { create(:project, group: group) }
+
+      let(:other_repository) do
+        create(:container_repository, name: 'test_repository2', project: other_project)
+      end
+
+      let(:container_type) { :group }
+      let(:id) { group.id }
+
+      it { is_expected.to match_array([project_repository, other_repository]) }
+    end
+
+    context 'when container_type is project' do
+      let(:container_type) { :project }
+      let(:id) { project.id }
+
+      it { is_expected.to match_array([project_repository]) }
+    end
+
+    context 'with invalid id' do
+      let(:container_type) { :project }
+      let(:id) { 123456789 }
+
+      it 'raises an error' do
+        expect { subject.execute }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
+end

spec/fixtures/api/schemas/registry/repository.json

@@ -17,6 +17,9 @@
     "path": {
       "type": "string"
     },
+    "project_id": {
+      "type": "integer"
+    },
     "location": {
       "type": "string"
     },
@@ -28,7 +31,8 @@
     },
     "destroy_path": {
       "type": "string"
-    }
+    },
+    "tags": { "$ref": "tags.json" }
   },
   "additionalProperties": false
 }

spec/models/group_spec.rb

@@ -23,6 +23,7 @@ describe Group do
     it { is_expected.to have_many(:badges).class_name('GroupBadge') }
     it { is_expected.to have_many(:cluster_groups).class_name('Clusters::Group') }
     it { is_expected.to have_many(:clusters).class_name('Clusters::Cluster') }
+    it { is_expected.to have_many(:container_repositories) }
 
     describe '#members & #requesters' do
       let(:requester) { create(:user) }

spec/requests/api/group_container_repositories_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::GroupContainerRepositories do
+  set(:group) { create(:group, :private) }
+  set(:project) { create(:project, :private, group: group) }
+  let(:reporter) { create(:user) }
+  let(:guest) { create(:user) }
+
+  let(:root_repository) { create(:container_repository, :root, project: project) }
+  let(:test_repository) { create(:container_repository, project: project) }
+
+  let(:users) do
+    {
+      anonymous: nil,
+      guest: guest,
+      reporter: reporter
+    }
+  end
+
+  let(:api_user) { reporter }
+
+  before do
+    group.add_reporter(reporter)
+    group.add_guest(guest)
+
+    stub_feature_flags(container_registry_api: true)
+    stub_container_registry_config(enabled: true)
+
+    root_repository
+    test_repository
+  end
+
+  describe 'GET /groups/:id/registry/repositories' do
+    let(:url) { "/groups/#{group.id}/registry/repositories" }
+
+    subject { get api(url, api_user) }
+
+    it_behaves_like 'rejected container repository access', :guest, :forbidden
+    it_behaves_like 'rejected container repository access', :anonymous, :not_found
+
+    it_behaves_like 'returns repositories for allowed users', :reporter, 'group' do
+      let(:object) { group }
+    end
+
+    context 'with invalid group id' do
+      let(:url) { '/groups/123412341234/registry/repositories' }
+
+      it 'returns not found' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+end

spec/requests/api/container_registry_spec.rb → spec/requests/api/project_container_repositories_spec.rb

 require 'spec_helper'
 
-describe API::ContainerRegistry do
+describe API::ProjectContainerRepositories do
   include ExclusiveLeaseHelpers
 
   set(:project) { create(:project, :private) }
@@ -12,6 +12,16 @@ describe API::ContainerRegistry do
   let(:root_repository) { create(:container_repository, :root, project: project) }
   let(:test_repository) { create(:container_repository, project: project) }
 
+  let(:users) do
+    {
+      anonymous: nil,
+      developer: developer,
+      guest: guest,
+      maintainer: maintainer,
+      reporter: reporter
+    }
+  end
+
   let(:api_user) { maintainer }
 
   before do
@@ -27,57 +37,24 @@ describe API::ContainerRegistry do
     test_repository
   end
 
-  shared_examples 'being disallowed' do |param|
-    context "for #{param}" do
-      let(:api_user) { public_send(param) }
-
-      it 'returns access denied' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:forbidden)
-      end
-    end
-
-    context "for anonymous" do
-      let(:api_user) { nil }
-
-      it 'returns not found' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
-  end
-
   describe 'GET /projects/:id/registry/repositories' do
-    subject { get api("/projects/#{project.id}/registry/repositories", api_user) }
-
-    it_behaves_like 'being disallowed', :guest
-
-    context 'for reporter' do
-      let(:api_user) { reporter }
-
-      it 'returns a list of repositories' do
-        subject
+    let(:url) { "/projects/#{project.id}/registry/repositories" }
 
-        expect(json_response.length).to eq(2)
-        expect(json_response.map { |repository| repository['id'] }).to contain_exactly(
-          root_repository.id, test_repository.id)
-      end
+    subject { get api(url, api_user) }
 
-      it 'returns a matching schema' do
-        subject
+    it_behaves_like 'rejected container repository access', :guest, :forbidden
+    it_behaves_like 'rejected container repository access', :anonymous, :not_found
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to match_response_schema('registry/repositories')
-      end
+    it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do
+      let(:object) { project }
     end
   end
 
   describe 'DELETE /projects/:id/registry/repositories/:repository_id' do
     subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}", api_user) }
 
-    it_behaves_like 'being disallowed', :developer
+    it_behaves_like 'rejected container repository access', :developer, :forbidden
+    it_behaves_like 'rejected container repository access', :anonymous, :not_found
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
@@ -96,7 +73,8 @@ describe API::ContainerRegistry do
   describe 'GET /projects/:id/registry/repositories/:repository_id/tags' do
     subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user) }
 
-    it_behaves_like 'being disallowed', :guest
+    it_behaves_like 'rejected container repository access', :guest, :forbidden
+    it_behaves_like 'rejected container repository access', :anonymous, :not_found
 
     context 'for reporter' do
       let(:api_user) { reporter }
@@ -124,10 +102,13 @@ describe API::ContainerRegistry do
   describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags' do
     subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user), params: params }
 
-    it_behaves_like 'being disallowed', :developer do
+    context 'disallowed' do
       let(:params) do
         { name_regex: 'v10.*' }
       end
+
+      it_behaves_like 'rejected container repository access', :developer, :forbidden
+      it_behaves_like 'rejected container repository access', :anonymous, :not_found
     end
 
     context 'for maintainer' do
@@ -191,7 +172,8 @@ describe API::ContainerRegistry do
   describe 'GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do
     subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) }
 
-    it_behaves_like 'being disallowed', :guest
+    it_behaves_like 'rejected container repository access', :guest, :forbidden
+    it_behaves_like 'rejected container repository access', :anonymous, :not_found
 
     context 'for reporter' do
       let(:api_user) { reporter }
@@ -222,7 +204,8 @@ describe API::ContainerRegistry do
   describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do
     subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) }
 
-    it_behaves_like 'being disallowed', :reporter
+    it_behaves_like 'rejected container repository access', :reporter, :forbidden
+    it_behaves_like 'rejected container repository access', :anonymous, :not_found
 
     context 'for developer' do
       let(:api_user) { developer }

spec/support/shared_contexts/policies/group_policy_shared_context.rb

@@ -16,7 +16,7 @@ RSpec.shared_context 'GroupPolicy context' do
       read_group_merge_requests
    ]
   end
-  let(:reporter_permissions) { [:admin_label] }
+  let(:reporter_permissions) { %i[admin_label read_container_image] }
   let(:developer_permissions) { [:admin_milestone] }
   let(:maintainer_permissions) do
     %i[

spec/support/shared_examples/container_repositories_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'rejected container repository access' do |user_type, status|
+  context "for #{user_type}" do
+    let(:api_user) { users[user_type] }
+
+    it "returns #{status}" do
+      subject
+
+      expect(response).to have_gitlab_http_status(status)
+    end
+  end
+end
+
+shared_examples 'returns repositories for allowed users' do |user_type, scope|
+  context "for #{user_type}" do
+    it 'returns a list of repositories' do
+      subject
+
+      expect(json_response.length).to eq(2)
+      expect(json_response.map { |repository| repository['id'] }).to contain_exactly(
+        root_repository.id, test_repository.id)
+      expect(response.body).not_to include('tags')
+    end
+
+    it 'returns a matching schema' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(response).to match_response_schema('registry/repositories')
+    end
+
+    context 'with tags param' do
+      let(:url) { "/#{scope}s/#{object.id}/registry/repositories?tags=true" }
+
+      before do
+        stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest), with_manifest: true)
+        stub_container_registry_tags(repository: test_repository.path, tags: %w(rootA latest), with_manifest: true)
+      end
+
+      it 'returns a list of repositories and their tags' do
+        subject
+
+        expect(json_response.length).to eq(2)
+        expect(json_response.map { |repository| repository['id'] }).to contain_exactly(
+          root_repository.id, test_repository.id)
+        expect(response.body).to include('tags')
+      end
+
+      it 'returns a matching schema' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('registry/repositories')
+      end
+    end
+  end
+end