From fe8f0819b40f534d5592c2c8749b3a326f79d5d2 Mon Sep 17 00:00:00 2001 From: Gerardo Date: Thu, 16 Nov 2023 21:33:18 +0100 Subject: [PATCH 1/3] feat: Add graphql mutation to update container protection rules This MR includes: - New graphql endpoint for updating container registry protection rules. Changelog: added --- .../protection/rule/update.rb | 66 +++++++ app/graphql/types/mutation_type.rb | 1 + .../protection/update_rule_service.rb | 54 ++++++ doc/api/graphql/reference/index.md | 28 +++ locale/gitlab.pot | 3 + .../protection/rule/update_spec.rb | 143 +++++++++++++++ .../protection/update_rule_service_spec.rb | 167 ++++++++++++++++++ 7 files changed, 462 insertions(+) create mode 100644 app/graphql/mutations/container_registry/protection/rule/update.rb create mode 100644 app/services/container_registry/protection/update_rule_service.rb create mode 100644 spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb create mode 100644 spec/services/container_registry/protection/update_rule_service_spec.rb diff --git a/app/graphql/mutations/container_registry/protection/rule/update.rb b/app/graphql/mutations/container_registry/protection/rule/update.rb new file mode 100644 index 00000000000000..b8c39a0927eaa1 --- /dev/null +++ b/app/graphql/mutations/container_registry/protection/rule/update.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Mutations + module ContainerRegistry + module Protection + module Rule + class Update < ::Mutations::BaseMutation + graphql_name 'UpdateContainerRegistryProtectionRule' + description 'Updates a container registry protection rule to restrict access to project containers. ' \ + 'You can prevent users without certain permissions from altering containers. ' \ + 'Available only when feature flag `container_registry_protected_containers` is enabled.' + + authorize :admin_container_image + + argument :id, + ::Types::GlobalIDType[::ContainerRegistry::Protection::Rule], + required: true, + description: 'Global ID of the container registry protection rule to be updated.' + + argument :container_path_pattern, + GraphQL::Types::String, + required: false, + validates: { allow_blank: false }, + description: + 'Container path pattern by the protection rule. For example, `my-scope/my-project/container-dev-*`. ' \ + 'Wildcard character `*` allowed.' + + argument :delete_protected_up_to_access_level, + Types::ContainerRegistry::Protection::RuleAccessLevelEnum, + required: false, + validates: { allow_blank: false }, + description: + 'Maximum GitLab access level unable to delete a container. ' \ + 'For example, `DEVELOPER`, `MAINTAINER`, `OWNER`.' + + argument :push_protected_up_to_access_level, + Types::ContainerRegistry::Protection::RuleAccessLevelEnum, + required: false, + validates: { allow_blank: false }, + description: + 'Maximum GitLab access level unable to push a container. ' \ + 'For example, `DEVELOPER`, `MAINTAINER`, `OWNER`.' + + field :container_registry_protection_rule, + Types::ContainerRegistry::Protection::RuleType, + null: true, + description: 'Container registry protection rule after mutation.' + + def resolve(id:, **kwargs) + container_registry_protection_rule = authorized_find!(id: id) + + if Feature.disabled?(:container_registry_protected_containers, container_registry_protection_rule.project) + raise_resource_not_available_error!("'container_registry_protected_containers' feature flag is disabled") + end + + response = ::ContainerRegistry::Protection::UpdateRuleService.new(container_registry_protection_rule, + current_user: current_user, params: kwargs).execute + + { container_registry_protection_rule: response.payload[:container_registry_protection_rule], + errors: response.errors } + end + end + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 5593911d1fbe5c..445176164833a2 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -140,6 +140,7 @@ class MutationType < BaseObject mount_mutation Mutations::ContainerExpirationPolicies::Update mount_mutation Mutations::ContainerRegistry::Protection::Rule::Create, alpha: { milestone: '16.6' } mount_mutation Mutations::ContainerRegistry::Protection::Rule::Delete, alpha: { milestone: '16.7' } + mount_mutation Mutations::ContainerRegistry::Protection::Rule::Update, alpha: { milestone: '16.7' } mount_mutation Mutations::ContainerRepositories::Destroy mount_mutation Mutations::ContainerRepositories::DestroyTags mount_mutation Mutations::Ci::Catalog::Resources::Create, alpha: { milestone: '15.11' } diff --git a/app/services/container_registry/protection/update_rule_service.rb b/app/services/container_registry/protection/update_rule_service.rb new file mode 100644 index 00000000000000..e2667e442a0a9e --- /dev/null +++ b/app/services/container_registry/protection/update_rule_service.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module ContainerRegistry + module Protection + class UpdateRuleService + include Gitlab::Allowable + + ALLOWED_ATTRIBUTES = %i[ + container_path_pattern + delete_protected_up_to_access_level + push_protected_up_to_access_level + ].freeze + + def initialize(container_registry_protection_rule, current_user:, params:) + if container_registry_protection_rule.blank? || current_user.blank? + raise ArgumentError, + 'container_registry_protection_rule and current_user must be set' + end + + @container_registry_protection_rule = container_registry_protection_rule + @current_user = current_user + @params = params || {} + end + + def execute + unless can?(current_user, :admin_container_image, container_registry_protection_rule.project) + error_message = _('Unauthorized to update a container registry protection rule') + return service_response_error(message: error_message) + end + + container_registry_protection_rule.update(params.slice(*ALLOWED_ATTRIBUTES)) + + if container_registry_protection_rule.errors.present? + return service_response_error(message: container_registry_protection_rule.errors.full_messages) + end + + ServiceResponse.success(payload: { container_registry_protection_rule: container_registry_protection_rule }) + rescue StandardError => e + service_response_error(message: e.message) + end + + private + + attr_reader :container_registry_protection_rule, :current_user, :params + + def service_response_error(message:) + ServiceResponse.error( + message: message, + payload: { container_registry_protection_rule: nil } + ) + end + end + end +end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index d0e18259bb4983..0112eb6f12ebff 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -7464,6 +7464,34 @@ Input type: `UpdateContainerExpirationPolicyInput` | `containerExpirationPolicy` | [`ContainerExpirationPolicy`](#containerexpirationpolicy) | Container expiration policy after mutation. | | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +### `Mutation.updateContainerRegistryProtectionRule` + +Updates a container registry protection rule to restrict access to project containers. You can prevent users without certain permissions from altering containers. Available only when feature flag `container_registry_protected_containers` is enabled. + +WARNING: +**Introduced** in 16.7. +This feature is an Experiment. It can be changed or removed at any time. + +Input type: `UpdateContainerRegistryProtectionRuleInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `containerPathPattern` | [`String`](#string) | Container path pattern by the protection rule. For example, `my-scope/my-project/container-dev-*`. Wildcard character `*` allowed. | +| `deleteProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level unable to delete a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | +| `id` | [`ContainerRegistryProtectionRuleID!`](#containerregistryprotectionruleid) | Global ID of the container registry protection rule to be updated. | +| `pushProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level unable to push a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `containerRegistryProtectionRule` | [`ContainerRegistryProtectionRule`](#containerregistryprotectionrule) | Container registry protection rule after mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `Mutation.updateDependencyProxyImageTtlGroupPolicy` These settings can be adjusted by the group Owner or Maintainer. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f8888a5e65bbb4..9d9262ae963dd4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -51673,6 +51673,9 @@ msgstr "" msgid "Unauthorized to delete a package protection rule" msgstr "" +msgid "Unauthorized to update a container registry protection rule" +msgstr "" + msgid "Unauthorized to update a package protection rule" msgstr "" diff --git a/spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb b/spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb new file mode 100644 index 00000000000000..64eb4a142cfb48 --- /dev/null +++ b/spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating the container registry protection rule', :aggregate_failures, feature_category: :container_registry do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be_with_reload(:container_registry_protection_rule) do + create(:container_registry_protection_rule, project: project, push_protected_up_to_access_level: :developer) + end + + let_it_be(:current_user) { create(:user, maintainer_projects: [project]) } + + let(:container_registry_protection_rule_attributes) do + build_stubbed(:container_registry_protection_rule, project: project) + end + + let(:mutation) do + graphql_mutation(:update_container_registry_protection_rule, input, + <<~QUERY + containerRegistryProtectionRule { + containerPathPattern + deleteProtectedUpToAccessLevel + pushProtectedUpToAccessLevel + } + clientMutationId + errors + QUERY + ) + end + + let(:input) do + { + id: container_registry_protection_rule.to_global_id, + container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-updated", + delete_protected_up_to_access_level: 'OWNER', + push_protected_up_to_access_level: 'MAINTAINER' + } + end + + let(:mutation_response) { graphql_mutation_response(:update_container_registry_protection_rule) } + + subject { post_graphql_mutation(mutation, current_user: current_user) } + + shared_examples 'a successful response' do + it { subject.tap { expect_graphql_errors_to_be_empty } } + + it 'returns the updated container registry protection rule' do + subject + + expect(mutation_response).to include( + 'containerRegistryProtectionRule' => { + 'containerPathPattern' => input[:container_path_pattern], + 'deleteProtectedUpToAccessLevel' => input[:delete_protected_up_to_access_level], + 'pushProtectedUpToAccessLevel' => input[:push_protected_up_to_access_level] + } + ) + end + + it do + subject.tap do + expect(container_registry_protection_rule.reload).to have_attributes( + container_path_pattern: input[:container_path_pattern], + push_protected_up_to_access_level: input[:push_protected_up_to_access_level].downcase + ) + end + end + end + + shared_examples 'an erroneous reponse' do + it { subject.tap { expect(mutation_response).to be_blank } } + it { expect { subject }.not_to change { container_registry_protection_rule.reload.updated_at } } + end + + it_behaves_like 'a successful response' + + context 'with other existing container registry protection rule with same container_path_pattern' do + let_it_be_with_reload(:other_existing_container_registry_protection_rule) do + create(:container_registry_protection_rule, project: project, + container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-other") + end + + let(:input) do + super().merge(container_path_pattern: other_existing_container_registry_protection_rule.container_path_pattern) + end + + it { is_expected.tap { expect_graphql_errors_to_be_empty } } + + it 'returns a blank container registry protection rule' do + is_expected.tap { expect(mutation_response['containerRegistryProtectionRule']).to be_blank } + end + + it 'includes error message in response' do + is_expected.tap { expect(mutation_response['errors']).to eq ['Container path pattern has already been taken'] } + end + end + + context 'with invalid input param `pushProtectedUpToAccessLevel`' do + let(:input) { super().merge(push_protected_up_to_access_level: nil) } + + it_behaves_like 'an erroneous reponse' + + it { is_expected.tap { expect_graphql_errors_to_include(/pushProtectedUpToAccessLevel can't be blank/) } } + end + + context 'with invalid input param `containerPathPattern`' do + let(:input) { super().merge(container_path_pattern: '') } + + it_behaves_like 'an erroneous reponse' + + it { is_expected.tap { expect_graphql_errors_to_include(/containerPathPattern can't be blank/) } } + end + + context 'when current_user does not have permission' do + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:reporter) { create(:user).tap { |u| project.add_reporter(u) } } + let_it_be(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let_it_be(:anonymous) { create(:user) } + + where(:current_user) do + [ref(:developer), ref(:reporter), ref(:guest), ref(:anonymous)] + end + + with_them do + it { is_expected.tap { expect_graphql_errors_to_include(/you don't have permission to perform this action/) } } + end + end + + context "when feature flag ':container_registry_protected_containers' disabled" do + before do + stub_feature_flags(container_registry_protected_containers: false) + end + + it_behaves_like 'an erroneous reponse' + + it 'returns error of disabled feature flag' do + is_expected.tap do + expect_graphql_errors_to_include(/'container_registry_protected_containers' feature flag is disabled/) + end + end + end +end diff --git a/spec/services/container_registry/protection/update_rule_service_spec.rb b/spec/services/container_registry/protection/update_rule_service_spec.rb new file mode 100644 index 00000000000000..0ee26557ee3956 --- /dev/null +++ b/spec/services/container_registry/protection/update_rule_service_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ContainerRegistry::Protection::UpdateRuleService, '#execute', feature_category: :container_registry do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:current_user) { create(:user, maintainer_projects: [project]) } + let_it_be_with_reload(:container_registry_protection_rule) do + create(:container_registry_protection_rule, project: project) + end + + let(:service) { described_class.new(container_registry_protection_rule, current_user: current_user, params: params) } + + let(:params) do + attributes_for( + :container_registry_protection_rule, + container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-updated", + delete_protected_up_to_access_level: 'owner', + push_protected_up_to_access_level: 'owner' + ) + end + + subject(:service_execute) { service.execute } + + shared_examples 'a successful service response' do + let(:expected_attributes) { params } + + it { is_expected.to be_success } + + it do + is_expected.to have_attributes( + errors: be_blank, + message: be_blank, + payload: { + container_registry_protection_rule: + be_a(ContainerRegistry::Protection::Rule) + .and(have_attributes(expected_attributes)) + } + ) + end + + it { expect { subject }.not_to change { ContainerRegistry::Protection::Rule.count } } + + it { subject.tap { expect(container_registry_protection_rule.reload).to have_attributes expected_attributes } } + end + + shared_examples 'an erroneous service response' do + it { is_expected.to be_error } + + it do + is_expected.to have_attributes( + errors: be_present, + message: be_present, + payload: { container_registry_protection_rule: nil } + ) + end + + it { expect { subject }.not_to change { ContainerRegistry::Protection::Rule.count } } + it { expect { subject }.not_to change { container_registry_protection_rule.reload.updated_at } } + end + + it_behaves_like 'a successful service response' + + context 'with disallowed params' do + let(:params) { super().merge!(project_id: 1, unsupported_param: 'unsupported_param_value') } + + it_behaves_like 'a successful service response' do + let(:expected_attributes) { params.except(:project_id, :unsupported_param) } + end + end + + context 'with invalid params' do + using RSpec::Parameterized::TableSyntax + + where(:params_invalid, :message_expected) do + { container_path_pattern: '' } | [/Container path pattern can't be blank/] + { delete_protected_up_to_access_level: 1000 } | /not a valid delete_protected_up_to_access_level/ + { push_protected_up_to_access_level: 1000 } | /not a valid push_protected_up_to_access_level/ + end + + with_them do + let(:params) do + super().merge(params_invalid) + end + + it_behaves_like 'an erroneous service response' + + it { is_expected.to have_attributes message: message_expected } + end + end + + context 'with empty params' do + let(:params) { {} } + + it_behaves_like 'a successful service response' do + let(:expected_attributes) { container_registry_protection_rule.attributes } + end + + it { expect { service_execute }.not_to change { container_registry_protection_rule.reload.updated_at } } + end + + context 'with nil params' do + let(:params) { nil } + + it_behaves_like 'a successful service response' do + let(:expected_attributes) { container_registry_protection_rule.attributes } + end + + it { expect { service_execute }.not_to change { container_registry_protection_rule.reload.updated_at } } + end + + context 'when updated field `container_path_pattern` is already taken' do + let_it_be_with_reload(:other_existing_container_registry_protection_rule) do + create(:container_registry_protection_rule, project: project, + container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-other") + end + + let(:params) do + { container_path_pattern: other_existing_container_registry_protection_rule.container_path_pattern } + end + + it_behaves_like 'an erroneous service response' + + it do + expect { service_execute }.not_to( + change { other_existing_container_registry_protection_rule.reload.container_path_pattern } + ) + end + + it do + is_expected.to have_attributes( + errors: match_array([/Container path pattern has already been taken/]), + message: match_array([/Container path pattern has already been taken/]) + ) + end + end + + context 'when current_user does not have permission' do + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:reporter) { create(:user).tap { |u| project.add_reporter(u) } } + let_it_be(:guest) { create(:user).tap { |u| project.add_guest(u) } } + let_it_be(:anonymous) { create(:user) } + + where(:current_user) do + [ref(:developer), ref(:reporter), ref(:guest), ref(:anonymous)] + end + + with_them do + it_behaves_like 'an erroneous service response' + + it { is_expected.to have_attributes errors: match_array(/Unauthorized/), message: /Unauthorized/ } + end + end + + context 'without container registry protection rule' do + let(:container_registry_protection_rule) { nil } + let(:params) { {} } + + it { expect { service_execute }.to raise_error(ArgumentError) } + end + + context 'without current_user' do + let(:current_user) { nil } + + it { expect { service_execute }.to raise_error(ArgumentError) } + end +end -- GitLab From 9ced5af2899d3caad30fb8c8a20066ecd3e318a3 Mon Sep 17 00:00:00 2001 From: Gerardo Navarro Date: Fri, 8 Dec 2023 17:19:10 +0000 Subject: [PATCH 2/3] docs: Apply suggestion from @eread, @marcel.amirault This commit includes: - Improvements to the description of graphql elements from the technical writing team --- .../container_registry/protection/rule/update.rb | 8 ++++---- doc/api/graphql/reference/index.md | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/graphql/mutations/container_registry/protection/rule/update.rb b/app/graphql/mutations/container_registry/protection/rule/update.rb index b8c39a0927eaa1..0ccb48019d549a 100644 --- a/app/graphql/mutations/container_registry/protection/rule/update.rb +++ b/app/graphql/mutations/container_registry/protection/rule/update.rb @@ -7,7 +7,7 @@ module Rule class Update < ::Mutations::BaseMutation graphql_name 'UpdateContainerRegistryProtectionRule' description 'Updates a container registry protection rule to restrict access to project containers. ' \ - 'You can prevent users without certain permissions from altering containers. ' \ + 'You can prevent users without certain roles from altering containers. ' \ 'Available only when feature flag `container_registry_protected_containers` is enabled.' authorize :admin_container_image @@ -22,7 +22,7 @@ class Update < ::Mutations::BaseMutation required: false, validates: { allow_blank: false }, description: - 'Container path pattern by the protection rule. For example, `my-scope/my-project/container-dev-*`. ' \ + 'Container path pattern of the protection rule. For example, `my-scope/my-project/container-dev-*`. ' \ 'Wildcard character `*` allowed.' argument :delete_protected_up_to_access_level, @@ -30,7 +30,7 @@ class Update < ::Mutations::BaseMutation required: false, validates: { allow_blank: false }, description: - 'Maximum GitLab access level unable to delete a container. ' \ + 'Maximum GitLab access level prevented from deleting a container. ' \ 'For example, `DEVELOPER`, `MAINTAINER`, `OWNER`.' argument :push_protected_up_to_access_level, @@ -38,7 +38,7 @@ class Update < ::Mutations::BaseMutation required: false, validates: { allow_blank: false }, description: - 'Maximum GitLab access level unable to push a container. ' \ + 'Maximum GitLab access level prevented from pushing a container. ' \ 'For example, `DEVELOPER`, `MAINTAINER`, `OWNER`.' field :container_registry_protection_rule, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 0112eb6f12ebff..c4e83b7e919c80 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -7466,7 +7466,7 @@ Input type: `UpdateContainerExpirationPolicyInput` ### `Mutation.updateContainerRegistryProtectionRule` -Updates a container registry protection rule to restrict access to project containers. You can prevent users without certain permissions from altering containers. Available only when feature flag `container_registry_protected_containers` is enabled. +Updates a container registry protection rule to restrict access to project containers. You can prevent users without certain roles from altering containers. Available only when feature flag `container_registry_protected_containers` is enabled. WARNING: **Introduced** in 16.7. @@ -7479,10 +7479,10 @@ Input type: `UpdateContainerRegistryProtectionRuleInput` | Name | Type | Description | | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `containerPathPattern` | [`String`](#string) | Container path pattern by the protection rule. For example, `my-scope/my-project/container-dev-*`. Wildcard character `*` allowed. | -| `deleteProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level unable to delete a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | +| `containerPathPattern` | [`String`](#string) | Container path pattern of the protection rule. For example, `my-scope/my-project/container-dev-*`. Wildcard character `*` allowed. | +| `deleteProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level prevented from deleting a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | | `id` | [`ContainerRegistryProtectionRuleID!`](#containerregistryprotectionruleid) | Global ID of the container registry protection rule to be updated. | -| `pushProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level unable to push a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | +| `pushProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level prevented from pushing a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | #### Fields -- GitLab From 00fa502887279d45ea1a1ea6eae0aa854ffa42c6 Mon Sep 17 00:00:00 2001 From: Gerardo Date: Wed, 13 Dec 2023 17:04:19 +0100 Subject: [PATCH 3/3] refactor: Adjust code after rebase This commit includes: - Applying the new attribute name `repository_path_pattern` to the new codebase --- .../protection/rule/update.rb | 5 +++-- .../protection/update_rule_service.rb | 2 +- doc/api/graphql/reference/index.md | 2 +- .../protection/rule/update_spec.rb | 22 +++++++++---------- .../protection/update_rule_service_spec.rb | 16 +++++++------- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/graphql/mutations/container_registry/protection/rule/update.rb b/app/graphql/mutations/container_registry/protection/rule/update.rb index 0ccb48019d549a..b4464e5b5f4e05 100644 --- a/app/graphql/mutations/container_registry/protection/rule/update.rb +++ b/app/graphql/mutations/container_registry/protection/rule/update.rb @@ -17,12 +17,13 @@ class Update < ::Mutations::BaseMutation required: true, description: 'Global ID of the container registry protection rule to be updated.' - argument :container_path_pattern, + argument :repository_path_pattern, GraphQL::Types::String, required: false, validates: { allow_blank: false }, description: - 'Container path pattern of the protection rule. For example, `my-scope/my-project/container-dev-*`. ' \ + 'Container\'s repository path pattern of the protection rule. ' \ + 'For example, `my-scope/my-project/container-dev-*`. ' \ 'Wildcard character `*` allowed.' argument :delete_protected_up_to_access_level, diff --git a/app/services/container_registry/protection/update_rule_service.rb b/app/services/container_registry/protection/update_rule_service.rb index e2667e442a0a9e..af74e542ac7874 100644 --- a/app/services/container_registry/protection/update_rule_service.rb +++ b/app/services/container_registry/protection/update_rule_service.rb @@ -6,7 +6,7 @@ class UpdateRuleService include Gitlab::Allowable ALLOWED_ATTRIBUTES = %i[ - container_path_pattern + repository_path_pattern delete_protected_up_to_access_level push_protected_up_to_access_level ].freeze diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index c4e83b7e919c80..853b5b317ce72a 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -7479,10 +7479,10 @@ Input type: `UpdateContainerRegistryProtectionRuleInput` | Name | Type | Description | | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `containerPathPattern` | [`String`](#string) | Container path pattern of the protection rule. For example, `my-scope/my-project/container-dev-*`. Wildcard character `*` allowed. | | `deleteProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level prevented from deleting a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | | `id` | [`ContainerRegistryProtectionRuleID!`](#containerregistryprotectionruleid) | Global ID of the container registry protection rule to be updated. | | `pushProtectedUpToAccessLevel` | [`ContainerRegistryProtectionRuleAccessLevel`](#containerregistryprotectionruleaccesslevel) | Maximum GitLab access level prevented from pushing a container. For example, `DEVELOPER`, `MAINTAINER`, `OWNER`. | +| `repositoryPathPattern` | [`String`](#string) | Container's repository path pattern of the protection rule. For example, `my-scope/my-project/container-dev-*`. Wildcard character `*` allowed. | #### Fields diff --git a/spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb b/spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb index 64eb4a142cfb48..cd2c8b9f0a264f 100644 --- a/spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb +++ b/spec/requests/api/graphql/mutations/container_registry/protection/rule/update_spec.rb @@ -20,7 +20,7 @@ graphql_mutation(:update_container_registry_protection_rule, input, <<~QUERY containerRegistryProtectionRule { - containerPathPattern + repositoryPathPattern deleteProtectedUpToAccessLevel pushProtectedUpToAccessLevel } @@ -33,7 +33,7 @@ let(:input) do { id: container_registry_protection_rule.to_global_id, - container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-updated", + repository_path_pattern: "#{container_registry_protection_rule.repository_path_pattern}-updated", delete_protected_up_to_access_level: 'OWNER', push_protected_up_to_access_level: 'MAINTAINER' } @@ -51,7 +51,7 @@ expect(mutation_response).to include( 'containerRegistryProtectionRule' => { - 'containerPathPattern' => input[:container_path_pattern], + 'repositoryPathPattern' => input[:repository_path_pattern], 'deleteProtectedUpToAccessLevel' => input[:delete_protected_up_to_access_level], 'pushProtectedUpToAccessLevel' => input[:push_protected_up_to_access_level] } @@ -61,7 +61,7 @@ it do subject.tap do expect(container_registry_protection_rule.reload).to have_attributes( - container_path_pattern: input[:container_path_pattern], + repository_path_pattern: input[:repository_path_pattern], push_protected_up_to_access_level: input[:push_protected_up_to_access_level].downcase ) end @@ -75,14 +75,14 @@ it_behaves_like 'a successful response' - context 'with other existing container registry protection rule with same container_path_pattern' do + context 'with other existing container registry protection rule with same repository_path_pattern' do let_it_be_with_reload(:other_existing_container_registry_protection_rule) do create(:container_registry_protection_rule, project: project, - container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-other") + repository_path_pattern: "#{container_registry_protection_rule.repository_path_pattern}-other") end let(:input) do - super().merge(container_path_pattern: other_existing_container_registry_protection_rule.container_path_pattern) + super().merge(repository_path_pattern: other_existing_container_registry_protection_rule.repository_path_pattern) end it { is_expected.tap { expect_graphql_errors_to_be_empty } } @@ -92,7 +92,7 @@ end it 'includes error message in response' do - is_expected.tap { expect(mutation_response['errors']).to eq ['Container path pattern has already been taken'] } + is_expected.tap { expect(mutation_response['errors']).to eq ['Repository path pattern has already been taken'] } end end @@ -104,12 +104,12 @@ it { is_expected.tap { expect_graphql_errors_to_include(/pushProtectedUpToAccessLevel can't be blank/) } } end - context 'with invalid input param `containerPathPattern`' do - let(:input) { super().merge(container_path_pattern: '') } + context 'with invalid input param `repositoryPathPattern`' do + let(:input) { super().merge(repository_path_pattern: '') } it_behaves_like 'an erroneous reponse' - it { is_expected.tap { expect_graphql_errors_to_include(/containerPathPattern can't be blank/) } } + it { is_expected.tap { expect_graphql_errors_to_include(/repositoryPathPattern can't be blank/) } } end context 'when current_user does not have permission' do diff --git a/spec/services/container_registry/protection/update_rule_service_spec.rb b/spec/services/container_registry/protection/update_rule_service_spec.rb index 0ee26557ee3956..28933b5764a52c 100644 --- a/spec/services/container_registry/protection/update_rule_service_spec.rb +++ b/spec/services/container_registry/protection/update_rule_service_spec.rb @@ -14,7 +14,7 @@ let(:params) do attributes_for( :container_registry_protection_rule, - container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-updated", + repository_path_pattern: "#{container_registry_protection_rule.repository_path_pattern}-updated", delete_protected_up_to_access_level: 'owner', push_protected_up_to_access_level: 'owner' ) @@ -73,7 +73,7 @@ using RSpec::Parameterized::TableSyntax where(:params_invalid, :message_expected) do - { container_path_pattern: '' } | [/Container path pattern can't be blank/] + { repository_path_pattern: '' } | [/Repository path pattern can't be blank/] { delete_protected_up_to_access_level: 1000 } | /not a valid delete_protected_up_to_access_level/ { push_protected_up_to_access_level: 1000 } | /not a valid push_protected_up_to_access_level/ end @@ -109,28 +109,28 @@ it { expect { service_execute }.not_to change { container_registry_protection_rule.reload.updated_at } } end - context 'when updated field `container_path_pattern` is already taken' do + context 'when updated field `repository_path_pattern` is already taken' do let_it_be_with_reload(:other_existing_container_registry_protection_rule) do create(:container_registry_protection_rule, project: project, - container_path_pattern: "#{container_registry_protection_rule.container_path_pattern}-other") + repository_path_pattern: "#{container_registry_protection_rule.repository_path_pattern}-other") end let(:params) do - { container_path_pattern: other_existing_container_registry_protection_rule.container_path_pattern } + { repository_path_pattern: other_existing_container_registry_protection_rule.repository_path_pattern } end it_behaves_like 'an erroneous service response' it do expect { service_execute }.not_to( - change { other_existing_container_registry_protection_rule.reload.container_path_pattern } + change { other_existing_container_registry_protection_rule.reload.repository_path_pattern } ) end it do is_expected.to have_attributes( - errors: match_array([/Container path pattern has already been taken/]), - message: match_array([/Container path pattern has already been taken/]) + errors: match_array([/Repository path pattern has already been taken/]), + message: match_array([/Repository path pattern has already been taken/]) ) end end -- GitLab