From 38d8ab018ddcdb58e7df70ec4fb51aa58abcae3b Mon Sep 17 00:00:00 2001 From: Nicholas Wittstruck Date: Mon, 1 Jul 2024 10:29:51 +0000 Subject: [PATCH 1/3] Protected containers: POST REST API for container protection rules Adds the POST route to the REST API for container protection rules to allow the creation of new container protection rules. Issue https://gitlab.com/gitlab-org/gitlab/-/issues/457518 Changelog: added --- doc/api/api_resources.md | 1 + ...ect_container_registry_protection_rules.md | 42 +++++++++ ...ect_container_registry_protection_rules.rb | 67 +++++++++++---- ...ontainer_registry_protection_rules_spec.rb | 85 +++++++++++++++---- 4 files changed, 164 insertions(+), 31 deletions(-) diff --git a/doc/api/api_resources.md b/doc/api/api_resources.md index f35bbd02caeb9c..e7fc2c9406f63d 100644 --- a/doc/api/api_resources.md +++ b/doc/api/api_resources.md @@ -92,6 +92,7 @@ The following API resources are available in the project context: | [Project-level variables](project_level_variables.md) | `/projects/:id/variables` | | [Projects](projects.md) including setting Webhooks | `/projects`, `/projects/:id/hooks` (also available for users) | | [Protected branches](protected_branches.md) | `/projects/:id/protected_branches` | +| [Protected container registry](project_container_registry_protection_rules.md) | `/projects/:id/registry/protection/rules` | | [Protected environments](protected_environments.md) | `/projects/:id/protected_environments` | | [Protected packages](project_packages_protection_rules.md) | `/projects/:id/protection/rules` | | [Protected tags](protected_tags.md) | `/projects/:id/protected_tags` | diff --git a/doc/api/project_container_registry_protection_rules.md b/doc/api/project_container_registry_protection_rules.md index 99a0e1fad659ff..79844fd1808b39 100644 --- a/doc/api/project_container_registry_protection_rules.md +++ b/doc/api/project_container_registry_protection_rules.md @@ -71,3 +71,45 @@ Example response: }, ] ``` + +## Create a container registry protection rule + +Create a container registry protection rule for a project. + +```plaintext +POST /api/v4/projects/:id/registry/protection/rules +``` + +Supported attributes: + +| Attribute | Type | Required | Description | +|---------------------------------------|-----------------|----------|--------------------------------| +| `id` | integer/string | Yes | ID or [URL-encoded path of the project](rest/index.md#namespaced-path-encoding) owned by the authenticated user. | +| `repository_path_pattern` | string | Yes | Container repository path pattern protected by the protection rule. For example `flight/flight-*`. Wildcard character `*` allowed. | +| `minimum_access_level_for_push` | string | Yes | Minimum GitLab access level to allow to push container images to the container registry. For example `maintainer` or `owner`. | +| `minimum_access_level_for_delete` | string | Yes | Minimum GitLab access level to allow to delete container images in the container registry. For example `maintainer` or `owner`. | + +If successful, returns [`201`](rest/index.md#status-codes) and the created container registry protection rule. + +Can return the following status codes: + +- `201 Created`: The container registry protection rule was created successfully. +- `400 Bad Request`: The container registry protection rule is invalid. +- `401 Unauthorized`: The access token is invalid. +- `403 Forbidden`: The user does not have permission to create a container registry protection rule. +- `404 Not Found`: The project was not found. +- `422 Unprocessable Entity`: The container registry protection rule could not be created, for example, because the `repository_path_pattern` is already taken. + +Example request: + +```shell +curl --request POST \ + --header "PRIVATE-TOKEN: " \ + --header "Content-Type: application/json" \ + --url "https://gitlab.example.com/api/v4/projects/7/registry/protection/rules" \ + --data '{ + "repository_path_pattern": "flightjs/flight-needs-to-be-a-unique-path", + "minimum_access_level_for_push": "maintainer", + "minimum_access_level_for_delete": "maintainer" + }' +``` diff --git a/lib/api/project_container_registry_protection_rules.rb b/lib/api/project_container_registry_protection_rules.rb index 233f88c3b4f711..ce67b39125e84b 100644 --- a/lib/api/project_container_registry_protection_rules.rb +++ b/lib/api/project_container_registry_protection_rules.rb @@ -13,26 +13,61 @@ class ProjectContainerRegistryProtectionRules < ::API::Base authorize! :admin_container_image, user_project end - resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do - desc 'Get list of container registry protection rules for a project' do - success Entities::Projects::ContainerRegistry::Protection::Rule - failure [ - { code: 401, message: 'Unauthorized' }, - { code: 403, message: 'Forbidden' }, - { code: 404, message: 'Not Found' } - ] - tags %w[projects] - is_array true - hidden true - end - end params do requires :id, types: [String, Integer], desc: 'The ID or URL-encoded path of the project' end resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do - get ':id/registry/protection/rules' do - present user_project.container_registry_protection_rules, - with: Entities::Projects::ContainerRegistry::Protection::Rule + resource ':id/registry/protection/rules' do + desc 'Get list of container registry protection rules for a project' do + success Entities::Projects::ContainerRegistry::Protection::Rule + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' } + ] + tags %w[projects] + is_array true + hidden true + end + get do + present user_project.container_registry_protection_rules, + with: Entities::Projects::ContainerRegistry::Protection::Rule + end + + desc 'Create a container protection rule for a project' do + success Entities::Projects::ContainerRegistry::Protection::Rule + failure [ + { code: 400, message: 'Bad Request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' }, + { code: 422, message: 'Unprocessable Entity' } + ] + tags %w[projects] + hidden true + end + params do + requires :repository_path_pattern, type: String, + desc: 'Container repository path pattern protected by the protection rule. + For example `flight/flight-*`. Wildcard character `*` allowed.' + requires :minimum_access_level_for_push, type: String, + values: ContainerRegistry::Protection::Rule.minimum_access_level_for_pushes.keys, + desc: 'Minimum GitLab access level to allow to push container images to the container registry. + For example developer, maintainer, owner.' + requires :minimum_access_level_for_delete, type: String, + values: ContainerRegistry::Protection::Rule.minimum_access_level_for_deletes.keys, + desc: 'Minimum GitLab access level to allow to delete container images in the container registry. + For example developer, maintainer, owner.' + end + post do + response = ::ContainerRegistry::Protection::CreateRuleService.new(user_project, + current_user, declared_params).execute + + render_api_error!({ error: response.message }, :unprocessable_entity) if response.error? + + present response[:container_registry_protection_rule], + with: Entities::Projects::ContainerRegistry::Protection::Rule + end end end end diff --git a/spec/requests/api/project_container_registry_protection_rules_spec.rb b/spec/requests/api/project_container_registry_protection_rules_spec.rb index dd00a28519b2df..ab305c3a9e8d22 100644 --- a/spec/requests/api/project_container_registry_protection_rules_spec.rb +++ b/spec/requests/api/project_container_registry_protection_rules_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::ProjectContainerRegistryProtectionRules, feature_category: :container_registry do +RSpec.describe API::ProjectContainerRegistryProtectionRules, :aggregate_failures, feature_category: :container_registry do include ExclusiveLeaseHelpers let_it_be(:project) { create(:project, :private) } @@ -15,7 +15,13 @@ let_it_be(:invalid_token) { 'invalid-token123' } let_it_be(:headers_with_invalid_token) { { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => invalid_token } } - shared_examples 'rejecting project container protection rules request' do + let(:params) do + { repository_path_pattern: "#{container_registry_protection_rule.repository_path_pattern}-unique", + minimum_access_level_for_push: container_registry_protection_rule.minimum_access_level_for_push, + minimum_access_level_for_delete: container_registry_protection_rule.minimum_access_level_for_delete } + end + + shared_examples 'rejecting project container protection rules request when not enough permissions' do using RSpec::Parameterized::TableSyntax where(:user_role, :status) do @@ -34,14 +40,34 @@ end end + shared_examples 'rejecting container registry protection rules request when enough permissions' do + context 'when feature flag is disabled' do + before do + stub_feature_flags(container_registry_protected_containers: false) + end + + it_behaves_like 'returning response status', :not_found + end + + context 'when the project id is invalid' do + let(:url) { "/projects/invalid/registry/protection/rules" } + + it_behaves_like 'returning response status', :not_found + end + + context 'when the project id does not exist' do + let(:url) { "/projects/#{non_existing_record_id}/registry/protection/rules" } + + it_behaves_like 'returning response status', :not_found + end + end + describe 'GET /projects/:id/registry/protection/rules' do let(:url) { "/projects/#{project.id}/registry/protection/rules" } subject(:get_container_registry_rules) { get(api(url, api_user)) } - context 'when not enough permissions' do - it_behaves_like 'rejecting project container protection rules request' - end + it_behaves_like 'rejecting project container protection rules request when not enough permissions' context 'for maintainer' do let(:api_user) { maintainer } @@ -74,29 +100,58 @@ ) end - context 'when the project id is invalid' do - let(:url) { '/projects/invalid/registry/protection/rules' } + it_behaves_like 'rejecting container registry protection rules request when enough permissions' + end + + context 'with invalid token' do + subject(:get_container_registry_rules) { get(api(url), headers: headers_with_invalid_token) } + + it_behaves_like 'returning response status', :unauthorized + end + end + + describe 'POST /projects/:id/registry/protection/rules' do + let(:url) { "/projects/#{project.id}/registry/protection/rules" } + + subject(:post_container_registry_rule) { post(api(url, api_user), params: params) } + + it_behaves_like 'rejecting project container protection rules request when not enough permissions' - it_behaves_like 'returning response status', :not_found + context 'for maintainer' do + let(:api_user) { maintainer } + + it 'creates a container registry protection rule' do + expect { post_container_registry_rule }.to change { ContainerRegistry::Protection::Rule.count }.by(1) + expect(response).to have_gitlab_http_status(:created) end - context 'when the project id does not exist' do - let(:url) { "/projects/#{non_existing_record_id}/registry/protection/rules" } + context 'with invalid minimum_access_level_for_push' do + before do + params[:minimum_access_level_for_push] = "not in enum" + end - it_behaves_like 'returning response status', :not_found + it 'does not create a container registry protection rule' do + expect { post_container_registry_rule }.to not_change(ContainerRegistry::Protection::Rule, :count) + expect(response).to have_gitlab_http_status(:bad_request) + end end - context 'when container_registry_protected_containers is disabled' do + context 'with already existing repository_path_pattern' do before do - stub_feature_flags(container_registry_protected_containers: false) + params[:repository_path_pattern] = container_registry_protection_rule.repository_path_pattern end - it_behaves_like 'returning response status', :not_found + it 'does not create a container registry protection rule' do + expect { post_container_registry_rule }.to not_change(ContainerRegistry::Protection::Rule, :count) + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end end + + it_behaves_like 'rejecting container registry protection rules request when enough permissions' end context 'with invalid token' do - subject(:get_container_registry_rules) { get(api(url), headers: headers_with_invalid_token) } + subject(:post_container_registry_rules) { post(api(url), headers: headers_with_invalid_token, params: params) } it_behaves_like 'returning response status', :unauthorized end -- GitLab From cccd7c2cc15b38feb11903e7937f38729e0a464d Mon Sep 17 00:00:00 2001 From: Nicholas <1494491-nwittstruck@users.noreply.gitlab.com> Date: Wed, 3 Jul 2024 06:55:38 +0000 Subject: [PATCH 2/3] Applies documentation feedback: - Add milestone 17.2 to history for endpoint - Format attribute table --- .../project_container_registry_protection_rules.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/doc/api/project_container_registry_protection_rules.md b/doc/api/project_container_registry_protection_rules.md index 79844fd1808b39..5db5b9d2d04909 100644 --- a/doc/api/project_container_registry_protection_rules.md +++ b/doc/api/project_container_registry_protection_rules.md @@ -74,6 +74,8 @@ Example response: ## Create a container registry protection rule +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/457518) in GitLab 17.2. + Create a container registry protection rule for a project. ```plaintext @@ -82,12 +84,12 @@ POST /api/v4/projects/:id/registry/protection/rules Supported attributes: -| Attribute | Type | Required | Description | -|---------------------------------------|-----------------|----------|--------------------------------| -| `id` | integer/string | Yes | ID or [URL-encoded path of the project](rest/index.md#namespaced-path-encoding) owned by the authenticated user. | -| `repository_path_pattern` | string | Yes | Container repository path pattern protected by the protection rule. For example `flight/flight-*`. Wildcard character `*` allowed. | -| `minimum_access_level_for_push` | string | Yes | Minimum GitLab access level to allow to push container images to the container registry. For example `maintainer` or `owner`. | -| `minimum_access_level_for_delete` | string | Yes | Minimum GitLab access level to allow to delete container images in the container registry. For example `maintainer` or `owner`. | +| Attribute | Type | Required | Description | +|-----------------------------------|----------------|----------|-------------| +| `id` | integer/string | Yes | ID or [URL-encoded path of the project](rest/index.md#namespaced-path-encoding) owned by the authenticated user. | +| `repository_path_pattern` | string | Yes | Container repository path pattern protected by the protection rule. For example `flight/flight-*`. Wildcard character `*` allowed. | +| `minimum_access_level_for_push` | string | Yes | Minimum GitLab access level to allow to push container images to the container registry. For example `maintainer` or `owner`. | +| `minimum_access_level_for_delete` | string | Yes | Minimum GitLab access level to allow to delete container images in the container registry. For example `maintainer` or `owner`. | If successful, returns [`201`](rest/index.md#status-codes) and the created container registry protection rule. -- GitLab From 2392d78636a88612ccb40855decbb7cb698e973f Mon Sep 17 00:00:00 2001 From: Nicholas Wittstruck Date: Wed, 3 Jul 2024 08:55:36 +0000 Subject: [PATCH 3/3] Protected containers: POST REST API for container protection rules Adds the POST route to the REST API for container protection rules to allow the creation of new container protection rules. Based on review feedback, this commit makes the restrictions optional. Issue https://gitlab.com/gitlab-org/gitlab/-/issues/457518 Changelog: added --- ...ect_container_registry_protection_rules.md | 4 +- ...ect_container_registry_protection_rules.rb | 9 ++-- ...ontainer_registry_protection_rules_spec.rb | 45 +++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/doc/api/project_container_registry_protection_rules.md b/doc/api/project_container_registry_protection_rules.md index 5db5b9d2d04909..cdbd8be23ee383 100644 --- a/doc/api/project_container_registry_protection_rules.md +++ b/doc/api/project_container_registry_protection_rules.md @@ -88,8 +88,8 @@ Supported attributes: |-----------------------------------|----------------|----------|-------------| | `id` | integer/string | Yes | ID or [URL-encoded path of the project](rest/index.md#namespaced-path-encoding) owned by the authenticated user. | | `repository_path_pattern` | string | Yes | Container repository path pattern protected by the protection rule. For example `flight/flight-*`. Wildcard character `*` allowed. | -| `minimum_access_level_for_push` | string | Yes | Minimum GitLab access level to allow to push container images to the container registry. For example `maintainer` or `owner`. | -| `minimum_access_level_for_delete` | string | Yes | Minimum GitLab access level to allow to delete container images in the container registry. For example `maintainer` or `owner`. | +| `minimum_access_level_for_push` | string | No | Minimum GitLab access level to allow to push container images to the container registry. For example `maintainer`, `owner` or `admin`. Must be provided when `minimum_access_level_for_delete` is not set. | +| `minimum_access_level_for_delete` | string | No | Minimum GitLab access level to allow to delete container images in the container registry. For example `maintainer`, `owner`, `admin`. Must be provided when `minimum_access_level_for_push` is not set. | If successful, returns [`201`](rest/index.md#status-codes) and the created container registry protection rule. diff --git a/lib/api/project_container_registry_protection_rules.rb b/lib/api/project_container_registry_protection_rules.rb index ce67b39125e84b..55520481e120e3 100644 --- a/lib/api/project_container_registry_protection_rules.rb +++ b/lib/api/project_container_registry_protection_rules.rb @@ -50,14 +50,15 @@ class ProjectContainerRegistryProtectionRules < ::API::Base requires :repository_path_pattern, type: String, desc: 'Container repository path pattern protected by the protection rule. For example `flight/flight-*`. Wildcard character `*` allowed.' - requires :minimum_access_level_for_push, type: String, + optional :minimum_access_level_for_push, type: String, values: ContainerRegistry::Protection::Rule.minimum_access_level_for_pushes.keys, desc: 'Minimum GitLab access level to allow to push container images to the container registry. - For example developer, maintainer, owner.' - requires :minimum_access_level_for_delete, type: String, + For example maintainer, owner or admin.' + optional :minimum_access_level_for_delete, type: String, values: ContainerRegistry::Protection::Rule.minimum_access_level_for_deletes.keys, desc: 'Minimum GitLab access level to allow to delete container images in the container registry. - For example developer, maintainer, owner.' + For example maintainer, owner or admin.' + at_least_one_of :minimum_access_level_for_push, :minimum_access_level_for_delete end post do response = ::ContainerRegistry::Protection::CreateRuleService.new(user_project, diff --git a/spec/requests/api/project_container_registry_protection_rules_spec.rb b/spec/requests/api/project_container_registry_protection_rules_spec.rb index ab305c3a9e8d22..4c29daf73ec594 100644 --- a/spec/requests/api/project_container_registry_protection_rules_spec.rb +++ b/spec/requests/api/project_container_registry_protection_rules_spec.rb @@ -125,6 +125,39 @@ expect(response).to have_gitlab_http_status(:created) end + context 'with empty minimum_access_level_for_push' do + before do + params[:minimum_access_level_for_push] = nil + end + + it 'creates a container registry protection rule' do + expect { post_container_registry_rule }.to change { ContainerRegistry::Protection::Rule.count }.by(1) + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'with invalid minimum_access_level_for_delete' do + before do + params[:minimum_access_level_for_delete] = "not in enum" + end + + it 'does not create a container registry protection rule' do + expect { post_container_registry_rule }.to not_change(ContainerRegistry::Protection::Rule, :count) + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'with empty minimum_access_level_for_delete' do + before do + params[:minimum_access_level_for_delete] = nil + end + + it 'creates a container registry protection rule' do + expect { post_container_registry_rule }.to change { ContainerRegistry::Protection::Rule.count }.by(1) + expect(response).to have_gitlab_http_status(:created) + end + end + context 'with invalid minimum_access_level_for_push' do before do params[:minimum_access_level_for_push] = "not in enum" @@ -147,6 +180,18 @@ end end + context 'with neither minimum_access_level_for_push nor minimum_access_level_for_delete' do + before do + params[:minimum_access_level_for_push] = nil + params[:minimum_access_level_for_delete] = nil + end + + it 'does not create a container registry protection rule' do + expect { post_container_registry_rule }.to not_change(ContainerRegistry::Protection::Rule, :count) + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + it_behaves_like 'rejecting container registry protection rules request when enough permissions' end -- GitLab