Backport #1186

lib/fluent/config/configure_proxy.rb

@@ -129,6 +129,12 @@ def merge_for_finalized(other)
         merged
       end
 
+      def option_value_type!(name, opts, key, klass)
+        if opts.has_key?(key) && !opts[key].is_a?(klass)
+          raise ArgumentError, "#{name}: #{key} must be a #{klass}, but #{opts[key].class}"
+        end
+      end
+
       def parameter_configuration(name, *args, &block)
         name = name.to_sym
 
@@ -145,16 +151,27 @@ def parameter_configuration(name, *args, &block)
 
         type = opts[:type]
         if block && type
-          raise ArgumentError, "#{self.name}: both of block and type cannot be specified"
+          raise ArgumentError, "#{name}: both of block and type cannot be specified"
         end
 
         begin
           type = :string if type.nil?
           block ||= Configurable.lookup_type(type)
         rescue ConfigError
           # override error message
-          raise ArgumentError, "#{self.name}: unknown config_argument type `#{type}'"
+          raise ArgumentError, "#{name}: unknown config_argument type `#{type}'"
+        end
+
+        option_value_type!(name, opts, :desc, String)
+        option_value_type!(name, opts, :alias, Symbol)
+        option_value_type!(name, opts, :deprecated, String)
+        option_value_type!(name, opts, :obsoleted, String)
+        if type == :enum
+          if !opts.has_key?(:list) || !opts[:list].all?{|v| v.is_a?(Symbol) }
+            raise ArgumentError, "#{name}: enum parameter requires :list of Symbols"
+          end
         end
+        option_value_type!(name, opts, :value_type, Symbol) # hash, array
 
         if opts.has_key?(:default)
           config_set_default(name, opts[:default])
@@ -164,6 +181,10 @@ def parameter_configuration(name, *args, &block)
           config_set_desc(name, opts[:desc])
         end
 
+        if opts[:deprecated] && opts[:obsoleted]
+          raise ArgumentError, "#{name}: both of deprecated and obsoleted cannot be specified at once"
+        end
+
         [name, block, opts]
       end
 
@@ -218,7 +239,7 @@ def desc(description)
 
       def config_section(name, *args, &block)
         unless block_given?
-          raise ArgumentError, "#{self.name}: config_section requires block parameter"
+          raise ArgumentError, "#{name}: config_section requires block parameter"
         end
         name = name.to_sym
 

lib/fluent/config/error.rb

@@ -20,4 +20,7 @@ class ConfigError < StandardError
 
   class ConfigParseError < ConfigError
   end
+
+  class ObsoletedParameterError < ConfigError
+  end
 end

lib/fluent/config/section.rb

@@ -100,6 +100,7 @@ def self.generate(proxy, conf, logger, plugin_class, stack = [])
             logger.error "config error in:\n#{conf}"
             raise ConfigError, "'<#{proxy.name} ARG>' section requires argument" + section_stack
           end
+          # argument should NOT be deprecated... (argument always has a value: '')
         end
 
         proxy.params.each_pair do |name, defval|
@@ -112,6 +113,20 @@ def self.generate(proxy, conf, logger, plugin_class, stack = [])
                     conf[opts[:alias].to_s]
                   end
             section_params[varname] = self.instance_exec(val, opts, name, &block)
+
+            # Source of definitions of deprecated/obsoleted:
+            # https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Deprecated_and_obsolete_features
+            #
+            # Deprecated: These deprecated features can still be used, but should be used with caution
+            #             because they are expected to be removed entirely sometime in the future.
+            # Obsoleted: These obsolete features have been entirely removed from JavaScript and can no longer be used.
+            if opts[:deprecated]
+              logger.warn "'#{name}' parameter is deprecated: #{opts[:deprecated]}"
+            end
+            if opts[:obsoleted]
+              logger.error "config error in:\n#{conf}" if logger
+              raise ObsoletedParameterError, "'#{name}' parameter is already removed: #{opts[:obsoleted]}" + section_stack
+            end
           end
           unless section_params.has_key?(varname)
             logger.error "config error in:\n#{conf}"

lib/fluent/plugin/out_forward.rb

@@ -86,9 +86,8 @@ def initialize
 
     attr_reader :nodes
 
-    # backward compatibility
-    config_param :port, :integer, default: DEFAULT_LISTEN_PORT
-    config_param :host, :string, default: nil
+    config_param :port, :integer, default: DEFAULT_LISTEN_PORT, deprecated: "User <server> host xxx </server> instead."
+    config_param :host, :string, default: nil, deprecated: "Use <server> port xxx </server> instead."
 
     attr_accessor :extend_internal_protocol
 
@@ -97,7 +96,6 @@ def configure(conf)
 
       # backward compatibility
       if host = conf['host']
-        log.warn "'host' option in forward output is obsoleted. Use '<server> host xxx </server>' instead."
         port = conf['port']
         port = port ? port.to_i : DEFAULT_LISTEN_PORT
         e = conf.add_element('server')

test/config/test_configurable.rb

@@ -158,6 +158,13 @@ class Example5
       config_param :secret_param2, :string, secret: true
     end
   end
+
+  class UnRecommended
+    include Fluent::Configurable
+    attr_accessor :log
+    config_param :key1, :string, default: 'deprecated', deprecated: "key1 will be removed."
+    config_param :key2, :string, default: 'obsoleted', obsoleted: "key2 has been removed."
+  end
 end
 
 module Fluent::Config
@@ -740,5 +747,87 @@ def assert_secret_param(key, value)
         end
       end
     end
+    sub_test_case 'non-required options for config_param' do
+      test 'desc must be a string if specified' do
+        assert_raise ArgumentError.new("key: desc must be a String, but Symbol") do
+          class InvalidDescClass
+            include Fluent::Configurable
+            config_param :key, :string, default: '', desc: :invalid_description
+          end
+        end
+      end
+      test 'alias must be a symbol if specified' do
+        assert_raise ArgumentError.new("key: alias must be a Symbol, but String") do
+          class InvalidAliasClass
+            include Fluent::Configurable
+            config_param :key, :string, default: '', alias: 'yay'
+          end
+        end
+      end
+      test 'deprecated must be a string if specified' do
+        assert_raise ArgumentError.new("key: deprecated must be a String, but TrueClass") do
+          class InvalidDeprecatedClass
+            include Fluent::Configurable
+            config_param :key, :string, default: '', deprecated: true
+          end
+        end
+      end
+      test 'obsoleted must be a string if specified' do
+        assert_raise ArgumentError.new("key: obsoleted must be a String, but TrueClass") do
+          class InvalidObsoletedClass
+            include Fluent::Configurable
+            config_param :key, :string, default: '', obsoleted: true
+          end
+        end
+      end
+      test 'value_type for hash must be a symbol' do
+        assert_raise ArgumentError.new("key: value_type must be a Symbol, but String") do
+          class InvalidValueTypeOfHashClass
+            include Fluent::Configurable
+            config_param :key, :hash, value_type: 'yay'
+          end
+        end
+      end
+      test 'value_type for array must be a symbol' do
+        assert_raise ArgumentError.new("key: value_type must be a Symbol, but String") do
+          class InvalidValueTypeOfArrayClass
+            include Fluent::Configurable
+            config_param :key, :array, value_type: 'yay'
+          end
+        end
+      end
+    end
+    sub_test_case 'enum parameters' do
+      test 'list must be specified as an array of symbols'
+    end
+    sub_test_case 'deprecated/obsoleted parameters' do
+      test 'both cannot be specified at once' do
+        assert_raise ArgumentError.new("param1: both of deprecated and obsoleted cannot be specified at once") do
+          class Buggy1
+            include Fluent::Configurable
+            config_param :param1, :string, default: '', deprecated: 'yay', obsoleted: 'foo!'
+          end
+        end
+      end
+
+      test 'warned if deprecated parameter is configured' do
+        obj = ConfigurableSpec::UnRecommended.new
+        obj.log = Fluent::Test::TestLogger.new
+        obj.configure(Fluent::Config::Element.new('ROOT', '', {'key1' => 'yay'}, []))
+
+        assert_equal 'yay', obj.key1
+        first_log = obj.log.logs.first
+        assert{ first_log && first_log.include?("[warn]") && first_log.include?("'key1' parameter is deprecated: key1 will be removed.") }
+      end
+
+      test 'error raised if obsoleted parameter is configured' do
+        obj = ConfigurableSpec::UnRecommended.new
+        obj.log = Fluent::Test::TestLogger.new
+
+        assert_raise Fluent::ObsoletedParameterError.new("'key2' parameter is already removed: key2 has been removed.") do
+          obj.configure(Fluent::Config::Element.new('ROOT', '', {'key2' => 'yay'}, []))
+        end
+      end
+    end
   end
 end

test/config/test_types.rb

@@ -72,9 +72,9 @@ class TestConfigTypes < ::Test::Unit::TestCase
       assert_equal :val, Config::ENUM_TYPE.call('val', {list: [:val, :value, :v]})
       assert_equal :v, Config::ENUM_TYPE.call('v', {list: [:val, :value, :v]})
       assert_equal :value, Config::ENUM_TYPE.call('value', {list: [:val, :value, :v]})
-      assert_raises(Fluent::ConfigError){ Config::ENUM_TYPE.call('x', {list: [:val, :value, :v]}) }
-      assert_raises(RuntimeError){ Config::ENUM_TYPE.call('val', {}) }
-      assert_raises(RuntimeError){ Config::ENUM_TYPE.call('val', {list: ["val", "value", "v"]}) }
+      assert_raises(Fluent::ConfigError.new("valid options are val,value,v but got x")){ Config::ENUM_TYPE.call('x', {list: [:val, :value, :v]}) }
+      assert_raises(RuntimeError.new("Plugin BUG: config type 'enum' requires :list of symbols")){ Config::ENUM_TYPE.call('val', {}) }
+      assert_raises(RuntimeError.new("Plugin BUG: config type 'enum' requires :list of symbols")){ Config::ENUM_TYPE.call('val', {list: ["val", "value", "v"]}) }
     end
 
     test 'integer' do
@@ -138,6 +138,8 @@ class TestConfigTypes < ::Test::Unit::TestCase
 
       assert_equal({"x"=>1,"y"=>60,"z"=>3600}, Config::HASH_TYPE.call('{"x":"1s","y":"1m","z":"1h"}', {value_type: :time}))
       assert_equal({"x"=>1,"y"=>60,"z"=>3600}, Config::HASH_TYPE.call('x:1s,y:1m,z:1h',               {value_type: :time}))
+
+      assert_raise(RuntimeError.new("unknown type in REFORMAT: foo")){ Config::HASH_TYPE.call("x:1,y:2", {value_type: :foo}) }
     end
 
     test 'array' do
@@ -162,6 +164,8 @@ class TestConfigTypes < ::Test::Unit::TestCase
       }
       assert_equal(["1","2"], Config::ARRAY_TYPE.call('["1","2"]', array_options))
       assert_equal(["3"], Config::ARRAY_TYPE.call('["3"]', array_options))
+
+      assert_raise(RuntimeError.new("unknown type in REFORMAT: foo")){ Config::ARRAY_TYPE.call("1,2", {value_type: :foo}) }
     end
   end
 end