Commit 63ddf132 authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch 'dblessing_cascading_settings_final' into 'master'

Cascade delayed project removal setting to parent namespace [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55678
parents 189a035e eb311888
# frozen_string_literal: true
#
# Cascading attributes enables managing settings in a flexible way.
#
# - Instance administrator can define an instance-wide default setting, or
# lock the setting to prevent change by group owners.
# - Group maintainers/owners can define a default setting for their group, or
# lock the setting to prevent change by sub-group maintainers/owners.
#
# Behavior:
#
# - When a group does not have a value (value is `nil`), cascade up the
# hierarchy to find the first non-nil value.
# - Settings can be locked at any level to prevent groups/sub-groups from
# overriding.
# - If the setting isn't locked, the default can be overridden.
# - An instance administrator or group maintainer/owner can push settings values
# to groups/sub-groups to override existing values, even when the setting
# is not otherwise locked.
#
module CascadingNamespaceSettingAttribute
extend ActiveSupport::Concern
include Gitlab::Utils::StrongMemoize
class_methods do
def cascading_settings_feature_enabled?
::Feature.enabled?(:cascading_namespace_settings, default_enabled: false)
end
private
# Facilitates the cascading lookup of values and,
# similar to Rails' `attr_accessor`, defines convenience methods such as
# a reader, writer, and validators.
#
# Example: `cascading_attr :delayed_project_removal`
#
# Public methods defined:
# - `delayed_project_removal`
# - `delayed_project_removal=`
# - `delayed_project_removal_locked?`
# - `delayed_project_removal_locked_by_ancestor?`
# - `delayed_project_removal_locked_by_application_setting?`
# - `delayed_project_removal?` (only defined for boolean attributes)
# - `delayed_project_removal_locked_ancestor` - Returns locked namespace settings object (only namespace_id)
#
# Defined validators ensure attribute value cannot be updated if locked by
# an ancestor or application settings.
#
# Requires database columns be present in both `namespace_settings` and
# `application_settings`.
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)
alias_boolean(attribute)
# private methods
define_validator_methods(attribute)
define_after_update(attribute)
validate :"#{attribute}_changeable?"
validate :"lock_#{attribute}_changeable?"
after_update :"clear_descendant_#{attribute}_locks", if: -> { saved_change_to_attribute?("lock_#{attribute}", to: true) }
end
end
# The cascading attribute reader method handles lookups
# with the following criteria:
#
# 1. Returns the dirty value, if the attribute has changed.
# 2. Return locked ancestor value.
# 3. Return locked instance-level application settings value.
# 4. Return this namespace's attribute, if not nil.
# 5. Return value from nearest ancestor where value is not nil.
# 6. Return instance-level application setting.
def define_attr_reader(attribute)
define_method(attribute) do
strong_memoize(attribute) do
next self[attribute] unless self.class.cascading_settings_feature_enabled?
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|
clear_memoization(attribute)
super(value)
end
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 alias_boolean(attribute)
return unless Gitlab::Database.exists? && type_for_attribute(attribute).type == :boolean
alias_method :"#{attribute}?", attribute
end
# Defines two validations - one for the cascadable attribute itself and one
# for the lock attribute. Only allows the respective value to change if
# an ancestor has not already locked the value.
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
define_method("lock_#{attribute}_changeable?") do
return unless cascading_attribute_changed?("lock_#{attribute}")
if cascading_attribute_locked?(attribute)
return errors.add(:"lock_#{attribute}", s_('CascadingSettings|cannot be changed because it is locked by an ancestor'))
end
# Don't allow locking a `nil` attribute.
# Even if the value being locked is currently cascaded from an ancestor,
# it should be copied to this record to avoid the ancestor changing the
# value unexpectedly later.
return unless self[attribute].nil? && public_send("lock_#{attribute}?") # rubocop:disable GitlabSecurity/PublicSend
errors.add(attribute, s_('CascadingSettings|cannot be nil when locking the attribute'))
end
private :"#{attribute}_changeable?", :"lock_#{attribute}_changeable?"
end
# When a particular group locks the attribute, clear all sub-group locks
# since the higher lock takes priority.
def define_after_update(attribute)
define_method("clear_descendant_#{attribute}_locks") do
self.class.where(namespace_id: descendants).update_all("lock_#{attribute}" => false)
end
private :"clear_descendant_#{attribute}_locks"
end
end
private
def locked_value(attribute)
ancestor = locked_ancestor(attribute)
return ancestor.read_attribute(attribute) if ancestor
Gitlab::CurrentSettings.public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend
end
def locked_ancestor(attribute)
return unless self.class.cascading_settings_feature_enabled?
return unless namespace.has_parent?
strong_memoize(:"#{attribute}_locked_ancestor") do
self.class
.select(:namespace_id, "lock_#{attribute}", attribute)
.where(namespace_id: namespace_ancestor_ids)
.where(self.class.arel_table["lock_#{attribute}"].eq(true))
.limit(1).load.first
end
end
def locked_by_ancestor?(attribute)
return false unless self.class.cascading_settings_feature_enabled?
locked_ancestor(attribute).present?
end
def locked_by_application_setting?(attribute)
return false unless self.class.cascading_settings_feature_enabled?
Gitlab::CurrentSettings.public_send("lock_#{attribute}") # rubocop:disable GitlabSecurity/PublicSend
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
end
def cascaded_ancestor_value(attribute)
return unless namespace.has_parent?
# rubocop:disable GitlabSecurity/SqlInjection
self.class
.select(attribute)
.joins("join unnest(ARRAY[#{namespace_ancestor_ids.join(',')}]) 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
end
def namespace_ancestor_ids
strong_memoize(:namespace_ancestor_ids) do
namespace.self_and_ancestors(hierarchy_order: :asc).pluck(:id).reject { |id| id == namespace_id }
end
end
def descendants
strong_memoize(:descendants) do
namespace.descendants.pluck(:id)
end
end
end
# frozen_string_literal: true # frozen_string_literal: true
class NamespaceSetting < ApplicationRecord class NamespaceSetting < ApplicationRecord
include CascadingNamespaceSettingAttribute
cascading_attr :delayed_project_removal
belongs_to :namespace, inverse_of: :namespace_settings belongs_to :namespace, inverse_of: :namespace_settings
validate :default_branch_name_content validate :default_branch_name_content
...@@ -9,7 +13,8 @@ class NamespaceSetting < ApplicationRecord ...@@ -9,7 +13,8 @@ class NamespaceSetting < ApplicationRecord
before_validation :normalize_default_branch_name before_validation :normalize_default_branch_name
NAMESPACE_SETTINGS_PARAMS = [:default_branch_name, :delayed_project_removal, :resource_access_token_creation_allowed].freeze NAMESPACE_SETTINGS_PARAMS = [:default_branch_name, :delayed_project_removal,
:lock_delayed_project_removal, :resource_access_token_creation_allowed].freeze
self.primary_key = :namespace_id self.primary_key = :namespace_id
......
---
title: Cascade delayed project removal setting lookup to parent namespace
merge_request: 55678
author:
type: added
---
name: cascading_namespace_settings
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55678
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/321724
milestone: '13.11'
type: development
group: group::access
default_enabled: false
# frozen_string_literal: true
class ChangeNamespaceSettingsDelayedProjectRemovalNull < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
change_column :namespace_settings, :delayed_project_removal, :boolean, null: true, default: nil
end
def down
change_column_default :namespace_settings, :delayed_project_removal, false
change_column_null :namespace_settings, :delayed_project_removal, false, false
end
end
# frozen_string_literal: true
class AddLockDelayedProjectRemovalToNamespaceSettings < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :namespace_settings, :lock_delayed_project_removal, :boolean, default: false, null: false
end
end
# frozen_string_literal: true
class AddDelayedProjectRemovalToApplicationSettings < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :application_settings, :delayed_project_removal, :boolean, default: false, null: false
end
end
# frozen_string_literal: true
class AddLockDelayedProjectRemovalToApplicationSettings < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :application_settings, :lock_delayed_project_removal, :boolean, default: false, null: false
end
end
ad6e0feff16589839714098a69673edcba50af7a62d98cd078585c5d2aada919
\ No newline at end of file
9263c522f0632f5b4fc0004e1fe9666bc3a44e4f70cf0d21aab5bb229f08ab5c
\ No newline at end of file
72491b1834a1256a197e8f49c599b28b41773226db4fe70ce402903674d2f622
\ No newline at end of file
e99b8a6242589992ae8b618cb502d16b67672856cef024c1aafe00a1e64e41b9
\ No newline at end of file
...@@ -9439,6 +9439,8 @@ CREATE TABLE application_settings ( ...@@ -9439,6 +9439,8 @@ CREATE TABLE application_settings (
in_product_marketing_emails_enabled boolean DEFAULT true NOT NULL, in_product_marketing_emails_enabled boolean DEFAULT true NOT NULL,
asset_proxy_whitelist text, asset_proxy_whitelist text,
admin_mode boolean DEFAULT false NOT NULL, admin_mode boolean DEFAULT false NOT NULL,
delayed_project_removal boolean DEFAULT false NOT NULL,
lock_delayed_project_removal 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_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)), CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
...@@ -14665,8 +14667,9 @@ CREATE TABLE namespace_settings ( ...@@ -14665,8 +14667,9 @@ CREATE TABLE namespace_settings (
allow_mfa_for_subgroups boolean DEFAULT true NOT NULL, allow_mfa_for_subgroups boolean DEFAULT true NOT NULL,
default_branch_name text, default_branch_name text,
repository_read_only boolean DEFAULT false NOT NULL, repository_read_only boolean DEFAULT false NOT NULL,
delayed_project_removal boolean DEFAULT false NOT NULL, delayed_project_removal boolean,
resource_access_token_creation_allowed boolean DEFAULT true NOT NULL, resource_access_token_creation_allowed boolean DEFAULT true NOT NULL,
lock_delayed_project_removal boolean DEFAULT false NOT NULL,
CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)) CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255))
); );
...@@ -5646,6 +5646,12 @@ msgstr "" ...@@ -5646,6 +5646,12 @@ msgstr ""
msgid "Capacity threshold" msgid "Capacity threshold"
msgstr "" msgstr ""
msgid "CascadingSettings|cannot be changed because it is locked by an ancestor"
msgstr ""
msgid "CascadingSettings|cannot be nil when locking the attribute"
msgstr ""
msgid "Certain user content will be moved to a system-wide \"Ghost User\" in order to maintain content for posterity. For further information, please refer to the %{link_start}user account deletion documentation.%{link_end}" msgid "Certain user content will be moved to a system-wide \"Ghost User\" in order to maintain content for posterity. For further information, please refer to the %{link_start}user account deletion documentation.%{link_end}"
msgstr "" msgstr ""
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe NamespaceSetting, 'CascadingNamespaceSettingAttribute' do
let(:group) { create(:group) }
let(:subgroup) { create(:group, parent: group) }
def group_settings
group.namespace_settings
end
def subgroup_settings
subgroup.namespace_settings
end
describe '#delayed_project_removal' do
subject(:delayed_project_removal) { subgroup_settings.delayed_project_removal }
context 'when the feature is disabled' do
before do
stub_feature_flags(cascading_namespace_settings: false)
group_settings.update!(delayed_project_removal: true)
end
it 'does not cascade' do
expect(delayed_project_removal).to eq(nil)
end
end
context 'when there is no parent' do
context 'and the value is not nil' do
before do
group_settings.update!(delayed_project_removal: true)
end
it 'returns the local value' do
expect(group_settings.delayed_project_removal).to eq(true)
end
end
context 'and the value is nil' do
before do
group_settings.update!(delayed_project_removal: nil)
stub_application_setting(delayed_project_removal: false)
end
it 'returns the application settings value' do
expect(group_settings.delayed_project_removal).to eq(false)
end
end
end
context 'when parent does not lock the attribute' do
context 'and value is not nil' do
before do
group_settings.update!(delayed_project_removal: false)
end
it 'returns local setting when present' do
subgroup_settings.update!(delayed_project_removal: true)
expect(delayed_project_removal).to eq(true)
end
it 'returns the parent value when local value is nil' do
subgroup_settings.update!(delayed_project_removal: nil)
expect(delayed_project_removal).to eq(false)
end
it 'returns the correct dirty value' do
subgroup_settings.delayed_project_removal = true
expect(delayed_project_removal).to eq(true)
end
it 'does not return the application setting value when parent value is false' do
stub_application_setting(delayed_project_removal: true)
expect(delayed_project_removal).to eq(false)
end
end
context 'and the value is nil' do
before do
group_settings.update!(delayed_project_removal: nil, lock_delayed_project_removal: false)
subgroup_settings.update!(delayed_project_removal: nil)
subgroup_settings.clear_memoization(:delayed_project_removal)
end
it 'cascades to the application settings value' do
expect(delayed_project_removal).to eq(false)
end
end
context 'when multiple ancestors set a value' do
let(:third_level_subgroup) { create(:group, parent: subgroup) }
before do
group_settings.update!(delayed_project_removal: true)
subgroup_settings.update!(delayed_project_removal: false)
end
it 'returns the closest ancestor value' do
expect(third_level_subgroup.namespace_settings.delayed_project_removal).to eq(false)
end
end
end
context 'when parent locks the attribute' do
before do
subgroup_settings.update!(delayed_project_removal: true)
group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false)
subgroup_settings.clear_memoization(:delayed_project_removal)
subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor)
end
it 'returns the parent value' do
expect(delayed_project_removal).to eq(false)
end
it 'does not allow the local value to be saved' do
subgroup_settings.delayed_project_removal = nil
expect { subgroup_settings.save! }
.to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/)
end
end
context 'when the application settings locks the attribute' do
before do
subgroup_settings.update!(delayed_project_removal: true)
stub_application_setting(lock_delayed_project_removal: true, delayed_project_removal: true)
end
it 'returns the application setting value' do
expect(delayed_project_removal).to eq(true)
end
it 'does not allow the local value to be saved' do
subgroup_settings.delayed_project_removal = nil
expect { subgroup_settings.save! }
.to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/)
end
end
end
describe '#delayed_project_removal?' do
before do
subgroup_settings.update!(delayed_project_removal: true)
group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false)
subgroup_settings.clear_memoization(:delayed_project_removal)
subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor)
end
it 'aliases the method when the attribute is a boolean' do
expect(subgroup_settings.delayed_project_removal?).to eq(subgroup_settings.delayed_project_removal)
end
end
describe '#delayed_project_removal_locked?' do
shared_examples 'not locked' do
it 'is not locked by an ancestor' do
expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(false)
end
it 'is not locked by application setting' do
expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(false)
end
it 'does not return a locked namespace' do
expect(subgroup_settings.delayed_project_removal_locked_ancestor).to be_nil
end
end
context 'when the feature is disabled' do
before do
stub_feature_flags(cascading_namespace_settings: false)
group_settings.update!(delayed_project_removal: true)
end
it_behaves_like 'not locked'
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
group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false)
subgroup_settings.clear_memoization(:delayed_project_removal)
subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor)
end
it 'is locked by an ancestor' do
expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(true)
end
it 'is not locked by application setting' do
expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(false)
end
it 'returns a locked namespace settings object' do
expect(subgroup_settings.delayed_project_removal_locked_ancestor.namespace_id).to eq(group_settings.namespace_id)
end
end
context 'when not locked by application settings' do
before do
stub_application_setting(lock_delayed_project_removal: false)
end
it_behaves_like 'not locked'
end
context 'when locked by application settings' do
before do
stub_application_setting(lock_delayed_project_removal: true)
end
it 'is not locked by an ancestor' do
expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(false)
end
it 'is locked by application setting' do
expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(true)
end
it 'does not return a locked namespace' do
expect(subgroup_settings.delayed_project_removal_locked_ancestor).to be_nil
end
end
end
describe '#lock_delayed_project_removal=' do
context 'when parent locks the attribute' do
before do
group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false)
subgroup_settings.clear_memoization(:delayed_project_removal)
subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor)
end
it 'does not allow the attribute to be saved' do
subgroup_settings.lock_delayed_project_removal = true
expect { subgroup_settings.save! }
.to raise_error(ActiveRecord::RecordInvalid, /Lock delayed project removal cannot be changed because it is locked by an ancestor/)
end
end
context 'when parent does not lock the attribute' do
before do
group_settings.update!(lock_delayed_project_removal: false)
subgroup_settings.lock_delayed_project_removal = true
end
it 'allows the lock to be set when the attribute is not nil' do
subgroup_settings.delayed_project_removal = true
expect(subgroup_settings.save).to eq(true)
end
it 'does not allow the lock to be saved when the attribute is nil' do
subgroup_settings.delayed_project_removal = nil
expect { subgroup_settings.save! }
.to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be nil when locking the attribute/)
end
end
context 'when application settings locks the attribute' do
before do
stub_application_setting(lock_delayed_project_removal: true)
end
it 'does not allow the attribute to be saved' do
subgroup_settings.lock_delayed_project_removal = true
expect { subgroup_settings.save! }
.to raise_error(ActiveRecord::RecordInvalid, /Lock delayed project removal cannot be changed because it is locked by an ancestor/)
end
end
context 'when application_settings does not lock the attribute' do
before do
stub_application_setting(lock_delayed_project_removal: false)
end
it 'allows the attribute to be saved' do
subgroup_settings.delayed_project_removal = true
subgroup_settings.lock_delayed_project_removal = true
expect(subgroup_settings.save).to eq(true)
end
end
end
describe 'after update callback' do
before do
subgroup_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false)
end
it 'clears descendant locks' do
group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: true)
expect(subgroup_settings.reload.lock_delayed_project_removal).to eq(false)
end
end
end
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