Merge branch 'jy-cascadig-duo-features-enabled-2' into 'master'

Add duo_features_enabled cascading setting

See merge request !144931



Merged-by: Drew Blessing <drew@gitlab.com>
Approved-by: Drew Blessing <drew@gitlab.com>
Approved-by: Alper Akgun <aakgun@gitlab.com>
Reviewed-by: Drew Blessing <drew@gitlab.com>
Reviewed-by: Mark Chao <mchao@gitlab.com>
Reviewed-by: Terri Chu <tchu@gitlab.com>
Co-authored-by: Jessie Young <jessieyoung@gitlab.com>

app/models/concerns/cascading_project_setting_attribute.rb

+# 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

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
 

db/migrate/20240213223630_add_duo_features_enabled_cascading_setting.rb

+# 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

db/schema_migrations/20240213223630

+b6e84d82dcc591adbdb7b08f79d6a7fd0da741e34fdb9205d26a8903e43868d0
\ No newline at end of file

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)),
@@ -11603,6 +11605,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))

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

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') }

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

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

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

spec/support/shared_examples/models/concerns/cascading_project_setting_shared_examples.rb

+# 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