Skip to content

Commit a0aa929

Browse files
andrewvcandrewvc
committed
Validate deprecated/obsolete options after sanitization
The old order made no sense since Password / SafeURI objects were not wrapped in their to_s suppressing containers. Only remove the obsolete params after they've been detected Fixes elastic#8785
1 parent e370407 commit a0aa929

2 files changed

Lines changed: 59 additions & 31 deletions

File tree

‎logstash-core/lib/logstash/config/mixin.rb‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -60,30 +60,6 @@ def config_init(params)
6060
# store the plugin type, turns LogStash::Inputs::Base into 'input'
6161
@plugin_type = self.class.ancestors.find { |a| a.name =~ /::Base$/ }.config_name
6262

63-
# warn about deprecated variable use
64-
params.each do |name, value|
65-
opts = self.class.get_config[name]
66-
if opts && opts[:deprecated]
67-
extra = opts[:deprecated].is_a?(String) ? opts[:deprecated] : ""
68-
extra.gsub!("%PLUGIN%", self.class.config_name)
69-
self.logger.warn("You are using a deprecated config setting " +
70-
"#{name.inspect} set in #{self.class.config_name}. " +
71-
"Deprecated settings will continue to work, " +
72-
"but are scheduled for removal from logstash " +
73-
"in the future. #{extra} If you have any questions " +
74-
"about this, please visit the #logstash channel " +
75-
"on freenode irc.", :name => name, :plugin => self)
76-
77-
end
78-
if opts && opts[:obsolete]
79-
extra = opts[:obsolete].is_a?(String) ? opts[:obsolete] : ""
80-
extra.gsub!("%PLUGIN%", self.class.config_name)
81-
raise LogStash::ConfigurationError,
82-
I18n.t("logstash.runner.configuration.obsolete", :name => name,
83-
:plugin => self.class.config_name, :extra => extra)
84-
end
85-
end
86-
8763
# Set defaults from 'config :foo, :default => somevalue'
8864
self.class.get_config.each do |name, opts|
8965
next if params.include?(name.to_s)
@@ -109,17 +85,46 @@ def config_init(params)
10985
params[name.to_s] = deep_replace(value)
11086
end
11187

112-
11388
if !self.class.validate(params)
11489
raise LogStash::ConfigurationError,
11590
I18n.t("logstash.runner.configuration.invalid_plugin_settings")
11691
end
11792

93+
# now that we know the parameters are valid, we can obfuscate the original copy
94+
# of the parameters before storing them as an instance variable
95+
self.class.secure_params!(original_params)
96+
@original_params = original_params
97+
98+
# warn about deprecated variable use
99+
original_params.each do |name, value|
100+
opts = self.class.get_config[name]
101+
if opts && opts[:deprecated]
102+
extra = opts[:deprecated].is_a?(String) ? opts[:deprecated] : ""
103+
extra.gsub!("%PLUGIN%", self.class.config_name)
104+
self.logger.warn("You are using a deprecated config setting " +
105+
"#{name.inspect} set in #{self.class.config_name}. " +
106+
"Deprecated settings will continue to work, " +
107+
"but are scheduled for removal from logstash " +
108+
"in the future. #{extra} If you have any questions " +
109+
"about this, please visit the #logstash channel " +
110+
"on freenode irc.", :name => name, :plugin => self)
111+
112+
end
113+
114+
if opts && opts[:obsolete]
115+
extra = opts[:obsolete].is_a?(String) ? opts[:obsolete] : ""
116+
extra.gsub!("%PLUGIN%", self.class.config_name)
117+
raise LogStash::ConfigurationError,
118+
I18n.t("logstash.runner.configuration.obsolete", :name => name,
119+
:plugin => self.class.config_name, :extra => extra)
120+
end
121+
end
122+
118123
# We remove any config options marked as obsolete,
119124
# no code should be associated to them and their values should not bleed
120125
# to the plugin context.
121126
#
122-
# This need to be done after fetching the options from the parents classed
127+
# This need to be done after fetching the options from the parents class
123128
params.reject! do |name, value|
124129
opts = self.class.get_config[name]
125130
opts.include?(:obsolete)
@@ -134,11 +139,6 @@ def config_init(params)
134139
instance_variable_set("@#{key}", value)
135140
end
136141

137-
# now that we know the parameters are valid, we can obfuscate the original copy
138-
# of the parameters before storing them as an instance variable
139-
self.class.secure_params!(original_params)
140-
@original_params = original_params
141-
142142
@config = params
143143
end # def config_init
144144

‎logstash-core/spec/logstash/config/mixin_spec.rb‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,34 @@
33
require "logstash/config/mixin"
44

55
describe LogStash::Config::Mixin do
6+
context "when encountering a deprecated option" do
7+
let(:password) { "sekret" }
8+
let(:double_logger) { double("logger").as_null_object }
9+
10+
subject do
11+
Class.new(LogStash::Filters::Base) do
12+
include LogStash::Config::Mixin
13+
config_name "test_deprecated"
14+
milestone 1
15+
config :old_opt, :validate => :string, :deprecated => "this is old school"
16+
config :password, :validate => :password
17+
end.new({
18+
"old_opt" => "whut",
19+
"password" => password
20+
})
21+
end
22+
23+
it "should not log the password" do
24+
expect(LogStash::Logging::Logger).to receive(:new).with(anything).and_return(double_logger)
25+
expect(double_logger).to receive(:warn) do |arg1,arg2|
26+
message = 'You are using a deprecated config setting "old_opt" set in test_deprecated. Deprecated settings will continue to work, but are scheduled for removal from logstash in the future. this is old school If you have any questions about this, please visit the #logstash channel on freenode irc.'
27+
expect(arg1).to eq(message)
28+
expect(arg2[:plugin].to_s).to include('"password"=><password>')
29+
end.once
30+
subject
31+
end
32+
end
33+
634
context "when validating :bytes successfully" do
735
subject do
836
local_num_bytes = num_bytes # needs to be locally scoped :(

0 commit comments

Comments
 (0)