Commit 2f884c4b authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch 'cleanup-evalute_protected_tag_for_release_permissions-feature-flag' into 'master'

Clean up evalute_protected_tag_for_release_permissions feature flag

See merge request gitlab-org/gitlab!66713
parents 1d632a1e dde58c83
...@@ -159,10 +159,6 @@ class ProjectPolicy < BasePolicy ...@@ -159,10 +159,6 @@ class ProjectPolicy < BasePolicy
::Feature.enabled?(:build_service_proxy, @subject) ::Feature.enabled?(:build_service_proxy, @subject)
end end
condition(:respect_protected_tag_for_release_permissions) do
::Feature.enabled?(:evalute_protected_tag_for_release_permissions, @subject, default_enabled: :yaml)
end
condition(:user_defined_variables_allowed) do condition(:user_defined_variables_allowed) do
!@subject.restrict_user_defined_variables? !@subject.restrict_user_defined_variables?
end end
...@@ -374,6 +370,7 @@ class ProjectPolicy < BasePolicy ...@@ -374,6 +370,7 @@ class ProjectPolicy < BasePolicy
enable :update_deployment enable :update_deployment
enable :create_release enable :create_release
enable :update_release enable :update_release
enable :destroy_release
enable :create_metrics_dashboard_annotation enable :create_metrics_dashboard_annotation
enable :delete_metrics_dashboard_annotation enable :delete_metrics_dashboard_annotation
enable :update_metrics_dashboard_annotation enable :update_metrics_dashboard_annotation
...@@ -657,10 +654,6 @@ class ProjectPolicy < BasePolicy ...@@ -657,10 +654,6 @@ class ProjectPolicy < BasePolicy
rule { build_service_proxy_enabled }.enable :build_service_proxy_enabled rule { build_service_proxy_enabled }.enable :build_service_proxy_enabled
rule { respect_protected_tag_for_release_permissions & can?(:developer_access) }.policy do
enable :destroy_release
end
rule { can?(:download_code) }.policy do rule { can?(:download_code) }.policy do
enable :read_repository_graphs enable :read_repository_graphs
end end
......
...@@ -9,11 +9,7 @@ class ReleasePolicy < BasePolicy ...@@ -9,11 +9,7 @@ class ReleasePolicy < BasePolicy
!access.can_create_tag?(@subject.tag) !access.can_create_tag?(@subject.tag)
end end
condition(:respect_protected_tag) do rule { protected_tag }.policy do
::Feature.enabled?(:evalute_protected_tag_for_release_permissions, @subject.project, default_enabled: :yaml)
end
rule { respect_protected_tag & protected_tag }.policy do
prevent :create_release prevent :create_release
prevent :update_release prevent :update_release
prevent :destroy_release prevent :destroy_release
......
...@@ -83,15 +83,6 @@ module Releases ...@@ -83,15 +83,6 @@ module Releases
release.execute_hooks(action) release.execute_hooks(action)
end end
def track_protected_tag_access_error!
unless ::Gitlab::UserAccess.new(current_user, container: project).can_create_tag?(tag_name)
Gitlab::ErrorTracking.log_exception(
ReleaseProtectedTagAccessError.new,
project_id: project.id,
user_id: current_user.id)
end
end
# overridden in EE # overridden in EE
def project_group_id; end def project_group_id; end
end end
......
...@@ -7,8 +7,6 @@ module Releases ...@@ -7,8 +7,6 @@ module Releases
return error('Release already exists', 409) if release return error('Release already exists', 409) if release
return error("Milestone(s) not found: #{inexistent_milestones.join(', ')}", 400) if inexistent_milestones.any? return error("Milestone(s) not found: #{inexistent_milestones.join(', ')}", 400) if inexistent_milestones.any?
track_protected_tag_access_error!
# should be found before the creation of new tag # should be found before the creation of new tag
# because tag creation can spawn new pipeline # because tag creation can spawn new pipeline
# which won't have any data for evidence yet # which won't have any data for evidence yet
...@@ -48,8 +46,6 @@ module Releases ...@@ -48,8 +46,6 @@ module Releases
end end
def can_create_tag? def can_create_tag?
return true unless ::Feature.enabled?(:evalute_protected_tag_for_release_permissions, project, default_enabled: :yaml)
::Gitlab::UserAccess.new(current_user, container: project).can_create_tag?(tag_name) ::Gitlab::UserAccess.new(current_user, container: project).can_create_tag?(tag_name)
end end
......
...@@ -6,8 +6,6 @@ module Releases ...@@ -6,8 +6,6 @@ module Releases
return error('Release does not exist', 404) unless release return error('Release does not exist', 404) unless release
return error('Access Denied', 403) unless allowed? return error('Access Denied', 403) unless allowed?
track_protected_tag_access_error!
if release.destroy if release.destroy
success(tag: existing_tag, release: release) success(tag: existing_tag, release: release)
else else
......
...@@ -7,8 +7,6 @@ module Releases ...@@ -7,8 +7,6 @@ module Releases
return error return error
end end
track_protected_tag_access_error!
if param_for_milestone_titles_provided? if param_for_milestone_titles_provided?
previous_milestones = release.milestones.map(&:title) previous_milestones = release.milestones.map(&:title)
params[:milestones] = milestones params[:milestones] = milestones
......
---
name: evalute_protected_tag_for_release_permissions
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64693
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334368
milestone: '14.1'
type: development
group: group::release
default_enabled: true
...@@ -589,25 +589,6 @@ As an example of release permission control, you can allow only ...@@ -589,25 +589,6 @@ As an example of release permission control, you can allow only
to create, update, and delete releases by protecting the tag with a wildcard (`*`), to create, update, and delete releases by protecting the tag with a wildcard (`*`),
and set **Maintainer** in the **Allowed to create** column. and set **Maintainer** in the **Allowed to create** column.
#### Enable or disable protected tag evaluation on releases **(FREE SELF)**
Protected tag evaluation on release permissions is under development but ready for production use.
It is deployed behind a feature flag that is **enabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
can opt to disable it.
To enable it:
```ruby
Feature.enable(:evalute_protected_tag_for_release_permissions)
```
To disable it:
```ruby
Feature.disable(:evalute_protected_tag_for_release_permissions)
```
## Release Command Line ## Release Command Line
> [Introduced](https://gitlab.com/gitlab-org/release-cli/-/merge_requests/6) in GitLab 12.10. > [Introduced](https://gitlab.com/gitlab-org/release-cli/-/merge_requests/6) in GitLab 12.10.
......
...@@ -17,29 +17,6 @@ RSpec.describe ReleasePolicy, :request_store do ...@@ -17,29 +17,6 @@ RSpec.describe ReleasePolicy, :request_store do
subject { described_class.new(user, release) } subject { described_class.new(user, release) }
context 'when the evalute_protected_tag_for_release_permissions feature flag is disabled' do
before do
stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
end
it 'allows the user to create and update a release' do
is_expected.to be_allowed(:create_release)
is_expected.to be_allowed(:update_release)
end
it 'prevents the user from destroying a release' do
is_expected.to be_disallowed(:destroy_release)
end
context 'when the user is maintainer' do
let(:user) { maintainer }
it 'allows the user to destroy a release' do
is_expected.to be_allowed(:destroy_release)
end
end
end
context 'when the user has access to the protected tag' do context 'when the user has access to the protected tag' do
let_it_be(:protected_tag) { create(:protected_tag, :developers_can_create, name: release.tag, project: project) } let_it_be(:protected_tag) { create(:protected_tag, :developers_can_create, name: release.tag, project: project) }
......
...@@ -44,21 +44,6 @@ RSpec.describe Releases::CreateService do ...@@ -44,21 +44,6 @@ RSpec.describe Releases::CreateService do
it_behaves_like 'a successful release creation' it_behaves_like 'a successful release creation'
context 'when tag is protected and user does not have access to it' do
let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
it 'track the error event' do
stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
kind_of(described_class::ReleaseProtectedTagAccessError),
project_id: project.id,
user_id: user.id)
service.execute
end
end
context 'when the tag does not exist' do context 'when the tag does not exist' do
let(:tag_name) { 'non-exist-tag' } let(:tag_name) { 'non-exist-tag' }
......
...@@ -28,21 +28,6 @@ RSpec.describe Releases::DestroyService do ...@@ -28,21 +28,6 @@ RSpec.describe Releases::DestroyService do
it 'returns the destroyed object' do it 'returns the destroyed object' do
is_expected.to include(status: :success, release: release) is_expected.to include(status: :success, release: release)
end end
context 'when tag is protected and user does not have access to it' do
let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
it 'track the error event' do
stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
kind_of(described_class::ReleaseProtectedTagAccessError),
project_id: project.id,
user_id: user.id)
service.execute
end
end
end end
context 'when tag does not exist in the repository' do context 'when tag does not exist in the repository' do
......
...@@ -38,21 +38,6 @@ RSpec.describe Releases::UpdateService do ...@@ -38,21 +38,6 @@ RSpec.describe Releases::UpdateService do
service.execute service.execute
end end
context 'when tag is protected and user does not have access to it' do
let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
it 'track the error event' do
stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
kind_of(described_class::ReleaseProtectedTagAccessError),
project_id: project.id,
user_id: user.id)
service.execute
end
end
context 'when the tag does not exists' do context 'when the tag does not exists' do
let(:tag_name) { 'foobar' } let(:tag_name) { 'foobar' }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment