diff options
| -rw-r--r-- | app/services/auth/container_registry_authentication_service.rb | 10 | ||||
| -rw-r--r-- | changelogs/unreleased/40096-allow-ci-token-to-delete-from-registry.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/auth.rb | 3 | ||||
| -rw-r--r-- | lib/gitlab/correlation_id.rb | 40 | ||||
| -rw-r--r-- | lib/gitlab/tracing.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/auth_spec.rb | 3 | ||||
| -rw-r--r-- | spec/lib/gitlab/tracing_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/auth/container_registry_authentication_service_spec.rb | 12 |
8 files changed, 26 insertions, 51 deletions
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 0a069320936..9e7319c1d9b 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -124,13 +124,21 @@ module Auth build_can_pull?(requested_project) || user_can_pull?(requested_project) || deploy_token_can_pull?(requested_project) when 'push' build_can_push?(requested_project) || user_can_push?(requested_project) - when '*', 'delete' + when 'delete' + build_can_delete?(requested_project) || user_can_admin?(requested_project) + when '*' user_can_admin?(requested_project) else false end end + def build_can_delete?(requested_project) + # Build can delete only from the project from which it originates + has_authentication_ability?(:build_destroy_container_image) && + requested_project == project + end + def registry Gitlab.config.registry end diff --git a/changelogs/unreleased/40096-allow-ci-token-to-delete-from-registry.yml b/changelogs/unreleased/40096-allow-ci-token-to-delete-from-registry.yml new file mode 100644 index 00000000000..3e5de08f6ae --- /dev/null +++ b/changelogs/unreleased/40096-allow-ci-token-to-delete-from-registry.yml @@ -0,0 +1,5 @@ +--- +title: Allow $CI_REGISTRY_USER to delete tags +merge_request: 31796 +author: +type: added diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 6769bd95c2b..bdc46abeb9f 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -265,7 +265,8 @@ module Gitlab :read_project, :build_download_code, :build_read_container_image, - :build_create_container_image + :build_create_container_image, + :build_destroy_container_image ] end diff --git a/lib/gitlab/correlation_id.rb b/lib/gitlab/correlation_id.rb deleted file mode 100644 index 0f9bde4390e..00000000000 --- a/lib/gitlab/correlation_id.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module CorrelationId - LOG_KEY = 'correlation_id'.freeze - - class << self - def use_id(correlation_id, &blk) - # always generate a id if null is passed - correlation_id ||= new_id - - ids.push(correlation_id || new_id) - - begin - yield(current_id) - ensure - ids.pop - end - end - - def current_id - ids.last - end - - def current_or_new_id - current_id || new_id - end - - private - - def ids - Thread.current[:correlation_id] ||= [] - end - - def new_id - SecureRandom.uuid - end - end - end -end diff --git a/lib/gitlab/tracing.rb b/lib/gitlab/tracing.rb index 29517591c51..7732d7c9d9c 100644 --- a/lib/gitlab/tracing.rb +++ b/lib/gitlab/tracing.rb @@ -30,7 +30,7 @@ module Gitlab # Avoid using `format` since it can throw TypeErrors # which we want to avoid on unsanitised env var input tracing_url_template.to_s - .gsub(/\{\{\s*correlation_id\s*\}\}/, Gitlab::CorrelationId.current_id.to_s) + .gsub(/\{\{\s*correlation_id\s*\}\}/, Labkit::Correlation::CorrelationId.current_id.to_s) .gsub(/\{\{\s*service\s*\}\}/, Gitlab.process_name) end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 098c33f9cb1..0365d63ea9c 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -587,7 +587,8 @@ describe Gitlab::Auth do :read_project, :build_download_code, :build_read_container_image, - :build_create_container_image + :build_create_container_image, + :build_destroy_container_image ] end diff --git a/spec/lib/gitlab/tracing_spec.rb b/spec/lib/gitlab/tracing_spec.rb index db75ce2a998..e913bb600ec 100644 --- a/spec/lib/gitlab/tracing_spec.rb +++ b/spec/lib/gitlab/tracing_spec.rb @@ -59,7 +59,7 @@ describe Gitlab::Tracing do it 'returns the correct state for .tracing_url' do expect(described_class).to receive(:tracing_url_enabled?).and_return(tracing_url_enabled?) allow(described_class).to receive(:tracing_url_template).and_return(tracing_url_template) - allow(Gitlab::CorrelationId).to receive(:current_id).and_return(correlation_id) + allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return(correlation_id) allow(Gitlab).to receive(:process_name).and_return(process_name) expect(described_class.tracing_url).to eq(tracing_url) diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 3ca389ba25b..2807b8c8c85 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -476,7 +476,7 @@ describe Auth::ContainerRegistryAuthenticationService do let(:current_user) { create(:user) } let(:authentication_abilities) do - [:build_read_container_image, :build_create_container_image] + [:build_read_container_image, :build_create_container_image, :build_destroy_container_image] end before do @@ -507,19 +507,19 @@ describe Auth::ContainerRegistryAuthenticationService do end end - context 'disallow to delete images' do + context 'allow to delete images since registry 2.7' do let(:current_params) do - { scopes: ["repository:#{current_project.full_path}:*"] } + { scopes: ["repository:#{current_project.full_path}:delete"] } end - it_behaves_like 'an inaccessible' do + it_behaves_like 'a deletable since registry 2.7' do let(:project) { current_project } end end - context 'disallow to delete images since registry 2.7' do + context 'disallow to delete images' do let(:current_params) do - { scopes: ["repository:#{current_project.full_path}:delete"] } + { scopes: ["repository:#{current_project.full_path}:*"] } end it_behaves_like 'an inaccessible' do |