diff --git a/lib/fluent/config/configure_proxy.rb b/lib/fluent/config/configure_proxy.rb index af9be8d135..b47b16e5af 100644 --- a/lib/fluent/config/configure_proxy.rb +++ b/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,7 +151,7 @@ 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 @@ -153,8 +159,19 @@ def parameter_configuration(name, *args, &block) 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 diff --git a/lib/fluent/config/error.rb b/lib/fluent/config/error.rb index 1b7809630e..30374611ae 100644 --- a/lib/fluent/config/error.rb +++ b/lib/fluent/config/error.rb @@ -20,4 +20,7 @@ class ConfigError < StandardError class ConfigParseError < ConfigError end + + class ObsoletedParameterError < ConfigError + end end diff --git a/lib/fluent/config/section.rb b/lib/fluent/config/section.rb index 618988fef3..b9856343e8 100644 --- a/lib/fluent/config/section.rb +++ b/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}" diff --git a/lib/fluent/plugin/out_forward.rb b/lib/fluent/plugin/out_forward.rb index 73858b1952..a19733b11e 100644 --- a/lib/fluent/plugin/out_forward.rb +++ b/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 host xxx instead." + config_param :host, :string, default: nil, deprecated: "Use port xxx 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 ' host xxx ' instead." port = conf['port'] port = port ? port.to_i : DEFAULT_LISTEN_PORT e = conf.add_element('server') diff --git a/test/config/test_configurable.rb b/test/config/test_configurable.rb index d09c40c5a6..c2f1d3bbfd 100644 --- a/test/config/test_configurable.rb +++ b/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 diff --git a/test/config/test_types.rb b/test/config/test_types.rb index ffd96b2bc2..6d826f4eec 100644 --- a/test/config/test_types.rb +++ b/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