diff --git a/app/models/concerns/cascading_project_setting_attribute.rb b/app/models/concerns/cascading_project_setting_attribute.rb new file mode 100644 index 0000000000000000000000000000000000000000..9dde6e034e7fbf71bac18988b409b4e917bfdd06 --- /dev/null +++ b/app/models/concerns/cascading_project_setting_attribute.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +module CascadingProjectSettingAttribute + extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize + + class_methods do + private + + # logic based on the cascading setting logic + # CascadingNamespaceSettingAttribute + # + # Code remove for this module: + # - logic related to 'lock_#{attribute}', because projects don't need to lock attributes. + # - logic related to descendants, because projects don't have descendants. + # - logic related to a `nil` value for the setting, because the first/only + # cascading project setting (`duo_features_enabled`) has a db-level not nil constraint. + def cascading_attr(*attributes) + attributes.map(&:to_sym).each do |attribute| + # public methods + define_attr_reader(attribute) + define_attr_writer(attribute) + define_lock_methods(attribute) + + # private methods + define_validator_methods(attribute) + define_attr_before_save(attribute) + + validate :"#{attribute}_changeable?" + + before_save :"before_save_#{attribute}", if: -> { will_save_change_to_attribute?(attribute) } + end + end + + def define_attr_reader(attribute) + define_method(attribute) do + strong_memoize(attribute) do + next self[attribute] if will_save_change_to_attribute?(attribute) + next locked_value(attribute) if cascading_attribute_locked?(attribute) + next self[attribute] unless self[attribute].nil? + + cascaded_value = cascaded_ancestor_value(attribute) + next cascaded_value unless cascaded_value.nil? + + application_setting_value(attribute) + end + end + end + + def define_attr_writer(attribute) + define_method("#{attribute}=") do |value| + return value if read_attribute(attribute).nil? && to_bool(value) == cascaded_ancestor_value(attribute) + + clear_memoization(attribute) + super(value) + end + end + + def define_attr_before_save(attribute) + # rubocop:disable GitlabSecurity/PublicSend -- model attribute, not user input + define_method("before_save_#{attribute}") do + new_value = public_send(attribute) + return unless public_send("#{attribute}_was").nil? && new_value == cascaded_ancestor_value(attribute) + + write_attribute(attribute, nil) + end + # rubocop:enable GitlabSecurity/PublicSend + + private :"before_save_#{attribute}" + end + + def define_lock_methods(attribute) + define_method("#{attribute}_locked?") do + cascading_attribute_locked?(attribute) + end + + define_method("#{attribute}_locked_by_ancestor?") do + locked_by_ancestor?(attribute) + end + + define_method("#{attribute}_locked_by_application_setting?") do + locked_by_application_setting?(attribute) + end + + define_method("#{attribute}_locked_ancestor") do + locked_ancestor(attribute) + end + end + + def define_validator_methods(attribute) + define_method("#{attribute}_changeable?") do + return unless cascading_attribute_changed?(attribute) + return unless cascading_attribute_locked?(attribute) + + errors.add(attribute, s_('CascadingSettings|cannot be changed because it is locked by an ancestor')) + end + + private :"#{attribute}_changeable?" + end + end + + private + + def locked_value(attribute) + return application_setting_value(attribute) if locked_by_application_setting?(attribute) + + ancestor = locked_ancestor(attribute) + ancestor.read_attribute(attribute) if ancestor + end + + def locked_ancestor(attribute) + return unless direct_ancestor_present? + + strong_memoize(:"#{attribute}_locked_ancestor") do + NamespaceSetting + .select(:namespace_id, "lock_#{attribute}", attribute) + .where(namespace_id: namespace_ancestor_ids) + .where(NamespaceSetting.arel_table["lock_#{attribute}"].eq(true)) + .limit(1).load.first + end + end + + def locked_by_ancestor?(attribute) + locked_ancestor(attribute).present? + end + + def locked_by_application_setting?(attribute) + Gitlab::CurrentSettings.public_send("lock_#{attribute}") # rubocop:disable GitlabSecurity/PublicSend -- model attribute, not user input + end + + def cascading_attribute_locked?(attribute) + locked_by_ancestor?(attribute) || locked_by_application_setting?(attribute) + end + + def cascading_attribute_changed?(attribute) + public_send("#{attribute}_changed?") # rubocop:disable GitlabSecurity/PublicSend -- model attribute, not user input + end + + def cascaded_ancestor_value(attribute) + return unless direct_ancestor_present? + + # rubocop:disable GitlabSecurity/SqlInjection -- model attribute, not user input + NamespaceSetting + .select(attribute) + .joins( + "join unnest(ARRAY[#{namespace_ancestor_ids_joined}]) with ordinality t(namespace_id, ord) USING (namespace_id)" + ) + .where("#{attribute} IS NOT NULL") + .order('t.ord') + .limit(1).first&.read_attribute(attribute) + # rubocop:enable GitlabSecurity/SqlInjection + end + + def application_setting_value(attribute) + Gitlab::CurrentSettings.public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend -- model attribute, not user input + end + + def direct_ancestor_present? + project.group.present? + end + + def namespace_ancestor_ids_joined + namespace_ancestor_ids.join(',') + end + + def namespace_ancestor_ids + project.project_namespace.ancestor_ids(hierarchy_order: :asc) + end + strong_memoize_attr :namespace_ancestor_ids + + def to_bool(value) + ActiveModel::Type::Boolean.new.cast(value) + end +end diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index 5d4a0f8e08a810954651b88428eaa0e8102a2112..7473a0ac6b26479679d8fc8e2b4571ff242b9258 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -4,6 +4,7 @@ class ProjectSetting < ApplicationRecord include ::Gitlab::Utils::StrongMemoize include EachBatch include IgnorableColumns + include CascadingProjectSettingAttribute ALLOWED_TARGET_PLATFORMS = %w[ios osx tvos watchos android].freeze diff --git a/db/migrate/20240213223630_add_duo_features_enabled_cascading_setting.rb b/db/migrate/20240213223630_add_duo_features_enabled_cascading_setting.rb new file mode 100644 index 0000000000000000000000000000000000000000..03001fd1d9341b8f3c6cf6f6e8cbc67693122e46 --- /dev/null +++ b/db/migrate/20240213223630_add_duo_features_enabled_cascading_setting.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddDuoFeaturesEnabledCascadingSetting < Gitlab::Database::Migration[2.2] + include Gitlab::Database::MigrationHelpers::CascadingNamespaceSettings + enable_lock_retries! + + milestone '16.10' + + def up + add_cascading_namespace_setting :duo_features_enabled, :boolean, default: true, null: false + end + + def down + remove_cascading_namespace_setting :duo_features_enabled + end +end diff --git a/db/schema_migrations/20240213223630 b/db/schema_migrations/20240213223630 new file mode 100644 index 0000000000000000000000000000000000000000..341ca1f6f4163db3d9168a09140a3b6b5422a15f --- /dev/null +++ b/db/schema_migrations/20240213223630 @@ -0,0 +1 @@ +b6e84d82dcc591adbdb7b08f79d6a7fd0da741e34fdb9205d26a8903e43868d0 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0ea5d4f940b970d66fff9c1e5f1f5b6bdc10de55..d538b795571533e61fbf36d5ab6583a16fa4f80c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4084,6 +4084,8 @@ CREATE TABLE application_settings ( encrypted_arkose_labs_client_xid_iv bytea, encrypted_arkose_labs_client_secret bytea, encrypted_arkose_labs_client_secret_iv bytea, + duo_features_enabled boolean DEFAULT true NOT NULL, + lock_duo_features_enabled boolean DEFAULT false NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_container_registry_pre_import_tags_rate_positive CHECK ((container_registry_pre_import_tags_rate >= (0)::numeric)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), @@ -11602,6 +11604,8 @@ CREATE TABLE namespace_settings ( lock_toggle_security_policies_policy_scope boolean DEFAULT false NOT NULL, math_rendering_limits_enabled boolean, lock_math_rendering_limits_enabled boolean DEFAULT false NOT NULL, + duo_features_enabled boolean, + lock_duo_features_enabled boolean DEFAULT false NOT NULL, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT namespace_settings_unique_project_download_limit_alertlist_size CHECK ((cardinality(unique_project_download_limit_alertlist) <= 100)), CONSTRAINT namespace_settings_unique_project_download_limit_allowlist_size CHECK ((cardinality(unique_project_download_limit_allowlist) <= 100)) diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb index e7b4227738f55ca16e3b2c03eb27ca5ea69af3c8..23b9b36d329a30eb2f6b91f8760a44cf3dff96e4 100644 --- a/ee/app/models/ee/namespace_setting.rb +++ b/ee/app/models/ee/namespace_setting.rb @@ -5,6 +5,8 @@ module NamespaceSetting extend ActiveSupport::Concern prepended do + cascading_attr :duo_features_enabled + validates :unique_project_download_limit, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 10_000 }, presence: true diff --git a/ee/app/models/ee/project_setting.rb b/ee/app/models/ee/project_setting.rb index 7651668561ac72a57de975815ce6996db49a5984..f6d6db332db23c385f3a1e605e337e4a19770671 100644 --- a/ee/app/models/ee/project_setting.rb +++ b/ee/app/models/ee/project_setting.rb @@ -5,6 +5,8 @@ module ProjectSetting extend ActiveSupport::Concern prepended do + cascading_attr :duo_features_enabled + belongs_to :push_rule scope :has_vulnerabilities, -> { where('has_vulnerabilities IS TRUE') } diff --git a/ee/spec/models/ee/project_setting_spec.rb b/ee/spec/models/ee/project_setting_spec.rb index 5fa6dd861ab283548ff437bbc78029b28db95cc0..16f3dbc877642adfedbcdf20f6ba3a725662c6b8 100644 --- a/ee/spec/models/ee/project_setting_spec.rb +++ b/ee/spec/models/ee/project_setting_spec.rb @@ -59,4 +59,8 @@ end end end + + describe '#duo_features_enabled' do + it_behaves_like 'a cascading project setting boolean attribute', settings_attribute_name: :duo_features_enabled + end end diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb index cd9770a1e251af8640a96cde79d7c16bf5d18674..6eeb062eb57a9b68c4c1a988ed67b26eabe3d7b4 100644 --- a/ee/spec/models/namespace_setting_spec.rb +++ b/ee/spec/models/namespace_setting_spec.rb @@ -534,4 +534,8 @@ it { is_expected.to eq result } end end + + describe '#duo_features_enabled' do + it_behaves_like 'a cascading namespace setting boolean attribute', settings_attribute_name: :duo_features_enabled + end end diff --git a/spec/requests/api/graphql/projects/projects_spec.rb b/spec/requests/api/graphql/projects/projects_spec.rb index dfebcb7c42ce0b423e251d778d700a6f38a6aa10..584f668ae862222af15932be04028064091478c7 100644 --- a/spec/requests/api/graphql/projects/projects_spec.rb +++ b/spec/requests/api/graphql/projects/projects_spec.rb @@ -50,9 +50,11 @@ end # There is an N+1 query for max_member_access_for_user_ids + # There is an N+1 query for duo_features_enabled cascading setting + # https://gitlab.com/gitlab-org/gitlab/-/issues/442164 expect do post_graphql(query, current_user: current_user) - end.not_to exceed_all_query_limit(control).with_threshold(5) + end.not_to exceed_all_query_limit(control).with_threshold(17) end it 'returns the expected projects' do diff --git a/spec/support/shared_examples/models/concerns/cascading_project_setting_shared_examples.rb b/spec/support/shared_examples/models/concerns/cascading_project_setting_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..e5ab07942b220567e55a5e1e2d80f19f23713c75 --- /dev/null +++ b/spec/support/shared_examples/models/concerns/cascading_project_setting_shared_examples.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'a cascading project setting boolean attribute' do + |settings_attribute_name:, settings_association: :project_setting| + let_it_be_with_reload(:parent_group) { create(:group) } + let_it_be_with_reload(:project) { create(:project, group: parent_group) } + let(:parent_group_settings) { parent_group.namespace_settings } + let(:project_settings) { project.send(settings_association) } + + describe "##{settings_attribute_name}" do + subject(:cascading_attribute) { project_settings.send(settings_attribute_name) } + + before do + stub_application_setting(settings_attribute_name => false) + end + + context 'when parent does not lock the attribute' do + before do + parent_group_settings.update!(settings_attribute_name => false) + end + + it 'returns project setting' do + project_settings.update!(settings_attribute_name => true) + + expect(cascading_attribute).to eq(true) + end + + it 'returns the correct dirty value' do + project_settings.send("#{settings_attribute_name}=", true) + + expect(cascading_attribute).to eq(true) + end + end + + context 'when parent locks the attribute' do + before do + project_settings.update!(settings_attribute_name => false) + parent_group_settings.update!( + "lock_#{settings_attribute_name}" => true, + settings_attribute_name => false + ) + project_settings.clear_memoization("#{settings_attribute_name}_locked_ancestor") + end + + it 'returns the parent value' do + expect(cascading_attribute).to eq(false) + end + + it 'does not allow the local value to be saved' do + project_settings.send("#{settings_attribute_name}=", true) + + expect { project_settings.save! }.to raise_error( + ActiveRecord::RecordInvalid, + /cannot be changed because it is locked by an ancestor/ + ) + end + end + + context 'when the application settings locks the attribute' do + before do + project_settings.update!(settings_attribute_name => true) + stub_application_setting("lock_#{settings_attribute_name}" => true, settings_attribute_name => true) + end + + it 'returns the application setting value' do + expect(cascading_attribute).to eq(true) + end + + it 'does not allow the local value to be saved' do + project_settings.send("#{settings_attribute_name}=", false) + + expect { project_settings.save! } + .to raise_error( + ActiveRecord::RecordInvalid, + /cannot be changed because it is locked by an ancestor/ + ) + end + end + + context 'when parent locked the attribute then the application settings locks it' do + before do + project_settings.update!(settings_attribute_name => true) + parent_group_settings.update!("lock_#{settings_attribute_name}" => true, settings_attribute_name => false) + stub_application_setting("lock_#{settings_attribute_name}" => true, settings_attribute_name => true) + end + + it 'returns the application setting value' do + expect(cascading_attribute).to eq(true) + end + end + end + + describe "##{settings_attribute_name}_locked?" do + shared_examples 'not locked' do + it 'is not locked by an ancestor' do + expect(project_settings.send("#{settings_attribute_name}_locked_by_ancestor?")).to eq(false) + end + + it 'is not locked by application setting' do + expect(project_settings.send("#{settings_attribute_name}_locked_by_application_setting?")).to eq(false) + end + + it 'does not return a locked namespace' do + expect(project_settings.send("#{settings_attribute_name}_locked_ancestor")).to be_nil + end + end + + context 'when parent does not lock the attribute' do + it_behaves_like 'not locked' + end + + context 'when parent locks the attribute' do + before do + parent_group_settings.update!("lock_#{settings_attribute_name}".to_sym => true, + settings_attribute_name => false) + end + + it 'is locked by an ancestor' do + expect(project_settings.send("#{settings_attribute_name}_locked_by_ancestor?")).to eq(true) + end + + it 'is not locked by application setting' do + expect(project_settings.send("#{settings_attribute_name}_locked_by_application_setting?")).to eq(false) + end + + it 'returns a locked namespace settings object' do + expect(project_settings.send("#{settings_attribute_name}_locked_ancestor").namespace_id) + .to eq(parent_group_settings.namespace_id) + end + end + + context 'when not locked by application settings' do + before do + stub_application_setting("lock_#{settings_attribute_name}" => false) + end + + it_behaves_like 'not locked' + end + + context 'when locked by application settings' do + before do + stub_application_setting("lock_#{settings_attribute_name}" => true) + end + + it 'is not locked by an ancestor' do + expect(project_settings.send("#{settings_attribute_name}_locked_by_ancestor?")).to eq(false) + end + + it 'is locked by application setting' do + expect(project_settings.send("#{settings_attribute_name}_locked_by_application_setting?")).to eq(true) + end + + it 'does not return a locked namespace' do + expect(project_settings.send("#{settings_attribute_name}_locked_ancestor")).to be_nil + end + end + end +end