Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config_param options to specify deprecated/obsoleted parameters #1186

Merged
merged 7 commits into from
Aug 29, 2016
31 changes: 26 additions & 5 deletions lib/fluent/config/configure_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ def overwrite_defaults(other) # other is owner plugin's corresponding proxy
end
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, type = nil, **kwargs, &block)
name = name.to_sym

Expand All @@ -216,16 +222,27 @@ def parameter_configuration(name, type = nil, **kwargs, &block)
opts.merge!(kwargs)

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 ||= @type_lookup.call(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])
Expand All @@ -235,6 +252,10 @@ def parameter_configuration(name, type = nil, **kwargs, &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

Expand Down Expand Up @@ -296,7 +317,7 @@ def desc(description)

def config_section(name, **kwargs, &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

Expand All @@ -305,10 +326,10 @@ def config_section(name, **kwargs, &block)

if sub_proxy.init?
if sub_proxy.argument && !sub_proxy.defaults.has_key?(sub_proxy.argument.first)
raise ArgumentError, "#{self.name}: init is specified, but default value of argument is missing"
raise ArgumentError, "#{name}: init is specified, but default value of argument is missing"
end
if sub_proxy.params.keys.any?{|param_name| !sub_proxy.defaults.has_key?(param_name)}
raise ArgumentError, "#{self.name}: init is specified, but there're parameters without default values"
raise ArgumentError, "#{name}: init is specified, but there're parameters without default values"
end
end

Expand Down
3 changes: 3 additions & 0 deletions lib/fluent/config/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,7 @@ class ConfigError < StandardError

class ConfigParseError < ConfigError
end

class ObsoletedParameterError < ConfigError
end
end
15 changes: 15 additions & 0 deletions lib/fluent/config/section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def self.generate(proxy, conf, logger, plugin_class, stack = [])
logger.error "config error in:\n#{conf}" if logger # logger should exist, but somethimes it's nil (e.g, in tests)
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|
Expand All @@ -138,6 +139,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}" if logger
Expand Down
15 changes: 2 additions & 13 deletions lib/fluent/plugin/out_forward.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,23 +89,12 @@ def initialize

attr_reader :nodes

# backward compatibility
config_param :port, :integer, default: LISTEN_PORT
config_param :host, :string, default: nil
config_param :port, :integer, default: LISTEN_PORT, obsoleted: "User <server> section instead."
config_param :host, :string, default: nil, obsoleted: "Use <server> section instead."

def configure(conf)
super

# 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 : LISTEN_PORT
element = conf.add_element('server')
element['host'] = host
element['port'] = port.to_s
end

recover_sample_size = @recover_wait / @heartbeat_interval

if @dns_round_robin
Expand Down
88 changes: 88 additions & 0 deletions test/config/test_configurable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,12 @@ class BufferChild
config_param :size_of_something, :size, default: 128
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
Expand Down Expand Up @@ -1102,5 +1108,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(config_element('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(config_element('ROOT', '', {'key2' => 'yay'}, []))
end
end
end
end
end
10 changes: 7 additions & 3 deletions test/config/test_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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