From ac18a4c33fff194166c57cb981b6ac2479ad1dd6 Mon Sep 17 00:00:00 2001 From: Alishan Ladhani <aladhani@gitlab.com> Date: Fri, 5 Aug 2022 14:32:00 -0400 Subject: [PATCH] Add feature toggle for Releases (backend) --- app/controllers/projects_controller.rb | 2 +- app/helpers/projects_helper.rb | 3 +- .../project_features_compatibility.rb | 4 ++ app/models/project.rb | 1 + app/models/project_feature.rb | 1 + app/policies/project_policy.rb | 5 ++ .../import_export/project/import_export.yml | 2 + rubocop/cop/gitlab/feature_available_usage.rb | 1 + spec/controllers/projects_controller_spec.rb | 1 + spec/factories/projects.rb | 1 + spec/helpers/projects_helper_spec.rb | 3 +- .../import_export/safe_model_attributes.yml | 1 + .../project_features_compatibility_spec.rb | 4 +- spec/models/project_spec.rb | 1 + spec/policies/project_policy_spec.rb | 68 +++++++++++++++++++ 15 files changed, 94 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index a93911595417..55e4b37c34c2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -428,7 +428,7 @@ def project_feature_attributes def operations_feature_attributes if Feature.enabled?(:split_operations_visibility_permissions, project) %i[ - environments_access_level feature_flags_access_level + environments_access_level feature_flags_access_level releases_access_level ] else %i[operations_access_level] diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index b257d9896a2e..dfc270adf8b6 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -641,7 +641,8 @@ def project_permissions_settings(project) securityAndComplianceAccessLevel: project.security_and_compliance_access_level, containerRegistryAccessLevel: feature.container_registry_access_level, environmentsAccessLevel: feature.environments_access_level, - featureFlagsAccessLevel: feature.feature_flags_access_level + featureFlagsAccessLevel: feature.feature_flags_access_level, + releasesAccessLevel: feature.releases_access_level } end diff --git a/app/models/concerns/project_features_compatibility.rb b/app/models/concerns/project_features_compatibility.rb index 8f87e8b3f224..7613691bc2e4 100644 --- a/app/models/concerns/project_features_compatibility.rb +++ b/app/models/concerns/project_features_compatibility.rb @@ -102,6 +102,10 @@ def feature_flags_access_level=(value) write_feature_attribute_string(:feature_flags_access_level, value) end + def releases_access_level=(value) + write_feature_attribute_string(:releases_access_level, value) + end + # TODO: Remove this method after we drop support for project create/edit APIs to set the # container_registry_enabled attribute. They can instead set the container_registry_access_level # attribute. diff --git a/app/models/project.rb b/app/models/project.rb index 16b3e1c09c51..efb8e8c03e6f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -453,6 +453,7 @@ def self.integration_association_name(name) :metrics_dashboard_access_level, :analytics_access_level, :operations_access_level, :security_and_compliance_access_level, :container_registry_access_level, :environments_access_level, :feature_flags_access_level, + :releases_access_level, to: :project_feature, allow_nil: true delegate :show_default_award_emojis, :show_default_award_emojis=, diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 82d98496b6b8..8623e477c06e 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -23,6 +23,7 @@ class ProjectFeature < ApplicationRecord package_registry environments feature_flags + releases ].freeze EXPORTABLE_FEATURES = (FEATURES - [:security_and_compliance, :pages]).freeze diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index c2bea7436eb1..f4f7275a78a3 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -211,6 +211,7 @@ class ProjectPolicy < BasePolicy security_and_compliance environments feature_flags + releases ] features.each do |f| @@ -396,6 +397,10 @@ class ProjectPolicy < BasePolicy prevent(:admin_feature_flags_client) end + rule { split_operations_visibility_permissions & releases_disabled }.policy do + prevent(*create_read_update_admin_destroy(:release)) + end + rule { can?(:metrics_dashboard) }.policy do enable :read_prometheus enable :read_deployment diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index b6c3da8e564c..e6212132e963 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -287,6 +287,7 @@ included_attributes: - :package_registry_access_level - :environments_access_level - :feature_flags_access_level + - :releases_access_level prometheus_metrics: - :created_at - :updated_at @@ -693,6 +694,7 @@ included_attributes: - :package_registry_access_level - :environments_access_level - :feature_flags_access_level + - :releases_access_level - :allow_merge_on_skipped_pipeline - :auto_devops_deploy_strategy - :auto_devops_enabled diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb index a153d3f7b2f5..f748b7d9111f 100644 --- a/rubocop/cop/gitlab/feature_available_usage.rb +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -25,6 +25,7 @@ class FeatureAvailableUsage < RuboCop::Cop::Cop container_registry environments feature_flags + releases ].freeze EE_FEATURES = %i[requirements].freeze ALL_FEATURES = (FEATURES + EE_FEATURES).freeze diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 388d7c2266b4..94d75ab8d7d9 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -919,6 +919,7 @@ def update_project_feature container_registry_access_level environments_access_level feature_flags_access_level + releases_access_level ] end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 98dfecb2888e..95b72648cf54 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -39,6 +39,7 @@ security_and_compliance_access_level { ProjectFeature::PRIVATE } environments_access_level { ProjectFeature::ENABLED } feature_flags_access_level { ProjectFeature::ENABLED } + releases_access_level { ProjectFeature::ENABLED } # we can't assign the delegated `#ci_cd_settings` attributes directly, as the # `#ci_cd_settings` relation needs to be created first diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 214086a38232..04c066986b72 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -968,7 +968,8 @@ def license_name securityAndComplianceAccessLevel: project.security_and_compliance_access_level, containerRegistryAccessLevel: project.project_feature.container_registry_access_level, environmentsAccessLevel: project.project_feature.environments_access_level, - featureFlagsAccessLevel: project.project_feature.feature_flags_access_level + featureFlagsAccessLevel: project.project_feature.feature_flags_access_level, + releasesAccessLevel: project.project_feature.releases_access_level ) end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 72e13943bf5f..5cbe05ccf535 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -585,6 +585,7 @@ ProjectFeature: - package_registry_access_level - environments_access_level - feature_flags_access_level +- releases_access_level - created_at - updated_at ProtectedBranch::MergeAccessLevel: diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index 48c391451edf..b49b9ce8a2a0 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -6,7 +6,9 @@ let(:project) { create(:project) } let(:features_enabled) { %w(issues wiki builds merge_requests snippets security_and_compliance) } let(:features) do - features_enabled + %w(repository pages operations container_registry package_registry environments feature_flags) + features_enabled + %w( + repository pages operations container_registry package_registry environments feature_flags releases + ) end # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e2911f2201ec..d69a358d4f9d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -835,6 +835,7 @@ it { is_expected.to delegate_method(:container_registry_access_level).to(:project_feature) } it { is_expected.to delegate_method(:environments_access_level).to(:project_feature) } it { is_expected.to delegate_method(:feature_flags_access_level).to(:project_feature) } + it { is_expected.to delegate_method(:releases_access_level).to(:project_feature) } describe 'read project settings' do %i( diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index c3817f671585..e8fdf9a8e256 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -2171,6 +2171,74 @@ def permissions_abilities(role) end end + describe 'Releases feature' do + using RSpec::Parameterized::TableSyntax + + let(:guest_permissions) { [:read_release] } + + let(:developer_permissions) do + guest_permissions + [:create_release, :update_release, :destroy_release] + end + + let(:maintainer_permissions) do + developer_permissions + end + + where(:project_visibility, :access_level, :role, :allowed) do + :public | ProjectFeature::ENABLED | :maintainer | true + :public | ProjectFeature::ENABLED | :developer | true + :public | ProjectFeature::ENABLED | :guest | true + :public | ProjectFeature::ENABLED | :anonymous | true + :public | ProjectFeature::PRIVATE | :maintainer | true + :public | ProjectFeature::PRIVATE | :developer | true + :public | ProjectFeature::PRIVATE | :guest | true + :public | ProjectFeature::PRIVATE | :anonymous | false + :public | ProjectFeature::DISABLED | :maintainer | false + :public | ProjectFeature::DISABLED | :developer | false + :public | ProjectFeature::DISABLED | :guest | false + :public | ProjectFeature::DISABLED | :anonymous | false + :internal | ProjectFeature::ENABLED | :maintainer | true + :internal | ProjectFeature::ENABLED | :developer | true + :internal | ProjectFeature::ENABLED | :guest | true + :internal | ProjectFeature::ENABLED | :anonymous | false + :internal | ProjectFeature::PRIVATE | :maintainer | true + :internal | ProjectFeature::PRIVATE | :developer | true + :internal | ProjectFeature::PRIVATE | :guest | true + :internal | ProjectFeature::PRIVATE | :anonymous | false + :internal | ProjectFeature::DISABLED | :maintainer | false + :internal | ProjectFeature::DISABLED | :developer | false + :internal | ProjectFeature::DISABLED | :guest | false + :internal | ProjectFeature::DISABLED | :anonymous | false + :private | ProjectFeature::ENABLED | :maintainer | true + :private | ProjectFeature::ENABLED | :developer | true + :private | ProjectFeature::ENABLED | :guest | true + :private | ProjectFeature::ENABLED | :anonymous | false + :private | ProjectFeature::PRIVATE | :maintainer | true + :private | ProjectFeature::PRIVATE | :developer | true + :private | ProjectFeature::PRIVATE | :guest | true + :private | ProjectFeature::PRIVATE | :anonymous | false + :private | ProjectFeature::DISABLED | :maintainer | false + :private | ProjectFeature::DISABLED | :developer | false + :private | ProjectFeature::DISABLED | :guest | false + :private | ProjectFeature::DISABLED | :anonymous | false + end + + with_them do + let(:current_user) { user_subject(role) } + let(:project) { project_subject(project_visibility) } + + it 'allows/disallows the abilities based on the Releases access level' do + project.project_feature.update!(releases_access_level: access_level) + + if allowed + expect_allowed(*permissions_abilities(role)) + else + expect_disallowed(*permissions_abilities(role)) + end + end + end + end + describe 'access_security_and_compliance' do context 'when the "Security & Compliance" is enabled' do before do -- GitLab