Fixes #32403 - Add validations to a setting DSL

Add ability to define validation per Setting through DSL.
It tries to keep similar syntax to rails model validations.

app/models/setting.rb

@@ -14,6 +14,8 @@ class Setting < ApplicationRecord
   TYPES = %w{integer boolean hash array string}
   FROZEN_ATTRS = %w{name category}
   NONZERO_ATTRS = %w{puppet_interval idle_timeout entries_per_page outofsync_interval}
+  # constant BLANK_ATTRS is deprecated and all settings without custom validation allow blank values
+  # if you wish to validate non-empty arrays, please add validation through the new setting DSL
   BLANK_ATTRS = %w{ host_owner trusted_hosts login_delegation_logout_url root_pass default_location default_organization websockets_ssl_key websockets_ssl_cert oauth_consumer_key oauth_consumer_secret login_text oidc_audience oidc_issuer oidc_algorithm
                     smtp_address smtp_domain smtp_user_name smtp_password smtp_openssl_verify_mode smtp_authentication sendmail_arguments sendmail_location http_proxy http_proxy_except_list default_locale default_timezone ssl_certificate ssl_ca_file server_ca_file ssl_priv_key default_pxe_item_global default_pxe_item_local oidc_jwks_url instance_title }
   ARRAY_HOSTNAMES = %w{trusted_hosts}
@@ -34,12 +36,9 @@ def validate(record)
 
   validates :name, :presence => true, :uniqueness => true
   validates :description, :presence => true
-  validates :default, :presence => true, :unless => proc { |s| s.settings_type == "boolean" || BLANK_ATTRS.include?(s.name) }
-  validates :default, :inclusion => {:in => [true, false]}, :if => proc { |s| s.settings_type == "boolean" }
   validates :value, :numericality => true, :length => {:maximum => 8}, :if => proc { |s| s.settings_type == "integer" }
   validates :value, :numericality => {:greater_than => 0}, :if => proc { |s| NONZERO_ATTRS.include?(s.name) }
   validates :value, :inclusion => {:in => [true, false]}, :if => proc { |s| s.settings_type == "boolean" }
-  validates :value, :presence => true, :if => proc { |s| s.settings_type == "array" && !BLANK_ATTRS.include?(s.name) }
   validates :settings_type, :inclusion => {:in => TYPES}, :allow_nil => true, :allow_blank => true
   validates :value, :url_schema => ['http', 'https'], :if => proc { |s| URI_ATTRS.include?(s.name) }
 
@@ -55,6 +54,7 @@ def validate(record)
   before_validation :set_setting_type_from_value
   before_save :clear_value_when_default
   validate :validate_frozen_attributes
+  # Custom validations are added from SettingManager class
   after_find :readonly_when_overridden
   after_save :refresh_registry_value
   default_scope -> { order(:name) }

app/registries/foreman/plugin.rb

@@ -328,7 +328,8 @@ def tests_to_skip(hash)
     #         default: true,
     #         description: N_('Use Puppet that goes to 11'),
     #         full_name: N_('Use shiny puppet'),
-    #         encrypt: true)
+    #         encrypted: true,
+    #         validate: /^cool/)
     #       end
     #     end
     #   end

app/registries/foreman/setting_manager.rb

@@ -25,6 +25,13 @@ def category(category_name, category_label = nil, &block)
       CategoryMapper.new(@context_name, category_name.to_s).instance_eval(&block)
     end
 
+    class ValueLambdaValidator < ActiveModel::Validator
+      def validate(record)
+        return true if options[:allow_blank] && record.value.blank?
+        record.errors.add(:value, :invalid) unless options[:proc].call(record.value)
+      end
+    end
+
     class CategoryMapper
       attr_reader :context_name, :category_name
 
@@ -52,11 +59,12 @@ def available_types
       #               default: true,
       #               description: N_('Use Puppet that goes to 11'),
       #               full_name: N_('Use shiny puppet'),
-      #               encrypt: true)
+      #               encrypted: true,
+      #               validate: /^cool/)
       #     end
       #   end
       #
-      def setting(name, default:, description:, type:, full_name: nil, value: nil, collection: nil, encrypted: false, **options)
+      def setting(name, default:, description:, type:, full_name: nil, value: nil, collection: nil, encrypted: false, validate: nil, **options)
         raise ::Foreman::Exception.new(N_("Setting '%s' is already defined, please avoid collisions"), name) if storage.key?(name.to_s)
         raise ::Foreman::Exception.new(N_("Setting '%s' has an invalid type definition. Please use a valid type."), name) unless available_types.include?(type)
         storage[name.to_s] = {
@@ -71,6 +79,40 @@ def setting(name, default:, description:, type:, full_name: nil, value: nil, col
           encrypted: encrypted,
           options: options,
         }
+        _inline_validates(name, validate) if validate
+        storage[name.to_s]
+      end
+
+      def _inline_validates(name, validations)
+        if validations.is_a?(Proc)
+          validates_with name, ValueLambdaValidator, proc: validations
+          return
+        elsif validations.is_a?(Regexp)
+          validations = { format: { with: validations } }
+        elsif validations.is_a?(Symbol)
+          validations = { validations => true }
+        end
+        validates(name, validations)
+      end
+
+      def validates(name, validations)
+        _wrap_validation_if(name, validations)
+        Setting.validates(:value, validations)
+      end
+
+      def validates_with(name, *args, &block)
+        options = args.extract_options!
+        _wrap_validation_if(name, options)
+        options[:attributes] = [:value]
+        args << options
+        Setting.validates_with(*args, &block)
+      end
+
+      def _wrap_validation_if(setting_name, options)
+        options[:if] = [
+          ->(setting) { setting.name == setting_name.to_s },
+          *options[:if],
+        ]
       end
     end
   end

developer_docs/how_to_create_a_plugin.asciidoc

@@ -1678,6 +1678,38 @@ The settings are strongly typed and you have to define it.
 The basic types supported by Foreman are: `:boolean`, `:integer`, `:float`, `:string`, `:text`, `:hash`, `:array`.
 The `:text` type supports markdown and usage of such setting should expect markdown syntax when using it.
 
+==== Validate setting values
+
+To validate setting value, you can use API, that tries to mimic the API of ActiveRecord.
+We can use most of the perks offered by ActiveRecord, only defining on setting name instead of attribute.
+We are adding just some shorthands like direct regexp validations.
+The attribute is always `value`, you can't validate anything else as it is the only user input.
+
+You have two ways to define the validations:
+* inline with setting definition by symbol matching ActiveRecord validator, RegExp on strings, or lambda function that gets value to validate as argument.
+* using `validates` of `validates_with` methods, that mimic [Rails validation methods](https://api.rubyonrails.org/classes/ActiveModel/Validations/ClassMethods.html), but are using setting names instead of attribute names, as the validated attribute is always value in this case.
+
+[source, ruby]
+....
+settings do
+  category(:cfgmgmt) do
+    # Following definitions are missing full names for simplification
+
+    setting(:blank_setting, type: :string,  default: '', description: 'Unnecessary setting')
+
+    setting(:cool_setting, type: :string, default: 'cool' description: 'Setting with only cool values', validate: /^cool.*/)
+
+    setting(:cooler_puppet, type: :integer, default: 5, description: 'Use Puppet that goes to 11', validate: ->(value) { value <= 11 })
+
+    validates(:cooler_puppet, numericality: { greater_than: 10 }, if: -> { Setting[:cool_setting] == 'coolest' }, allow_blank: true)
+
+    # the validator needs to be ActiveModel::Validator
+    validates_with :cool_setting, MyCoolnessValidator
+  end
+end
+....
+
+
 [[config-files]]
 ==== Config files
 

test/models/setting_test.rb

@@ -2,7 +2,6 @@
 
 class SettingTest < ActiveSupport::TestCase
   should validate_presence_of(:name)
-  should validate_presence_of(:default)
   should validate_uniqueness_of(:name)
   should validate_inclusion_of(:settings_type).in_array(Setting::TYPES)
 
@@ -230,11 +229,6 @@ def test_should_autoselect_correct_type_for_boolean_value
     check_correct_type_for "boolean", false
   end
 
-  # tests for default type constraints
-  test "arrays cannot be empty by default" do
-    check_setting_did_not_save_with :name => "foo", :value => [], :default => ["a", "b", "c"], :description => "test foo"
-  end
-
   test "hashes can be empty by default" do
     check_properties_saved_and_loaded_ok :name => "foo", :value => {}, :default => {"a" => "A"}, :description => "test foo"
   end

test/unit/setting_manager_test.rb

@@ -83,4 +83,78 @@ class SettingManagerTest < ActiveSupport::TestCase
       end
     end
   end
+
+  describe 'Validations' do
+    setup do
+      @validation_backup = Setting._validators[:value].dup
+    end
+
+    teardown do
+      Setting._validators[:value] = @validation_backup
+    end
+
+    it 'defines validation on Setting model for given setting name' do
+      Foreman::SettingManager.define(:test_context) do
+        category(:general) do
+          setting(:validfoo,
+            type: :string,
+            default: 'bar@example.com',
+            description: 'This is nicely described foo setting',
+            full_name: 'Foo setting',
+            validate: :email)
+        end
+      end
+      Foreman.settings.load_definitions
+      setting = Setting.new(name: 'validfoo', default: '-omited-', description: 'Omited', value: 'notanemail')
+      assert_not setting.valid?
+    end
+
+    it 'defines validation through validates' do
+      Foreman::SettingManager.define(:test_context) do
+        category(:general) do
+          setting(:validfoo,
+            type: :string,
+            default: 'bar@example.com',
+            description: 'This is nicely described foo setting',
+            full_name: 'Foo setting')
+          validates(:validfoo, email: true)
+        end
+      end
+      Foreman.settings.load_definitions
+      setting = Setting.new(name: 'validfoo', default: '-omited-', description: 'Omited', value: 'notanemail')
+      assert_not setting.valid?
+    end
+
+    it 'defines validation through RegExp' do
+      Foreman::SettingManager.define(:test_context) do
+        category(:general) do
+          setting(:validfoo2,
+            type: :string,
+            default: 'bar@include.com',
+            description: 'This is nicely described foo setting',
+            full_name: 'Foo setting',
+            validate: /.*@include.com/)
+        end
+      end
+      Foreman.settings.load_definitions
+      setting = Setting.new(name: 'validfoo2', default: '-omited-', description: 'Omited', value: 'bar@notvalid.com')
+      assert_not setting.valid?
+    end
+
+    it 'defines validation through Proc' do
+      Foreman::SettingManager.define(:test_context) do
+        category(:general) do
+          setting(:validfoo3,
+            type: :string,
+            default: 'bar@example.com',
+            description: 'This is nicely described foo setting',
+            full_name: 'Foo setting',
+            validate: ->(value) { !value.nil? && value.starts_with?('bar') })
+        end
+      end
+      Foreman.settings.load_definitions
+      setting = Setting.new(name: 'validfoo3', default: '-omited-', description: 'Omited', value: 'notvalid@example.com')
+      assert_not setting.valid?
+    end
+  end
 end