From 3dbf3997bbf51eca8a313c4e152c77c1b038fd5d Mon Sep 17 00:00:00 2001 From: Steve Abrams Date: Mon, 5 Aug 2019 20:00:50 +0000 Subject: Add group level container repository endpoints API endpoints for requesting container repositories and container repositories with their tag information are enabled for users that want to specify the group containing the repository rather than the specific project. --- spec/finders/container_repositories_finder_spec.rb | 44 ++++ spec/fixtures/api/schemas/registry/repository.json | 6 +- spec/models/group_spec.rb | 1 + spec/requests/api/container_registry_spec.rb | 245 --------------------- .../api/group_container_repositories_spec.rb | 57 +++++ .../api/project_container_repositories_spec.rb | 228 +++++++++++++++++++ .../policies/group_policy_shared_context.rb | 2 +- .../container_repositories_shared_examples.rb | 58 +++++ 8 files changed, 394 insertions(+), 247 deletions(-) create mode 100644 spec/finders/container_repositories_finder_spec.rb delete mode 100644 spec/requests/api/container_registry_spec.rb create mode 100644 spec/requests/api/group_container_repositories_spec.rb create mode 100644 spec/requests/api/project_container_repositories_spec.rb create mode 100644 spec/support/shared_examples/container_repositories_shared_examples.rb (limited to 'spec') diff --git a/spec/finders/container_repositories_finder_spec.rb b/spec/finders/container_repositories_finder_spec.rb new file mode 100644 index 00000000000..deec62d6598 --- /dev/null +++ b/spec/finders/container_repositories_finder_spec.rb @@ -0,0 +1,44 @@ +# 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 diff --git a/spec/fixtures/api/schemas/registry/repository.json b/spec/fixtures/api/schemas/registry/repository.json index e0fd4620c43..d0a068b65a7 100644 --- a/spec/fixtures/api/schemas/registry/repository.json +++ b/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 } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 7e9bbf5a407..1c41ceb7deb 100644 --- a/spec/models/group_spec.rb +++ b/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) } diff --git a/spec/requests/api/container_registry_spec.rb b/spec/requests/api/container_registry_spec.rb deleted file mode 100644 index b64f3ea1081..00000000000 --- a/spec/requests/api/container_registry_spec.rb +++ /dev/null @@ -1,245 +0,0 @@ -require 'spec_helper' - -describe API::ContainerRegistry do - include ExclusiveLeaseHelpers - - set(:project) { create(:project, :private) } - set(:maintainer) { create(:user) } - set(:developer) { create(:user) } - set(:reporter) { create(:user) } - set(:guest) { create(:user) } - - let(:root_repository) { create(:container_repository, :root, project: project) } - let(:test_repository) { create(:container_repository, project: project) } - - let(:api_user) { maintainer } - - before do - project.add_maintainer(maintainer) - project.add_developer(developer) - project.add_reporter(reporter) - project.add_guest(guest) - - stub_feature_flags(container_registry_api: true) - stub_container_registry_config(enabled: true) - - root_repository - 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 - - expect(json_response.length).to eq(2) - expect(json_response.map { |repository| repository['id'] }).to contain_exactly( - root_repository.id, test_repository.id) - 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 - - 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 - - context 'for maintainer' do - let(:api_user) { maintainer } - - it 'schedules removal of repository' do - expect(DeleteContainerRepositoryWorker).to receive(:perform_async) - .with(maintainer.id, root_repository.id) - - subject - - expect(response).to have_gitlab_http_status(:accepted) - end - end - end - - 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 - - context 'for reporter' do - let(:api_user) { reporter } - - before do - stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest)) - end - - it 'returns a list of tags' do - subject - - expect(json_response.length).to eq(2) - expect(json_response.map { |repository| repository['name'] }).to eq %w(latest rootA) - end - - it 'returns a matching schema' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/tags') - end - end - end - - 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 - let(:params) do - { name_regex: 'v10.*' } - end - end - - context 'for maintainer' do - let(:api_user) { maintainer } - - context 'without required parameters' do - let(:params) { } - - 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.*', - keep_n: 100, - older_than: '1 day', - other: 'some value' } - end - - let(:worker_params) do - { name_regex: 'v10.*', - keep_n: 100, - older_than: '1 day' } - end - - let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } - - it 'schedules cleanup of tags repository' do - 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 - - context 'called multiple times in one hour' do - it 'returns 400 with an error message' do - stub_exclusive_lease_taken(lease_key, timeout: 1.hour) - subject - - expect(response).to have_gitlab_http_status(400) - expect(response.body).to include('This request has already been made.') - end - - it 'executes service only for the first time' do - expect(CleanupContainerRepositoryWorker).to receive(:perform_async).once - - 2.times { subject } - end - end - end - end - end - - 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 - - context 'for reporter' do - let(:api_user) { reporter } - - before do - stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) - end - - it 'returns a details of tag' do - subject - - expect(json_response).to include( - 'name' => 'rootA', - 'digest' => 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15', - 'revision' => 'd7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac', - 'total_size' => 2319870) - end - - it 'returns a matching schema' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/tag') - end - end - end - - 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 - - context 'for developer' do - let(:api_user) { developer } - - before do - stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) - end - - it 'properly removes tag' do - expect_any_instance_of(ContainerRegistry::Client) - .to receive(:delete_repository_tag).with(root_repository.path, - 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15') - - subject - - expect(response).to have_gitlab_http_status(:ok) - end - end - end -end diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb new file mode 100644 index 00000000000..0a41e455d01 --- /dev/null +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -0,0 +1,57 @@ +# 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 diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb new file mode 100644 index 00000000000..f1dc4e6f0b2 --- /dev/null +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -0,0 +1,228 @@ +require 'spec_helper' + +describe API::ProjectContainerRepositories do + include ExclusiveLeaseHelpers + + set(:project) { create(:project, :private) } + set(:maintainer) { create(:user) } + set(:developer) { create(:user) } + set(:reporter) { create(:user) } + set(: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, + developer: developer, + guest: guest, + maintainer: maintainer, + reporter: reporter + } + end + + let(:api_user) { maintainer } + + before do + project.add_maintainer(maintainer) + project.add_developer(developer) + project.add_reporter(reporter) + project.add_guest(guest) + + stub_feature_flags(container_registry_api: true) + stub_container_registry_config(enabled: true) + + root_repository + test_repository + end + + describe 'GET /projects/:id/registry/repositories' do + let(:url) { "/projects/#{project.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, '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 'rejected container repository access', :developer, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for maintainer' do + let(:api_user) { maintainer } + + it 'schedules removal of repository' do + expect(DeleteContainerRepositoryWorker).to receive(:perform_async) + .with(maintainer.id, root_repository.id) + + subject + + expect(response).to have_gitlab_http_status(:accepted) + end + end + end + + 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 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for reporter' do + let(:api_user) { reporter } + + before do + stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest)) + end + + it 'returns a list of tags' do + subject + + expect(json_response.length).to eq(2) + expect(json_response.map { |repository| repository['name'] }).to eq %w(latest rootA) + end + + it 'returns a matching schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('registry/tags') + end + end + end + + 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 } + + 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 + let(:api_user) { maintainer } + + context 'without required parameters' do + let(:params) { } + + 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.*', + keep_n: 100, + older_than: '1 day', + other: 'some value' } + end + + let(:worker_params) do + { name_regex: 'v10.*', + keep_n: 100, + older_than: '1 day' } + end + + let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } + + it 'schedules cleanup of tags repository' do + 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 + + context 'called multiple times in one hour' do + it 'returns 400 with an error message' do + stub_exclusive_lease_taken(lease_key, timeout: 1.hour) + subject + + expect(response).to have_gitlab_http_status(400) + expect(response.body).to include('This request has already been made.') + end + + it 'executes service only for the first time' do + expect(CleanupContainerRepositoryWorker).to receive(:perform_async).once + + 2.times { subject } + end + end + end + end + end + + 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 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for reporter' do + let(:api_user) { reporter } + + before do + stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) + end + + it 'returns a details of tag' do + subject + + expect(json_response).to include( + 'name' => 'rootA', + 'digest' => 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15', + 'revision' => 'd7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac', + 'total_size' => 2319870) + end + + it 'returns a matching schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('registry/tag') + end + end + end + + 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 'rejected container repository access', :reporter, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for developer' do + let(:api_user) { developer } + + before do + stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) + end + + it 'properly removes tag' do + expect_any_instance_of(ContainerRegistry::Client) + .to receive(:delete_repository_tag).with(root_repository.path, + 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15') + + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index c11725c63d2..fd24c443288 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/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[ diff --git a/spec/support/shared_examples/container_repositories_shared_examples.rb b/spec/support/shared_examples/container_repositories_shared_examples.rb new file mode 100644 index 00000000000..946b130fca2 --- /dev/null +++ b/spec/support/shared_examples/container_repositories_shared_examples.rb @@ -0,0 +1,58 @@ +# 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 -- cgit v1.2.3